Skip to content

Commit f6495d8

Browse files
cvinayakdanieldegrasse
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 <vich@nordicsemi.no>
1 parent e0b706b commit f6495d8

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
@@ -968,8 +968,8 @@ void ll_reset(void)
968968
uint8_t ll_rx_get(void **node_rx, uint16_t *handle)
969969
{
970970
struct node_rx_pdu *rx;
971-
memq_link_t *link;
972971
uint8_t cmplt = 0U;
972+
memq_link_t *link;
973973

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

985985
*node_rx = NULL;
986986

987+
#if defined(CONFIG_BT_CONN) || defined(CONFIG_BT_CTLR_ADV_ISO)
988+
/* Save the tx_ack FIFO's last index to avoid the value being changed if there were no
989+
* Rx PDUs and we were pre-empted before calling `tx_cmplt_get()`, that may advance the
990+
* first index beyond ack_last value recorded in node_rx enqueued by `ll_rx_put()` call
991+
* when we are in the `else` clause below.
992+
*/
993+
uint8_t tx_ack_last = mfifo_fifo_tx_ack.l;
994+
995+
/* Ensure that the value is fetched before call to memq_peek, i.e. compiler shall not
996+
* reorder memory write before above read.
997+
*/
998+
cpu_dmb();
999+
#endif /* CONFIG_BT_CONN || CONFIG_BT_CTLR_ADV_ISO */
1000+
9871001
link = memq_peek(memq_ll_rx.head, memq_ll_rx.tail, (void **)&rx);
9881002
if (link) {
9891003
#if defined(CONFIG_BT_CONN) || defined(CONFIG_BT_CTLR_ADV_ISO)
@@ -1064,8 +1078,11 @@ uint8_t ll_rx_get(void **node_rx, uint16_t *handle)
10641078
#if defined(CONFIG_BT_CONN) || defined(CONFIG_BT_CTLR_ADV_ISO)
10651079
}
10661080
} else {
1067-
cmplt = tx_cmplt_get(handle, &mfifo_fifo_tx_ack.f,
1068-
mfifo_fifo_tx_ack.l);
1081+
/* Use the saved ack last value, before call was done to memq_peek, to ensure we
1082+
* do not advance the first index `f` beyond the value that was the last index `l`
1083+
* when memq_peek was called.
1084+
*/
1085+
cmplt = tx_cmplt_get(handle, &mfifo_fifo_tx_ack.f, tx_ack_last);
10691086
#endif /* CONFIG_BT_CONN || CONFIG_BT_CTLR_ADV_ISO */
10701087
}
10711088

0 commit comments

Comments
 (0)