l2cap_data_pull hard fault on disconnect with queued tx data

[denis]

When using l2cap to stream data at a high rate from device to an app we are seeing a hard fault in l2cap_data_pull when there is queued l2cap send data.

When the fault occurs l2cap_data_pull, conn is in the disconnected state.  It appears that the pdu->data pointer is 0.  Then net_buf_push tries to adjust pdu->data which results in 0xfffffffc.  Dereferencing that causes the hard fault:

hdr = net_buf_push(pdu, sizeof(*hdr));

hdr->len = sys_cpu_to_le16(pdu_len);

Using nRF Connect SDK 2.9.0 and nRF5340.

Should there be a check for conn status disconnected?  Or for pdu->data == NULL?  Or is this a race condition?

[Amanda Hsieh]

Hi, 

The race condition could be in the your applications, please check if you already do something like in the linked code snippet to avoid calling APIs with the conn pointer while you unref the conn pointer on disconnected: 

https://github.com/nrfconnect/sdk-zephyr/blob/v3.7.99-ncs1/samples/bluetooth/central_gatt_write/src/central_gatt_write.c#L104-L116

Regards,
Amanda H.

[denis]

The  bt_l2cap_chan_send function that we use checks the channel connection state. Is that not adequate?

[denis]

I can't do that exactly because I'm using the l2cap calls, but I did test adding a ref/unref around the send and the same hard fault still happens.

struct bt_conn *conn = bt_conn_ref(conn_connected);

int result = bt_l2cap_chan_send(&pt_ble.l2cap.le_chan.chan, buf);

bt_conn_unref(conn);

[Amanda Hsieh]

Hi, 

It might be a race condition between l2cap_data_pull and bt_l2cap_chan_del or l2cap_chan_shutdown. It should not happen since l2cap_data_pull runs on the system work queue, which is forced to be non-preemptible by the Bluetooth subsystem Kconfig. Have you overridden this restriction and made the system work queue preemtible?

[denis]

We have not modified the SYSTEM_WORKQUEUE_PRIORITY configuration.

[Amanda Hsieh]

Could you try to surround bt_l2cap_dyn_chan_send in a k_sched_lock as a step toward debugging? This makes the thread temporarily cooperative. It can be done by applying the attached diff to NCS v2.9.0. 

Fullscreen bt_l2cap_chan_send-k_sched_lock.patch.txt Download

#!/usr/bin/env -S git apply
# Attachment to Nordic Devzone https://devzone.nordicsemi.com/f/nordic-q-a/118471/l2cap_data_pull-hard-fault-on-disconnect-with-queued-tx-data
diff --git a/subsys/bluetooth/host/l2cap.c b/subsys/bluetooth/host/l2cap.c
index ed185d12fd7..58bb4cf13aa 100644
--- a/subsys/bluetooth/host/l2cap.c
+++ b/subsys/bluetooth/host/l2cap.c
@@ -3283,7 +3283,18 @@ static int bt_l2cap_dyn_chan_send(struct bt_l2cap_le_chan *le_chan, struct net_b
 	return 0;
 }

-int bt_l2cap_chan_send(struct bt_l2cap_chan *chan, struct net_buf *buf)
+static int bt_l2cap_chan_send_(struct bt_l2cap_chan *chan, struct net_buf *buf);
+int bt_l2cap_chan_send(struct bt_l2cap_chan *chan, struct net_buf *buf)
+{
+	int err;
+
+	k_sched_lock();
+	err = bt_l2cap_chan_send_(chan, buf);
+	k_sched_unlock();
+
+	return err;
+}
+static int bt_l2cap_chan_send_(struct bt_l2cap_chan *chan, struct net_buf *buf)
 {
 	if (!buf || !chan) {
 		return -EINVAL;

Please let me know if it can work or not.  

[denis]

I added that change and was still able to reproduce the issue.

[Amanda Hsieh]

Could you provide a simple project to help us reproduce the issue?