Bluetooth: Host: Fix resource leak in bt_gatt_unsubscribe

There are two simmilar functions for unsubscribing from GATT handles.

 - `gatt_sub_remove`, which is called on disconnect for every
 subscription will free the `subscriptions` entry when the entry
 represents no subscriptions.

 - `bt_gatt_unsubscribe`, called by the application, which forgets to
 free the `subscriptions` entry.

If all subscriptions grouped in a `subscriptions` entry are removed
using `bt_gatt_unsubscribe` before disconnect, there are no
subscriptions left to call `gatt_sub_remove` on. The `subscriptions`
entry is then never freed.

The above results in a resource leak of a `subscriptions` entry.

This fix makes explicit and enforces the invariant that there should not
be entries in `subscriptions` with an empty subscription list.

Fixes #38688

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

Author: alwa-nordic

subsys/bluetooth/host/gatt.c

@@ -70,6 +70,11 @@ struct gatt_sub {
 #define SUB_MAX 0
 #endif /* CONFIG_BT_GATT_CLIENT */
 
+/**
+ * Entry x is free for reuse whenever (subscriptions[x].peer == BT_ADDR_LE_ANY).
+ * Invariant: (sys_slist_is_empty(subscriptions[x].list))
+ *              <=> (subscriptions[x].peer == BT_ADDR_LE_ANY).
+ */
 static struct gatt_sub subscriptions[SUB_MAX];
 static sys_slist_t callback_list;
 
@@ -2699,6 +2704,19 @@ bool bt_gatt_is_subscribed(struct bt_conn *conn,
 	return false;
 }
 
+static bool gatt_sub_is_empty(struct gatt_sub *sub)
+{
+	return sys_slist_is_empty(&sub->list);
+}
+
+/** @brief Free sub for reuse.
+ */
+static void gatt_sub_free(struct gatt_sub *sub)
+{
+	__ASSERT_NO_MSG(gatt_sub_is_empty(sub));
+	bt_addr_le_copy(&sub->peer, BT_ADDR_LE_ANY);
+}
+
 static void gatt_sub_remove(struct bt_conn *conn, struct gatt_sub *sub,
 			    sys_snode_t *prev,
 			    struct bt_gatt_subscribe_params *params)
@@ -2710,9 +2728,8 @@ static void gatt_sub_remove(struct bt_conn *conn, struct gatt_sub *sub,
 		params->notify(conn, params, NULL, 0);
 	}
 
-	if (sys_slist_is_empty(&sub->list)) {
-		/* Reset address if there are no subscription left */
-		bt_addr_le_copy(&sub->peer, BT_ADDR_LE_ANY);
+	if (gatt_sub_is_empty(sub)) {
+		gatt_sub_free(sub);
 	}
 }
 
@@ -4597,6 +4614,10 @@ int bt_gatt_unsubscribe(struct bt_conn *conn,
 		return -EINVAL;
 	}
 
+	if (gatt_sub_is_empty(sub)) {
+		gatt_sub_free(sub);
+	}
+
 	if (has_subscription) {
 		/* Notify with NULL data to complete unsubscribe */
 		params->notify(conn, params, NULL, 0);