From 100c57cf6502b8b3a288fba7a8dd88b41ffdddb0 Mon Sep 17 00:00:00 2001 From: Morten Priess Date: Mon, 6 May 2024 16:08:42 +0200 Subject: [PATCH 1/3] Bluetooth: controller: Prevent invalid compiler code reordering In ull_disable, it is imperative that the callback is set up before a second reference counter check, otherwise it may happen that an LLL done event has already passed when the disable callback and semaphore is assigned. This causes the HCI thread to wait until timeout and assert after ull_ticker_stop_with_mark. For certain compilers, due to compiler optimizations, it can be seen from the assembler code that the callback is assigned after the second reference counter check. By adding memory barriers, the code correctly reorders code to the expected sequence. Signed-off-by: Morten Priess --- subsys/bluetooth/controller/ll_sw/ull.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/subsys/bluetooth/controller/ll_sw/ull.c b/subsys/bluetooth/controller/ll_sw/ull.c index 631c84659241..65a1c9afef7b 100644 --- a/subsys/bluetooth/controller/ll_sw/ull.c +++ b/subsys/bluetooth/controller/ll_sw/ull.c @@ -1667,12 +1667,15 @@ int ull_disable(void *lll) if (!hdr || !ull_ref_get(hdr)) { return 0; } + cpu_dmb(); /* Ensure synchronized data access */ k_sem_init(&sem, 0, 1); hdr->disabled_param = &sem; hdr->disabled_cb = disabled_cb; + cpu_dmb(); /* Ensure synchronized data access */ + /* ULL_HIGH can run after we have call `ull_ref_get` and it can * decrement the ref count. Hence, handle this race condition by * ensuring that `disabled_cb` has been set while the ref count is still From a5b3a14c289cc0c93e441860eec291a66f97a41a Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Mon, 20 May 2024 09:46:28 +0200 Subject: [PATCH 2/3] Bluetooth: Controller: Refactor BT_CTLR_LE_ENC implementation Refactor reused function in BT_CTLR_LE_ENC feature. Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/hci/hci.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/subsys/bluetooth/controller/hci/hci.c b/subsys/bluetooth/controller/hci/hci.c index 50f4829f4624..c0158c9ae260 100644 --- a/subsys/bluetooth/controller/hci/hci.c +++ b/subsys/bluetooth/controller/hci/hci.c @@ -6319,7 +6319,7 @@ static void le_ltk_request(struct pdu_data *pdu_data, uint16_t handle, } static void encrypt_change(uint8_t err, uint16_t handle, - struct net_buf *buf) + struct net_buf *buf, bool encryption_on) { struct bt_hci_evt_encrypt_change *ep; @@ -6332,7 +6332,7 @@ static void encrypt_change(uint8_t err, uint16_t handle, ep->status = err; ep->handle = sys_cpu_to_le16(handle); - ep->encrypt = !err ? 1 : 0; + ep->encrypt = encryption_on ? 1 : 0; } #endif /* CONFIG_BT_CTLR_LE_ENC */ @@ -6457,7 +6457,7 @@ static void encode_data_ctrl(struct node_rx_pdu *node_rx, break; case PDU_DATA_LLCTRL_TYPE_START_ENC_RSP: - encrypt_change(0x00, handle, buf); + encrypt_change(0x00, handle, buf, true); break; #endif /* CONFIG_BT_CTLR_LE_ENC */ @@ -6474,7 +6474,7 @@ static void encode_data_ctrl(struct node_rx_pdu *node_rx, #if defined(CONFIG_BT_CTLR_LE_ENC) case PDU_DATA_LLCTRL_TYPE_REJECT_IND: encrypt_change(pdu_data->llctrl.reject_ind.error_code, handle, - buf); + buf, false); break; #endif /* CONFIG_BT_CTLR_LE_ENC */ From 303ff3df2d892dfa898257b1e31296d8d1b038a2 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Wed, 5 Jun 2024 12:47:12 +0200 Subject: [PATCH 3/3] Bluetooth: Controller: Use BT_HCI_ERR_UNSPECIFIED as needed A Host shall consider any error code that it does not explicitly understand equivalent to the error code Unspecified Error (0x1F). Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/hci/hci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subsys/bluetooth/controller/hci/hci.c b/subsys/bluetooth/controller/hci/hci.c index c0158c9ae260..574bdd5f4322 100644 --- a/subsys/bluetooth/controller/hci/hci.c +++ b/subsys/bluetooth/controller/hci/hci.c @@ -6330,7 +6330,7 @@ static void encrypt_change(uint8_t err, uint16_t handle, hci_evt_create(buf, BT_HCI_EVT_ENCRYPT_CHANGE, sizeof(*ep)); ep = net_buf_add(buf, sizeof(*ep)); - ep->status = err; + ep->status = err ? err : (encryption_on ? err : BT_HCI_ERR_UNSPECIFIED); ep->handle = sys_cpu_to_le16(handle); ep->encrypt = encryption_on ? 1 : 0; }