Bluetooth: Host: Fix deadlock when failing to allocate att buf on BT RX

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]>

subsys/bluetooth/host/att.c

@@ -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)) {
+			/* No blocking in the sysqueue. */
+			timeout = K_NO_WAIT;
+		} else if (current_thread == att_handle_rsp_thread) {
+			/* No req will be fulfilled while blocking on the bt_recv thread.
+			 * Blocking would cause deadlock.
+			 */
+			LOG_DBG("Timeout discarded. No blocking on bt_recv thread.");
 			timeout = K_NO_WAIT;
 		} else {
 			timeout = K_FOREVER;