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]

If I add some checks to l2cap_data_pull then I don't see the hard fault:

struct net_buf *pdu = k_fifo_peek_head(&lechan->tx_queue);

// It seems there are items being taken from the fifo after the connection is closed. -denis

if (conn->state != BT_CONN_CONNECTED) {

return NULL;

}

if (pdu->data == NULL) {

return NULL;

}

[Amanda Hsieh]

If your application is calling bt_conn_unref()  from multiple threads/callbacks on a global variable conn pointer that the application has declared, then every threads/callbacks that pass this global variable  conn pointer will need  to take a reference count (call bt_conn_ref() to ensure the other thread does not unreference this global variable conn pointer.

[denis]

We only call bt_conn_ref and hold the conn in one (volatile) global when the connected callback of struct bt_conn_cb is called.  That global is zeroed and then bt_conn_unref is called from struct bt_conn_cb disconnected callback.  The conn is not referenced when sending.  The struct bt_l2cap_le_chan is used when sending.  There is only one place that calls bt_l2cap_chan_send.  I added a check there that the conn is not zero and it is never zero when bt_l2cap_chan_send is called.