Bluetooth: Host: Don't call user callback from TX thread

ATT is invoking user callbacks in its net_buf destroy function. It is
common practice that these callbacks can block on bt_hci_cmd_alloc(), so
we must consider these callbacks as blocking. This is a deadlock when
the net_buf_unref() happens inside the HCI driver, driver invoked from
tx_processor. Blocking callbacks like this appear in our own samples.
See further down about how this problem was detected.

Currently, tx_processor not protect against blocking callbacks so it is
de-facto forbidden. The Host should not equip net_bufs with dangerous
destroy callbacks.

This commit makes ATT defer its net_buf destruction and user callback
invocation to the system workqueue, so that net_buf_unref is safe to
call from non-blocking threads. Unsafe code is banished to the system
workqueue wild west. Future improvement may be to allow the user to
provide their own workqueue for ATT callbacks.

This deadlock was detected because the following test was failing while
tx_processor to the bt_taskq:

    tests/bsim/bluetooth/ll/throughput/tests_scripts/gatt_write.sh

The above test has an ATT callback `write_cmd_cb` invokes
`bt_conn_le_param_update` can block waiting for `tx_processor`.

Signed-off-by: Aleksander Wasaznik <[email protected]>

Author: alwa-nordic

subsys/bluetooth/host/att.c

@@ -248,8 +248,13 @@ const char *bt_att_err_to_str(uint8_t att_err)
 }
 #endif /* CONFIG_BT_ATT_ERR_TO_STR */
 
-static void att_tx_destroy(struct net_buf *buf)
+static void att_tx_destroy_work_handler(struct k_work *work);
+static K_WORK_DEFINE(att_tx_destroy_work, att_tx_destroy_work_handler);
+static sys_slist_t tx_destroy_queue;
+
+static void att_tx_destroy_work_handler(struct k_work *work)
 {
+	struct net_buf *buf = net_buf_slist_get(&tx_destroy_queue);
 	struct bt_att_tx_meta_data *p_meta = att_get_tx_meta_data(buf);
 	struct bt_att_tx_meta_data meta;
 
@@ -278,6 +283,17 @@ static void att_tx_destroy(struct net_buf *buf)
 	if (meta.opcode != 0) {
 		att_on_sent_cb(&meta);
 	}
+
+	if (!sys_slist_is_empty(&tx_destroy_queue)) {
+		k_work_submit_to_queue(bt_workq_chosen, &att_tx_destroy_work);
+	}
+}
+
+static void att_tx_destroy(struct net_buf *buf)
+{
+	/* We need to invoke `att_on_sent_cb` which may block. Defer to bt_workq. */
+	net_buf_slist_put(&tx_destroy_queue, buf);
+	k_work_submit_to_queue(bt_workq_chosen, &att_tx_destroy_work); /* att_tx_destroy_work_handler */
 }
 
 NET_BUF_POOL_DEFINE(att_pool, CONFIG_BT_ATT_TX_COUNT,