mtu_update/peripheral can crash if the connection context is NULL

[sayooj-atmosic]

Describe the bug

when the application tries to get the Attribute Maximum Transmission Unit when the peripheral is in not connected
The following function gets called:

bt_gatt_get_uatt_mtu(default_conn) < -- The default connection will be NULL
The above function calls
bt_att_get_uatt_mtu(conn)
Which in turn calls
att = att_get(conn);
att_get tries to access conn context without NULL check. Hence we end up in crash.

static struct bt_att *att_get(struct bt_conn *conn)
{
2990     struct bt_l2cap_chan *chan;
2991     struct bt_att_chan *att_chan;
2992 
2993     if (conn->state != BT_CONN_CONNECTED) {
2994         LOG_WRN("Not connected");
2995         return NULL;
2996     }
2997 
2998     chan = bt_l2cap_le_lookup_rx_cid(conn, BT_L2CAP_CID_ATT);
2999     if (!chan) {
3000         LOG_ERR("Unable to find ATT channel");
3001         return NULL;
3002     }
3003 
3004     att_chan = ATT_CHAN(chan);
3005     if (!atomic_test_bit(att_chan->flags, ATT_CONNECTED)) {
3006         LOG_ERR("ATT channel not connected");
3007         return NULL;
3008     }
3009 
3010     return att_chan->att;
3011 }

To Reproduce

Expected behavior

We can fix this by introducing a NULL check in the application:

diff --git a/samples/bluetooth/mtu_update/peripheral/src/peripheral_mtu_update.c b/samples/bluetooth/mtu_update/peripheral/src/peripheral_mtu_update.c
index a5bbef6f5ee..40d6b57d40d 100644
--- a/samples/bluetooth/mtu_update/peripheral/src/peripheral_mtu_update.c
+++ b/samples/bluetooth/mtu_update/peripheral/src/peripheral_mtu_update.c
@@ -98,7 +98,9 @@ void run_peripheral_sample(uint8_t *notify_data, size_t notify_data_size, uint16
        for (int i = 0; (i < seconds) || infinite; i++) {
                k_sleep(K_SECONDS(1));
                /* Only send the notification if the UATT MTU supports the required length */
-               if (bt_gatt_get_uatt_mtu(default_conn) >= ATT_NTF_SIZE(notify_data_size)) {
+               if (!default_conn) {
+                       printk("Skipping notification since connection is not yet established\n");
+               } else if (bt_gatt_get_uatt_mtu(default_conn) >= ATT_NTF_SIZE(notify_data_size)) {
                        bt_gatt_notify(default_conn, notify_crch, notify_data, notify_data_size);
                } else {
                        printk("Skipping notification since UATT MTU is not sufficient."

We may also have to introduce a NULL check in att_get to avoid this problem

Impact

Logs and console output

Environment (please complete the following information):

• OS: (e.g. Linux, MacOS, Windows)
• Toolchain (e.g Zephyr SDK, ...)
• Commit SHA or Version used

Additional context

[github-actions]

Hi @sayooj-atmosic! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[sayooj-atmosic]

I have a fix for this as explained above. I will raise it soon.

[sayooj-atmosic]

#88889