From 4350021f09c486405f1b92ca8da7137fb2e82a89 Mon Sep 17 00:00:00 2001 From: Joakim Andersson Date: Thu, 19 Nov 2020 17:11:09 +0100 Subject: [PATCH] Bluetooth: host: Make connection lookup functions thread-safe Make the connection lookup functions thread-safe by re-using the bt_conn_ref returning NULL mechanism and keeping a valid reference. Signed-off-by: Joakim Andersson --- subsys/bluetooth/host/conn.c | 133 ++++++++++++++++---------- subsys/bluetooth/host/conn_internal.h | 6 +- 2 files changed, 89 insertions(+), 50 deletions(-) diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index 5d5a48dc56a..034daa82730 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -404,7 +404,7 @@ struct bt_conn *bt_conn_new(struct bt_conn *conns, size_t size) int i; for (i = 0; i < size; i++) { - if (!atomic_get(&conns[i].ref)) { + if (atomic_cas(&conns[i].ref, 0, 1)) { conn = &conns[i]; break; } @@ -414,9 +414,7 @@ struct bt_conn *bt_conn_new(struct bt_conn *conns, size_t size) return NULL; } - (void)memset(conn, 0, sizeof(*conn)); - - atomic_set(&conn->ref, 1); + (void)memset(conn, 0, offsetof(struct bt_conn, ref)); return conn; } @@ -567,17 +565,23 @@ struct bt_conn *bt_conn_lookup_addr_sco(const bt_addr_t *peer) int i; for (i = 0; i < ARRAY_SIZE(sco_conns); i++) { - if (!atomic_get(&sco_conns[i].ref)) { + struct bt_conn *conn = bt_conn_ref(&sco_conns[i]); + + if (!conn) { continue; } - if (sco_conns[i].type != BT_CONN_TYPE_SCO) { + if (conn->type != BT_CONN_TYPE_SCO) { + bt_conn_unref(conn); continue; } - if (!bt_addr_cmp(peer, &sco_conns[i].sco.acl->br.dst)) { - return bt_conn_ref(&sco_conns[i]); + if (bt_addr_cmp(peer, &conn->sco.acl->br.dst) != 0) { + bt_conn_unref(conn); + continue; } + + return conn; } return NULL; @@ -588,17 +592,23 @@ struct bt_conn *bt_conn_lookup_addr_br(const bt_addr_t *peer) int i; for (i = 0; i < ARRAY_SIZE(acl_conns); i++) { - if (!atomic_get(&acl_conns[i].ref)) { + struct bt_conn *conn = bt_conn_ref(&acl_conns[i]); + + if (!conn) { continue; } - if (acl_conns[i].type != BT_CONN_TYPE_BR) { + if (conn->type != BT_CONN_TYPE_BR) { + bt_conn_unref(conn); continue; } - if (!bt_addr_cmp(peer, &acl_conns[i].br.dst)) { - return bt_conn_ref(&acl_conns[i]); + if (bt_addr_cmp(peer, &conn->br.dst) != 0) { + bt_conn_unref(conn); + continue; } + + return conn; } return NULL; @@ -1453,18 +1463,24 @@ struct bt_conn *conn_lookup_handle(struct bt_conn *conns, size_t size, int i; for (i = 0; i < size; i++) { - if (!atomic_get(&conns[i].ref)) { + struct bt_conn *conn = bt_conn_ref(&conns[i]); + + if (!conn) { continue; } /* We only care about connections with a valid handle */ - if (!bt_conn_is_handle_valid(&conns[i])) { + if (!bt_conn_is_handle_valid(conn)) { + bt_conn_unref(conn); continue; } - if (conns[i].handle == handle) { - return bt_conn_ref(&conns[i]); + if (conn->handle != handle) { + bt_conn_unref(conn); + continue; } + + return conn; } return NULL; @@ -1476,17 +1492,21 @@ struct bt_conn *conn_lookup_iso(struct bt_conn *conn) int i; for (i = 0; i < ARRAY_SIZE(iso_conns); i++) { - if (!atomic_get(&iso_conns[i].ref)) { + struct bt_conn *iso_conn = bt_conn_ref(&iso_conns[i]); + + if (!iso_conn) { continue; } - if (&iso_conns[i] == conn) { - return bt_conn_ref(conn); + if (iso_conn == conn) { + return iso_conn; } - if (iso_conns[i].iso.acl == conn) { - return bt_conn_ref(&iso_conns[i]); + if (conn->iso.acl == conn) { + return iso_conn; } + + bt_conn_unref(iso_conn); } return NULL; @@ -1735,17 +1755,23 @@ struct bt_conn *bt_conn_lookup_addr_le(uint8_t id, const bt_addr_le_t *peer) int i; for (i = 0; i < ARRAY_SIZE(acl_conns); i++) { - if (!atomic_get(&acl_conns[i].ref)) { + struct bt_conn *conn = bt_conn_ref(&acl_conns[i]); + + if (!conn) { continue; } - if (acl_conns[i].type != BT_CONN_TYPE_LE) { + if (conn->type != BT_CONN_TYPE_LE) { + bt_conn_unref(conn); continue; } - if (bt_conn_is_peer_addr_le(&acl_conns[i], id, peer)) { - return bt_conn_ref(&acl_conns[i]); + if (!bt_conn_is_peer_addr_le(conn, id, peer)) { + bt_conn_unref(conn); + continue; } + + return conn; } return NULL; @@ -1757,21 +1783,28 @@ struct bt_conn *bt_conn_lookup_state_le(uint8_t id, const bt_addr_le_t *peer, int i; for (i = 0; i < ARRAY_SIZE(acl_conns); i++) { - if (!atomic_get(&acl_conns[i].ref)) { + struct bt_conn *conn = bt_conn_ref(&acl_conns[i]); + + if (!conn) { continue; } - if (acl_conns[i].type != BT_CONN_TYPE_LE) { + if (conn->type != BT_CONN_TYPE_LE) { + bt_conn_ref(conn); continue; } - if (peer && !bt_conn_is_peer_addr_le(&acl_conns[i], id, peer)) { + if (peer && !bt_conn_is_peer_addr_le(conn, id, peer)) { + bt_conn_unref(conn); continue; } - if (acl_conns[i].state == state && acl_conns[i].id == id) { - return bt_conn_ref(&acl_conns[i]); + if (!(conn->state == state && conn->id == id)) { + bt_conn_unref(conn); + continue; } + + return conn; } return NULL; @@ -1783,24 +1816,31 @@ void bt_conn_foreach(int type, void (*func)(struct bt_conn *conn, void *data), int i; for (i = 0; i < ARRAY_SIZE(acl_conns); i++) { - if (!atomic_get(&acl_conns[i].ref)) { + struct bt_conn *conn = bt_conn_ref(&acl_conns[i]); + + if (!conn) { continue; } - if (!(acl_conns[i].type & type)) { + if (!(conn->type & type)) { + bt_conn_unref(conn); continue; } - func(&acl_conns[i], data); + func(conn, data); + bt_conn_unref(conn); } #if defined(CONFIG_BT_BREDR) if (type & BT_CONN_TYPE_SCO) { for (i = 0; i < ARRAY_SIZE(sco_conns); i++) { - if (!atomic_get(&sco_conns[i].ref)) { + struct bt_conn *conn = bt_conn_ref(&sco_conns[i]); + + if (!conn) { continue; } - func(&sco_conns[i], data); + func(conn, data); + bt_conn_unref(conn); } } #endif /* defined(CONFIG_BT_BREDR) */ @@ -1808,11 +1848,14 @@ void bt_conn_foreach(int type, void (*func)(struct bt_conn *conn, void *data), #if defined(CONFIG_BT_ISO) if (type & BT_CONN_TYPE_ISO) { for (i = 0; i < ARRAY_SIZE(iso_conns); i++) { - if (!atomic_get(&iso_conns[i].ref)) { + struct bt_conn *conn = bt_conn_ref(&iso_conns[i]); + + if (!conn) { continue; } - func(&iso_conns[i], data); + func(conn, data); + bt_conn_unref(conn); } } #endif /* defined(CONFIG_BT_ISO) */ @@ -2616,19 +2659,11 @@ uint8_t bt_conn_index(struct bt_conn *conn) struct bt_conn *bt_conn_lookup_index(uint8_t index) { - struct bt_conn *conn; - if (index >= ARRAY_SIZE(acl_conns)) { return NULL; } - conn = &acl_conns[index]; - - if (!atomic_get(&conn->ref)) { - return NULL; - } - - return bt_conn_ref(conn); + return bt_conn_ref(&acl_conns[index]); } int bt_conn_init(void) @@ -2651,9 +2686,9 @@ int bt_conn_init(void) /* Initialize background scan */ if (IS_ENABLED(CONFIG_BT_CENTRAL)) { for (i = 0; i < ARRAY_SIZE(acl_conns); i++) { - struct bt_conn *conn = &acl_conns[i]; + struct bt_conn *conn = bt_conn_ref(&acl_conns[i]); - if (!atomic_get(&conn->ref)) { + if (!conn) { continue; } @@ -2665,6 +2700,8 @@ int bt_conn_init(void) bt_conn_set_state(conn, BT_CONN_CONNECT_SCAN); } #endif /* !defined(CONFIG_BT_WHITELIST) */ + + bt_conn_unref(conn); } } diff --git a/subsys/bluetooth/host/conn_internal.h b/subsys/bluetooth/host/conn_internal.h index 4518c97eaf4..d9e1b4f5f7d 100644 --- a/subsys/bluetooth/host/conn_internal.h +++ b/subsys/bluetooth/host/conn_internal.h @@ -169,8 +169,6 @@ struct bt_conn { /* Active L2CAP/ISO channels */ sys_slist_t channels; - atomic_t ref; - /* Delayed work deferred tasks: * - Peripheral delayed connection update. * - Initiator connect create cancel. @@ -196,6 +194,10 @@ struct bt_conn { uint16_t subversion; } rv; #endif + /* Must be at the end so that everything else in the structure can be + * memset to zero without affecting the ref. + */ + atomic_t ref; }; void bt_conn_reset_rx_state(struct bt_conn *conn);