Skip to content

Commit

Permalink
Bluetooth: Host: Fix deadlock when failing to allocate att buf on BT RX
Browse files Browse the repository at this point in the history
thread

We are inferring in many bt_gatt_* functions that if called from a BT RX
thread (which is inherently the case if called from a callback when
running a Bluetooth application), we don't block and instead return
-ENOMEM when the ATT request queue is full, avoiding a deadlock.
This promise is fulfilled within bt_att_req_alloc, where the timeout for
allocation of the request slab is set to K_NO_WAIT if we are on the BT
RX thread. Unfortunately, we break this promise in
bt_att_chan_create_pdu, where the timeout for allocation of the att pool
is still K_FOREVER and deadlocks can (and do) occur when too many
requests are sent yet the pool is depleted.

Note: Both req_slab and att_pool sizes are defined by
CONFIG_BT_ATT_TX_COUNT. If applications start getting -ENOMEM with this
change, they were at risk of such a deadlock, and may increase
CONFIG_BT_ATT_TX_COUNT to allocate the att pool for their requests.

Note: This possible deadlock has been flying under the radar, as
att_pools are freed when the HCI driver has sent it to the controller
(instead of when receiving the response, as it happens with req_slabs)
and due to the att_pool and the req_slab being both sized by
CONFIG_BT_ATT_TX_COUNT, and req_slab being allocated before and
returning -ENOMEM already if there is no space, it takes a more specific
situation to deplete the att_pool but not the req_slab pool at this
point.

Signed-off-by: Kyra Lengfeld <[email protected]>
  • Loading branch information
KyraLengfeld committed Feb 27, 2025
1 parent f527494 commit 555b06b
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion subsys/bluetooth/host/att.c
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,15 @@ static struct net_buf *bt_att_chan_create_pdu(struct bt_att_chan *chan, uint8_t
timeout = BT_ATT_TIMEOUT;
break;
default:
if (k_current_get() == k_work_queue_thread_get(&k_sys_work_q)) {
k_tid_t current_thread = k_current_get();
if (current_thread == k_work_queue_thread_get(&k_sys_work_q)) {

Check warning on line 734 in subsys/bluetooth/host/att.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LINE_SPACING

subsys/bluetooth/host/att.c:734 Missing a blank line after declarations
/* No blocking in the sysqueue. */
timeout = K_NO_WAIT;
} else if(current_thread == att_handle_rsp_thread) {

Check failure on line 737 in subsys/bluetooth/host/att.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

SPACING

subsys/bluetooth/host/att.c:737 space required before the open parenthesis '('
/* No req will be fulfilled while blocking on the bt_recv thread.
* Blocking would cause deadlock.

Check warning on line 739 in subsys/bluetooth/host/att.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

BLOCK_COMMENT_STYLE

subsys/bluetooth/host/att.c:739 Block comments should align the * on each line
*/
LOG_DBG("Timeout discarded. No blocking on bt_recv thread.");
timeout = K_NO_WAIT;
} else {
timeout = K_FOREVER;
Expand Down

0 comments on commit 555b06b

Please sign in to comment.