Skip to content

Commit 6464ffa

Browse files
KyraLengfeldkartben
authored andcommitted
Bluetooth: Host: Fix deadlock when failing to alloc on BT RX thread
This commit alignes the timeout value for allocating buffers within att on the BT RX thread, making it consistent within att.c, see bt_att_req_alloc. 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. Note: Ideally, we don't want functions to behave differently depending on which thread they are running, and while this commit makes it more consistent, it should be considered a workaround solution. Signed-off-by: Kyra Lengfeld <[email protected]>
1 parent 210adaf commit 6464ffa

File tree

1 file changed

+7
-1
lines changed
  • subsys/bluetooth/host

1 file changed

+7
-1
lines changed

subsys/bluetooth/host/att.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,13 @@ static struct net_buf *bt_att_chan_create_pdu(struct bt_att_chan *chan, uint8_t
730730
timeout = BT_ATT_TIMEOUT;
731731
break;
732732
default:
733-
if (k_current_get() == k_work_queue_thread_get(&k_sys_work_q)) {
733+
k_tid_t current_thread = k_current_get();
734+
735+
if (current_thread == k_work_queue_thread_get(&k_sys_work_q)) {
736+
/* No blocking in the sysqueue. */
737+
timeout = K_NO_WAIT;
738+
} else if (current_thread == att_handle_rsp_thread) {
739+
/* Blocking would cause deadlock. */
734740
timeout = K_NO_WAIT;
735741
} else {
736742
timeout = K_FOREVER;

0 commit comments

Comments
 (0)