From 2506a7b41b4c2d04b554076610f661e940a956b2 Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Fri, 1 Nov 2024 15:05:30 +0100 Subject: [PATCH] Bluetooth: CSIP: Handle disconnects while in procedure If a device disconnects while we are in a procedure then get_next_active_instance would return a service instance pointer with the `conn` set to NULL. The issue was caused by the set_info being potentially memset when the device that disconnected was the one that held the set_info pointer. The solution is to not use a pointer, but rather a copy of the set_info, so that the active.set_info value is still valid after a disconnect. Since the set_info is not longer a pointer to a specific set_info from one of the members, the logs have been updated as well, as the pointer of the active.set_info is useless for debugging. Signed-off-by: Emil Gydesen --- subsys/bluetooth/audio/csip_set_coordinator.c | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/subsys/bluetooth/audio/csip_set_coordinator.c b/subsys/bluetooth/audio/csip_set_coordinator.c index 3436f512c49f5d..f27a85087ee1f9 100644 --- a/subsys/bluetooth/audio/csip_set_coordinator.c +++ b/subsys/bluetooth/audio/csip_set_coordinator.c @@ -58,7 +58,7 @@ LOG_MODULE_REGISTER(bt_csip_set_coordinator, CONFIG_BT_CSIP_SET_COORDINATOR_LOG_ static struct active_members { struct bt_csip_set_coordinator_set_member *members[CONFIG_BT_MAX_CONN]; - const struct bt_csip_set_coordinator_set_info *info; + struct bt_csip_set_coordinator_set_info info; uint8_t members_count; uint8_t members_handled; uint8_t members_restored; @@ -169,7 +169,7 @@ static struct bt_csip_set_coordinator_svc_inst *lookup_instance_by_set_info( member_set_info = &member->insts[i].info; if (member_set_info->set_size == set_info->set_size && - memcmp(&member_set_info->sirk, &set_info->sirk, sizeof(set_info->sirk)) == 0) { + memcmp(member_set_info->sirk, set_info->sirk, sizeof(set_info->sirk)) == 0) { return bt_csip_set_coordinator_lookup_instance_by_index(inst->conn, i); } } @@ -184,9 +184,9 @@ static struct bt_csip_set_coordinator_svc_inst *get_next_active_instance(void) member = active.members[active.members_handled]; - svc_inst = lookup_instance_by_set_info(member, active.info); + svc_inst = lookup_instance_by_set_info(member, &active.info); if (svc_inst == NULL) { - LOG_DBG("Failed to lookup instance by set_info %p", active.info); + LOG_DBG("Failed to lookup instance by set_info"); } return svc_inst; @@ -201,8 +201,8 @@ static int member_rank_compare_asc(const void *m1, const void *m2) struct bt_csip_set_coordinator_svc_inst *svc_inst_1; struct bt_csip_set_coordinator_svc_inst *svc_inst_2; - svc_inst_1 = lookup_instance_by_set_info(member_1, active.info); - svc_inst_2 = lookup_instance_by_set_info(member_2, active.info); + svc_inst_1 = lookup_instance_by_set_info(member_1, &active.info); + svc_inst_2 = lookup_instance_by_set_info(member_2, &active.info); if (svc_inst_1 == NULL) { LOG_ERR("svc_inst_1 was NULL for member %p", member_1); @@ -232,7 +232,7 @@ static void active_members_store_ordered(const struct bt_csip_set_coordinator_se { (void)memcpy(active.members, members, count * sizeof(members[0U])); active.members_count = count; - active.info = info; + memcpy(&active.info, info, sizeof(active.info)); if (count > 1U && CONFIG_BT_MAX_CONN > 1) { qsort(active.members, count, sizeof(members[0U]), @@ -1079,7 +1079,7 @@ static void csip_set_coordinator_write_restore_cb(struct bt_conn *conn, int csip_err; member = active.members[active.members_handled - active.members_restored - 1]; - client->cur_inst = lookup_instance_by_set_info(member, active.info); + client->cur_inst = lookup_instance_by_set_info(member, &active.info); if (client->cur_inst == NULL) { release_set_complete(-ENOENT); @@ -1114,9 +1114,9 @@ static void csip_set_coordinator_write_lock_cb(struct bt_conn *conn, active.members_restored = 0; member = active.members[active.members_handled - active.members_restored]; - client->cur_inst = lookup_instance_by_set_info(member, active.info); + client->cur_inst = lookup_instance_by_set_info(member, &active.info); if (client->cur_inst == NULL) { - LOG_DBG("Failed to lookup instance by set_info %p", active.info); + LOG_DBG("Failed to lookup instance by set_info"); lock_set_complete(-ENOENT); return; @@ -1214,7 +1214,7 @@ static void csip_set_coordinator_write_release_cb(struct bt_conn *conn, uint8_t static void csip_set_coordinator_lock_state_read_cb(int err, bool locked) { - const struct bt_csip_set_coordinator_set_info *info = active.info; + const struct bt_csip_set_coordinator_set_info *info = &active.info; struct bt_csip_set_coordinator_set_member *cur_member = NULL; if (err || locked) { @@ -1606,9 +1606,9 @@ csip_set_coordinator_get_lock_state(const struct bt_csip_set_coordinator_set_mem for (uint8_t i = 0U; i < count; i++) { struct bt_csip_set_coordinator_svc_inst *svc_inst; - svc_inst = lookup_instance_by_set_info(active.members[i], active.info); + svc_inst = lookup_instance_by_set_info(active.members[i], &active.info); if (svc_inst == NULL) { - LOG_DBG("Failed to lookup instance by set_info %p", active.info); + LOG_DBG("Failed to lookup instance by set_info"); active_members_reset(); return -ENOENT; @@ -1632,11 +1632,11 @@ csip_set_coordinator_get_lock_state(const struct bt_csip_set_coordinator_set_mem * here. */ if (active.oap_cb == NULL || - !active.oap_cb(active.info, active.members, active.members_count)) { + !active.oap_cb(&active.info, active.members, active.members_count)) { err = -ECANCELED; } - ordered_access_complete(active.info, err, false, NULL); + ordered_access_complete(&active.info, err, false, NULL); } return err; @@ -1718,9 +1718,9 @@ int bt_csip_set_coordinator_lock( active_members_store_ordered(members, count, set_info, true); - svc_inst = lookup_instance_by_set_info(active.members[0], active.info); + svc_inst = lookup_instance_by_set_info(active.members[0], &active.info); if (svc_inst == NULL) { - LOG_DBG("Failed to lookup instance by set_info %p", active.info); + LOG_DBG("Failed to lookup instance by set_info"); active_members_reset(); return -ENOENT; @@ -1764,9 +1764,9 @@ int bt_csip_set_coordinator_release(const struct bt_csip_set_coordinator_set_mem active_members_store_ordered(members, count, set_info, false); - svc_inst = lookup_instance_by_set_info(active.members[0], active.info); + svc_inst = lookup_instance_by_set_info(active.members[0], &active.info); if (svc_inst == NULL) { - LOG_DBG("Failed to lookup instance by set_info %p", active.info); + LOG_DBG("Failed to lookup instance by set_info"); active_members_reset(); return -ENOENT;