Skip to content

Commit 323240e

Browse files
cvinayakgithub-actions[bot]
authored andcommitted
Bluetooth: Controller: Fix Tx Ack FIFO index corruption under race
Fix Tx Ack FIFO's first index being advanced beyond a recorded ack_last value in a node_rx when under race between ll_rx_get() being pre-empted while executing the `tx_cmplt_get()` and a call to `ll_rx_put()` in an interrupt service routine. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]> (cherry picked from commit f6495d8)
1 parent e57a404 commit 323240e

File tree

1 file changed

+20
-3
lines changed
  • subsys/bluetooth/controller/ll_sw

1 file changed

+20
-3
lines changed

subsys/bluetooth/controller/ll_sw/ull.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -959,8 +959,8 @@ void ll_reset(void)
959959
uint8_t ll_rx_get(void **node_rx, uint16_t *handle)
960960
{
961961
struct node_rx_pdu *rx;
962-
memq_link_t *link;
963962
uint8_t cmplt = 0U;
963+
memq_link_t *link;
964964

965965
#if defined(CONFIG_BT_CONN) || \
966966
(defined(CONFIG_BT_OBSERVER) && defined(CONFIG_BT_CTLR_ADV_EXT)) || \
@@ -975,6 +975,20 @@ uint8_t ll_rx_get(void **node_rx, uint16_t *handle)
975975

976976
*node_rx = NULL;
977977

978+
#if defined(CONFIG_BT_CONN) || defined(CONFIG_BT_CTLR_ADV_ISO)
979+
/* Save the tx_ack FIFO's last index to avoid the value being changed if there were no
980+
* Rx PDUs and we were pre-empted before calling `tx_cmplt_get()`, that may advance the
981+
* first index beyond ack_last value recorded in node_rx enqueued by `ll_rx_put()` call
982+
* when we are in the `else` clause below.
983+
*/
984+
uint8_t tx_ack_last = mfifo_fifo_tx_ack.l;
985+
986+
/* Ensure that the value is fetched before call to memq_peek, i.e. compiler shall not
987+
* reorder memory write before above read.
988+
*/
989+
cpu_dmb();
990+
#endif /* CONFIG_BT_CONN || CONFIG_BT_CTLR_ADV_ISO */
991+
978992
link = memq_peek(memq_ll_rx.head, memq_ll_rx.tail, (void **)&rx);
979993
if (link) {
980994
#if defined(CONFIG_BT_CONN) || defined(CONFIG_BT_CTLR_ADV_ISO)
@@ -1055,8 +1069,11 @@ uint8_t ll_rx_get(void **node_rx, uint16_t *handle)
10551069
#if defined(CONFIG_BT_CONN) || defined(CONFIG_BT_CTLR_ADV_ISO)
10561070
}
10571071
} else {
1058-
cmplt = tx_cmplt_get(handle, &mfifo_fifo_tx_ack.f,
1059-
mfifo_fifo_tx_ack.l);
1072+
/* Use the saved ack last value, before call was done to memq_peek, to ensure we
1073+
* do not advance the first index `f` beyond the value that was the last index `l`
1074+
* when memq_peek was called.
1075+
*/
1076+
cmplt = tx_cmplt_get(handle, &mfifo_fifo_tx_ack.f, tx_ack_last);
10601077
#endif /* CONFIG_BT_CONN || CONFIG_BT_CTLR_ADV_ISO */
10611078
}
10621079

0 commit comments

Comments
 (0)