Bluetooth: host: Fix missing connection id checks

Fix issue where a new connection with the same peer would use the CCC
from from first connection, despite different local identity.
Since there is no CCC for the new connection yet this caused the
application to think that CCC was enabled but the remote device had not
yet subscribed.

Fix this issue by making the id as an input to the peer address check
function. This will force us to make the check every time. This commit
might also fix similar issues not yes discovered as the ID check was
missing in a few other places as well.

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
This commit is contained in:
Joakim Andersson 2020-02-19 18:48:53 +01:00 committed by Johan Hedberg
commit 58908aa5d0
4 changed files with 103 additions and 59 deletions

View file

@ -1803,19 +1803,24 @@ struct bt_conn *bt_conn_lookup_handle(u16_t handle)
return NULL;
}
int bt_conn_addr_le_cmp(const struct bt_conn *conn, const bt_addr_le_t *peer)
bool bt_conn_is_peer_addr_le(const struct bt_conn *conn, u8_t id,
const bt_addr_le_t *peer)
{
if (id != conn->id) {
return false;
}
/* Check against conn dst address as it may be the identity address */
if (!bt_addr_le_cmp(peer, &conn->le.dst)) {
return 0;
return true;
}
/* Check against initial connection address */
if (conn->role == BT_HCI_ROLE_MASTER) {
return bt_addr_le_cmp(peer, &conn->le.resp_addr);
return bt_addr_le_cmp(peer, &conn->le.resp_addr) == 0;
}
return bt_addr_le_cmp(peer, &conn->le.init_addr);
return bt_addr_le_cmp(peer, &conn->le.init_addr) == 0;
}
struct bt_conn *bt_conn_lookup_addr_le(u8_t id, const bt_addr_le_t *peer)
@ -1831,8 +1836,7 @@ struct bt_conn *bt_conn_lookup_addr_le(u8_t id, const bt_addr_le_t *peer)
continue;
}
if (conns[i].id == id &&
!bt_conn_addr_le_cmp(&conns[i], peer)) {
if (bt_conn_is_peer_addr_le(&conns[i], id, peer)) {
return bt_conn_ref(&conns[i]);
}
}
@ -1840,7 +1844,7 @@ struct bt_conn *bt_conn_lookup_addr_le(u8_t id, const bt_addr_le_t *peer)
return NULL;
}
struct bt_conn *bt_conn_lookup_state_le(const bt_addr_le_t *peer,
struct bt_conn *bt_conn_lookup_state_le(u8_t id, const bt_addr_le_t *peer,
const bt_conn_state_t state)
{
int i;
@ -1854,11 +1858,11 @@ struct bt_conn *bt_conn_lookup_state_le(const bt_addr_le_t *peer,
continue;
}
if (peer && bt_conn_addr_le_cmp(&conns[i], peer)) {
if (peer && !bt_conn_is_peer_addr_le(&conns[i], id, peer)) {
continue;
}
if (conns[i].state == state) {
if (conns[i].state == state && conns[i].id == id) {
return bt_conn_ref(&conns[i]);
}
}
@ -2114,7 +2118,8 @@ int bt_conn_create_auto_le(const struct bt_le_conn_param *param)
return -EINVAL;
}
conn = bt_conn_lookup_state_le(BT_ADDR_LE_NONE, BT_CONN_CONNECT_AUTO);
conn = bt_conn_lookup_state_le(BT_ID_DEFAULT, BT_ADDR_LE_NONE,
BT_CONN_CONNECT_AUTO);
if (conn) {
bt_conn_unref(conn);
return -EALREADY;
@ -2169,7 +2174,8 @@ int bt_conn_create_auto_stop(void)
return -EINVAL;
}
conn = bt_conn_lookup_state_le(BT_ADDR_LE_NONE, BT_CONN_CONNECT_AUTO);
conn = bt_conn_lookup_state_le(BT_ID_DEFAULT, BT_ADDR_LE_NONE,
BT_CONN_CONNECT_AUTO);
if (!conn) {
return -EINVAL;
}

View file

@ -200,9 +200,9 @@ void bt_conn_disconnect_all(u8_t id);
/* Look up an existing connection */
struct bt_conn *bt_conn_lookup_handle(u16_t handle);
/* Compare an address with bt_conn destination address */
int bt_conn_addr_le_cmp(const struct bt_conn *conn, const bt_addr_le_t *peer);
/* Check if the connection is with the given peer. */
bool bt_conn_is_peer_addr_le(const struct bt_conn *conn, u8_t id,
const bt_addr_le_t *peer);
/* Helpers for identifying & looking up connections based on the the index to
* the connection list. This is useful for O(1) lookups, but can't be used
@ -214,7 +214,7 @@ struct bt_conn *bt_conn_lookup_index(u8_t index);
/* Look up a connection state. For BT_ADDR_LE_ANY, returns the first connection
* with the specific state
*/
struct bt_conn *bt_conn_lookup_state_le(const bt_addr_le_t *peer,
struct bt_conn *bt_conn_lookup_state_le(u8_t id, const bt_addr_le_t *peer,
const bt_conn_state_t state);
/* Set connection object in certain state and perform action related to state */

View file

@ -59,6 +59,7 @@ struct ccc_store {
};
struct gatt_sub {
u8_t id;
bt_addr_le_t peer;
sys_slist_t list;
};
@ -430,12 +431,14 @@ static struct gatt_cf_cfg *find_cf_cfg(struct bt_conn *conn)
int i;
for (i = 0; i < ARRAY_SIZE(cf_cfg); i++) {
struct gatt_cf_cfg *cfg = &cf_cfg[i];
if (!conn) {
if (!bt_addr_le_cmp(&cf_cfg[i].peer, BT_ADDR_LE_ANY)) {
return &cf_cfg[i];
if (!bt_addr_le_cmp(&cfg->peer, BT_ADDR_LE_ANY)) {
return cfg;
}
} else if (!bt_conn_addr_le_cmp(conn, &cf_cfg[i].peer)) {
return &cf_cfg[i];
} else if (bt_conn_is_peer_addr_le(conn, cfg->id, &cfg->peer)) {
return cfg;
}
}
@ -518,6 +521,7 @@ static ssize_t cf_write(struct bt_conn *conn, const struct bt_gatt_attr *attr,
}
bt_addr_le_copy(&cfg->peer, &conn->le.dst);
cfg->id = conn->id;
atomic_set_bit(cfg->flags, CF_CHANGE_AWARE);
return len;
@ -1406,13 +1410,15 @@ static struct bt_gatt_ccc_cfg *find_ccc_cfg(const struct bt_conn *conn,
struct _bt_gatt_ccc *ccc)
{
for (size_t i = 0; i < ARRAY_SIZE(ccc->cfg); i++) {
struct bt_gatt_ccc_cfg *cfg = &ccc->cfg[i];
if (conn) {
if (conn->id == ccc->cfg[i].id &&
!bt_conn_addr_le_cmp(conn, &ccc->cfg[i].peer)) {
return &ccc->cfg[i];
if (bt_conn_is_peer_addr_le(conn, cfg->id,
&cfg->peer)) {
return cfg;
}
} else if (!bt_addr_le_cmp(&ccc->cfg[i].peer, BT_ADDR_LE_ANY)) {
return &ccc->cfg[i];
} else if (!bt_addr_le_cmp(&cfg->peer, BT_ADDR_LE_ANY)) {
return cfg;
}
}
@ -1729,7 +1735,7 @@ static u8_t notify_cb(const struct bt_gatt_attr *attr, void *user_data)
continue;
}
conn = bt_conn_lookup_state_le(&cfg->peer,
conn = bt_conn_lookup_state_le(cfg->id, &cfg->peer,
BT_CONN_CONNECTED);
if (!conn) {
struct sc_data *sc;
@ -1740,6 +1746,7 @@ static u8_t notify_cb(const struct bt_gatt_attr *attr, void *user_data)
sys_le16_to_cpu(sc->end));
continue;
}
bt_conn_unref(conn);
}
}
@ -2054,9 +2061,11 @@ static u8_t update_ccc(const struct bt_gatt_attr *attr, void *user_data)
ccc = attr->user_data;
for (i = 0; i < ARRAY_SIZE(ccc->cfg); i++) {
struct bt_gatt_ccc_cfg *cfg = &ccc->cfg[i];
/* Ignore configuration for different peer or not active */
if (!ccc->cfg[i].value ||
bt_conn_addr_le_cmp(conn, &ccc->cfg[i].peer)) {
if (!cfg->value ||
!bt_conn_is_peer_addr_le(conn, cfg->id, &cfg->peer)) {
continue;
}
@ -2129,8 +2138,7 @@ static u8_t disconnected_cb(const struct bt_gatt_attr *attr, void *user_data)
continue;
}
if (conn->id != cfg->id ||
bt_conn_addr_le_cmp(conn, &cfg->peer)) {
if (!bt_conn_is_peer_addr_le(conn, cfg->id, &cfg->peer)) {
struct bt_conn *tmp;
/* Skip if there is another peer connected */
@ -2209,8 +2217,9 @@ bool bt_gatt_is_subscribed(struct bt_conn *conn,
/* Check if the connection is subscribed */
for (size_t i = 0; i < BT_GATT_CCC_MAX; i++) {
if (conn->id == ccc->cfg[i].id &&
!bt_conn_addr_le_cmp(conn, &ccc->cfg[i].peer) &&
const struct bt_gatt_ccc_cfg *cfg = &ccc->cfg[i];
if (bt_conn_is_peer_addr_le(conn, cfg->id, &cfg->peer) &&
(ccc_value & ccc->cfg[i].value)) {
return true;
}
@ -2249,7 +2258,7 @@ static struct gatt_sub *gatt_sub_find_free(struct bt_conn *conn,
for (i = 0; i < ARRAY_SIZE(subscriptions); i++) {
struct gatt_sub *sub = &subscriptions[i];
if (!bt_conn_addr_le_cmp(conn, &sub->peer)) {
if (bt_conn_is_peer_addr_le(conn, sub->id, &sub->peer)) {
return sub;
} else if (free_sub &&
!bt_addr_le_cmp(BT_ADDR_LE_ANY, &sub->peer)) {
@ -2274,6 +2283,7 @@ static struct gatt_sub *gatt_sub_add(struct bt_conn *conn)
if (free_sub) {
bt_addr_le_copy(&free_sub->peer, &conn->le.dst);
free_sub->id = conn->id;
return free_sub;
}
@ -4076,13 +4086,15 @@ void bt_gatt_disconnected(struct bt_conn *conn)
#endif
}
static struct gatt_cf_cfg *find_cf_cfg_by_addr(const bt_addr_le_t *addr)
static struct gatt_cf_cfg *find_cf_cfg_by_addr(u8_t id,
const bt_addr_le_t *addr)
{
if (IS_ENABLED(CONFIG_BT_GATT_CACHING)) {
int i;
for (i = 0; i < ARRAY_SIZE(cf_cfg); i++) {
if (!bt_addr_le_cmp(addr, &cf_cfg[i].peer)) {
if (id == cf_cfg[i].id &&
!bt_addr_le_cmp(addr, &cf_cfg[i].peer)) {
return &cf_cfg[i];
}
}
@ -4262,7 +4274,9 @@ static int cf_set(const char *name, size_t len_rd, settings_read_cb read_cb,
{
struct gatt_cf_cfg *cfg;
bt_addr_le_t addr;
const char *next;
int len, err;
u8_t id;
if (!name) {
BT_ERR("Insufficient number of arguments");
@ -4275,7 +4289,15 @@ static int cf_set(const char *name, size_t len_rd, settings_read_cb read_cb,
return -EINVAL;
}
cfg = find_cf_cfg_by_addr(&addr);
settings_name_next(name, &next);
if (!next) {
id = BT_ID_DEFAULT;
} else {
id = strtol(next, NULL, 10);
}
cfg = find_cf_cfg_by_addr(id, &addr);
if (!cfg) {
cfg = find_cf_cfg(NULL);
if (!cfg) {
@ -4417,7 +4439,7 @@ static int bt_gatt_clear_cf(u8_t id, const bt_addr_le_t *addr)
{
struct gatt_cf_cfg *cfg;
cfg = find_cf_cfg_by_addr(addr);
cfg = find_cf_cfg_by_addr(id, addr);
if (cfg) {
clear_cf_cfg(cfg);
}
@ -4456,20 +4478,28 @@ static int sc_clear_by_addr(u8_t id, const bt_addr_le_t *addr)
return 0;
}
static void bt_gatt_clear_subscriptions(const bt_addr_le_t *addr)
static struct gatt_sub *find_gatt_sub(u8_t id, const bt_addr_le_t *addr)
{
struct gatt_sub *sub = NULL;
for (int i = 0; i < ARRAY_SIZE(subscriptions); i++) {
struct gatt_sub *sub = &subscriptions[i];
if (id == sub->id &&
!bt_addr_le_cmp(addr, &sub->peer)) {
return sub;
}
}
return NULL;
}
static void bt_gatt_clear_subscriptions(u8_t id, const bt_addr_le_t *addr)
{
struct gatt_sub *sub;
struct bt_gatt_subscribe_params *params, *tmp;
sys_snode_t *prev = NULL;
int i;
for (i = 0; i < ARRAY_SIZE(subscriptions); i++) {
if (!bt_addr_le_cmp(addr, &subscriptions[i].peer)) {
sub = &subscriptions[i];
break;
}
}
sub = find_gatt_sub(id, addr);
if (!sub) {
return;
}
@ -4505,7 +4535,7 @@ int bt_gatt_clear(u8_t id, const bt_addr_le_t *addr)
}
if (IS_ENABLED(CONFIG_BT_GATT_CLIENT)) {
bt_gatt_clear_subscriptions(addr);
bt_gatt_clear_subscriptions(id, addr);
}
return 0;

View file

@ -596,7 +596,8 @@ static void rpa_timeout(struct k_work *work)
if (IS_ENABLED(CONFIG_BT_CENTRAL)) {
struct bt_conn *conn =
bt_conn_lookup_state_le(NULL, BT_CONN_CONNECT_SCAN);
bt_conn_lookup_state_le(BT_ID_DEFAULT, NULL,
BT_CONN_CONNECT_SCAN);
if (conn) {
bt_conn_unref(conn);
@ -1215,9 +1216,11 @@ static struct bt_conn *find_pending_connect(u8_t role, bt_addr_le_t *peer_addr)
* CONNECT or DIR_ADV state associated with passed peer LE address.
*/
if (IS_ENABLED(CONFIG_BT_CENTRAL) && role == BT_HCI_ROLE_MASTER) {
conn = bt_conn_lookup_state_le(peer_addr, BT_CONN_CONNECT);
conn = bt_conn_lookup_state_le(BT_ID_DEFAULT, peer_addr,
BT_CONN_CONNECT);
if (IS_ENABLED(CONFIG_BT_WHITELIST) && !conn) {
conn = bt_conn_lookup_state_le(BT_ADDR_LE_NONE,
conn = bt_conn_lookup_state_le(BT_ID_DEFAULT,
BT_ADDR_LE_NONE,
BT_CONN_CONNECT_AUTO);
}
@ -1225,10 +1228,11 @@ static struct bt_conn *find_pending_connect(u8_t role, bt_addr_le_t *peer_addr)
}
if (IS_ENABLED(CONFIG_BT_PERIPHERAL) && role == BT_HCI_ROLE_SLAVE) {
conn = bt_conn_lookup_state_le(peer_addr,
conn = bt_conn_lookup_state_le(bt_dev.adv_id, peer_addr,
BT_CONN_CONNECT_DIR_ADV);
if (!conn) {
conn = bt_conn_lookup_state_le(BT_ADDR_LE_NONE,
conn = bt_conn_lookup_state_le(bt_dev.adv_id,
BT_ADDR_LE_NONE,
BT_CONN_CONNECT_ADV);
}
@ -1761,7 +1765,8 @@ static void check_pending_conn(const bt_addr_le_t *id_addr,
return;
}
conn = bt_conn_lookup_state_le(id_addr, BT_CONN_CONNECT_SCAN);
conn = bt_conn_lookup_state_le(BT_ID_DEFAULT, id_addr,
BT_CONN_CONNECT_SCAN);
if (!conn) {
return;
}
@ -3083,7 +3088,7 @@ void bt_id_add(struct bt_keys *keys)
return;
}
conn = bt_conn_lookup_state_le(NULL, BT_CONN_CONNECT);
conn = bt_conn_lookup_state_le(BT_ID_DEFAULT, NULL, BT_CONN_CONNECT);
if (conn) {
atomic_set_bit(bt_dev.flags, BT_DEV_ID_PENDING);
keys->flags |= BT_KEYS_ID_PENDING_ADD;
@ -3190,7 +3195,7 @@ void bt_id_del(struct bt_keys *keys)
return;
}
conn = bt_conn_lookup_state_le(NULL, BT_CONN_CONNECT);
conn = bt_conn_lookup_state_le(BT_ID_DEFAULT, NULL, BT_CONN_CONNECT);
if (conn) {
atomic_set_bit(bt_dev.flags, BT_DEV_ID_PENDING);
keys->flags |= BT_KEYS_ID_PENDING_DEL;
@ -3736,13 +3741,15 @@ int bt_le_scan_update(bool fast_scan)
struct bt_conn *conn;
/* don't restart scan if we have pending connection */
conn = bt_conn_lookup_state_le(NULL, BT_CONN_CONNECT);
conn = bt_conn_lookup_state_le(BT_ID_DEFAULT, NULL,
BT_CONN_CONNECT);
if (conn) {
bt_conn_unref(conn);
return 0;
}
conn = bt_conn_lookup_state_le(NULL, BT_CONN_CONNECT_SCAN);
conn = bt_conn_lookup_state_le(BT_ID_DEFAULT, NULL,
BT_CONN_CONNECT_SCAN);
if (!conn) {
return 0;
}
@ -6192,14 +6199,15 @@ int bt_le_adv_stop(void)
if (IS_ENABLED(CONFIG_BT_PERIPHERAL)) {
struct bt_conn *conn;
conn = bt_conn_lookup_state_le(BT_ADDR_LE_NONE,
conn = bt_conn_lookup_state_le(bt_dev.adv_id, BT_ADDR_LE_NONE,
BT_CONN_CONNECT_ADV);
if (conn) {
bt_conn_set_state(conn, BT_CONN_DISCONNECTED);
bt_conn_unref(conn);
}
conn = bt_conn_lookup_state_le(NULL, BT_CONN_CONNECT_DIR_ADV);
conn = bt_conn_lookup_state_le(bt_dev.adv_id, NULL,
BT_CONN_CONNECT_DIR_ADV);
if (conn) {
bt_conn_set_state(conn, BT_CONN_DISCONNECTED);
bt_conn_unref(conn);