Bluetooth: controller: Guard against race in conn. establishment

In the time between a NODE_RX_TYPE_CONNECTION node is sent from LLL and
demuxed in ULL, an ADV role disable may be executed.
This makes the LLL data referenced in the node NULL/invald, and
ull_conn_setup would operate on invalid data.

This commit introduces a check in ADV disable to disallow the operation
(including conn invalidation), if a connection has been initiated.

To prevent pipeline-queued prepares from advertising after disable has
been initiated, set 'cancelled' flag for immediate signalling to LLL.

Signed-off-by: Morten Priess <mtpr@oticon.com>
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

subsys/bluetooth/controller/ll_sw/lll_conn.h

@@ -64,6 +64,7 @@ struct lll_conn {
 #if defined(CONFIG_BT_PERIPHERAL)
 		struct {
 			uint8_t  initiated:1;
+			uint8_t  cancelled:1;
 			uint8_t  latency_enabled:1;
 
 			uint32_t window_widening_periodic_us;

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c

@@ -741,10 +741,14 @@ static int prepare_cb(struct lll_prepare_param *p)
 	lll = p->param;
 
 #if defined(CONFIG_BT_PERIPHERAL)
-	/* Check if stopped (on connection establishment race between LLL and
+	/* Check if stopped (on connection establishment- or disabled race
-	 * ULL.
+	 * between LLL and ULL.
+	 * When connectable advertising is disabled in thread context, cancelled
+	 * flag is set, and initiated flag is checked. Here, we avoid
+	 * transmitting connectable advertising event if cancelled flag is set.
 	 */
-	if (unlikely(lll->conn && lll->conn->slave.initiated)) {
+	if (unlikely(lll->conn &&
+		(lll->conn->slave.initiated || lll->conn->slave.cancelled))) {
 		radio_isr_set(lll_isr_early_abort, lll);
 		radio_disable();
 
@@ -1079,7 +1083,14 @@ static void isr_done(void *param)
 	}
 #endif /* CONFIG_BT_PERIPHERAL */
 
-	if (lll->chan_map_curr) {
+	/* NOTE: Do not continue to connectable advertise if advertising is
+	 *       being disabled, by checking the cancelled flag.
+	 */
+	if (lll->chan_map_curr &&
+#if defined(CONFIG_BT_PERIPHERAL)
+	    (!lll->conn || !lll->conn->slave.cancelled) &&
+#endif /* CONFIG_BT_PERIPHERAL */
+	    1) {
 		struct pdu_adv *pdu;
 		uint32_t start_us;
 
@@ -1321,12 +1332,22 @@ static inline int isr_rx_pdu(struct lll_adv *lll,
 		return 0;
 
 #if defined(CONFIG_BT_PERIPHERAL)
+	/* NOTE: Do not accept CONNECT_IND if cancelled flag is set in thread
+	 *       context when disabling connectable advertising. This is to
+	 *       avoid any race in checking the initiated flags in thread mode
+	 *       which is set here if accepting a connection establishment.
+	 *
+	 *       Under this race, peer central would get failed to establish
+	 *       connection as the disconnect reason. This is an acceptable
+	 *       outcome to keep the thread mode implementation simple when
+	 *       disabling connectable advertising.
+	 */
 	} else if ((pdu_rx->type == PDU_ADV_TYPE_CONNECT_IND) &&
 		   (pdu_rx->len == sizeof(struct pdu_adv_connect_ind)) &&
+		   lll->conn && !lll->conn->slave.cancelled &&
 		   lll_adv_connect_ind_check(lll, pdu_rx, tx_addr, addr,
 					     rx_addr, tgt_addr,
-					     devmatch_ok, &rl_idx) &&
+					     devmatch_ok, &rl_idx)) {
-		   lll->conn) {
 		struct node_rx_ftr *ftr;
 		struct node_rx_pdu *rx;
 

subsys/bluetooth/controller/ll_sw/ull_adv.c

@@ -874,6 +874,7 @@ uint8_t ll_adv_enable(uint8_t enable)
 		/* FIXME: BEGIN: Move to ULL? */
 		conn_lll->role = 1;
 		conn_lll->slave.initiated = 0;
+		conn_lll->slave.cancelled = 0;
 		conn_lll->data_chan_sel = 0;
 		conn_lll->data_chan_use = 0;
 		conn_lll->event_counter = 0;
@@ -1995,6 +1996,21 @@ static inline uint8_t disable(uint8_t handle)
 		return BT_HCI_ERR_CMD_DISALLOWED;
 	}
 
+#if defined(CONFIG_BT_PERIPHERAL)
+	if (adv->lll.conn) {
+		/* Indicate to LLL that a cancellation is requested */
+		adv->lll.conn->slave.cancelled = 1U;
+		cpu_dmb();
+
+		/* Check if a connection was initiated (connection
+		 * establishment race between LLL and ULL).
+		 */
+		if (unlikely(adv->lll.conn->slave.initiated)) {
+			return BT_HCI_ERR_CMD_DISALLOWED;
+		}
+	}
+#endif /* CONFIG_BT_PERIPHERAL */
+
 	mark = ull_disable_mark(adv);
 	LL_ASSERT(mark == adv);
 
@@ -2012,7 +2028,7 @@ static inline uint8_t disable(uint8_t handle)
 			return BT_HCI_ERR_CMD_DISALLOWED;
 		}
 	}
-#endif
+#endif /* CONFIG_BT_PERIPHERAL */
 
 	ret_cb = TICKER_STATUS_BUSY;
 	ret = ticker_stop(TICKER_INSTANCE_ID_CTLR, TICKER_USER_ID_THREAD,