From 8f99c0d6d64c4db7bcd0d3cbfd173b52bd3808f1 Mon Sep 17 00:00:00 2001 From: Sumeet Singh Date: Thu, 7 Nov 2024 17:05:50 +0530 Subject: [PATCH] fix(nimble): Added 1. Option to disable automatic discovery when receiving out-of-sync 2. Fixed robust caching aware state parity 3. Fixed database hash related bugs and memory leaks --- nimble/host/include/host/ble_gatt.h | 3 ++ nimble/host/src/ble_att_svr.c | 11 ++---- nimble/host/src/ble_gap.c | 12 ++++++ nimble/host/src/ble_gattc_cache.c | 23 ++++++++++-- nimble/host/src/ble_gattc_cache_conn.c | 52 ++++++++++++++++++-------- nimble/host/src/ble_gatts.c | 12 ++++-- 6 files changed, 83 insertions(+), 30 deletions(-) diff --git a/nimble/host/include/host/ble_gatt.h b/nimble/host/include/host/ble_gatt.h index f361516e9..06d021782 100644 --- a/nimble/host/include/host/ble_gatt.h +++ b/nimble/host/include/host/ble_gatt.h @@ -28,6 +28,7 @@ */ #include +#include "nimble/ble.h" #include "host/ble_att.h" #include "host/ble_uuid.h" #include "host/ble_esp_gatt.h" @@ -954,6 +955,8 @@ int ble_gatts_indicate(uint16_t conn_handle, uint16_t chr_val_handle); */ int ble_gattc_indicate(uint16_t conn_handle, uint16_t chr_val_handle); +void ble_gattc_cache_conn_undisc_all(ble_addr_t peer_addr); + /** Initialize the BLE GATT client. */ int ble_gattc_init(void); diff --git a/nimble/host/src/ble_att_svr.c b/nimble/host/src/ble_att_svr.c index b5d304fdb..b423991fe 100644 --- a/nimble/host/src/ble_att_svr.c +++ b/nimble/host/src/ble_att_svr.c @@ -1526,9 +1526,10 @@ ble_att_svr_rx_read_type(uint16_t conn_handle, uint16_t cid, struct os_mbuf **rx #if MYNEWT_VAL(BLE_GATT_CACHING) ble_hs_lock(); - if(start_handle == 0x0001 && end_handle == 0xFFFF && uuid.u.type == BLE_UUID_TYPE_16 && - (uuid.u16.value == BLE_ATT_UUID_INCLUDE || uuid.u16.value == BLE_ATT_UUID_CHARACTERISTIC) - ) { + if (uuid.u.type == BLE_UUID_TYPE_16 && ( + uuid.u16.value == BLE_ATT_UUID_INCLUDE || + uuid.u16.value == BLE_ATT_UUID_CHARACTERISTIC || + uuid.u16.value == BLE_SVC_GATT_CHR_DATABASE_HASH_UUID16)) { int i = 0; conn = ble_hs_conn_find_assert(conn_handle); @@ -1543,10 +1544,6 @@ ble_att_svr_rx_read_type(uint16_t conn_handle, uint16_t cid, struct os_mbuf **rx ble_gatts_conn_aware_states[i].half_aware = 0; } } - } else if (uuid.u.type == BLE_UUID_TYPE_16 && uuid.u16.value == BLE_SVC_GATT_CHR_DATABASE_HASH_UUID16) { - if (!ble_att_svr_check_conn_aware(conn_handle)) { - ble_att_svr_make_conn_aware(conn_handle); - } } else { if((ble_att_svr_get_csfs(conn_handle)[0] & 1) && ble_svc_gatt_csf_handle() != err_handle ) { diff --git a/nimble/host/src/ble_gap.c b/nimble/host/src/ble_gap.c index 645bc21e5..558f7a5a4 100644 --- a/nimble/host/src/ble_gap.c +++ b/nimble/host/src/ble_gap.c @@ -2478,6 +2478,18 @@ ble_gap_rx_conn_complete(struct ble_gap_conn_complete *evt, uint8_t instance) if (evt->role == BLE_HCI_LE_CONN_COMPLETE_ROLE_MASTER) { ble_hs_conn_addrs(conn, &addrs); rc = ble_gattc_cache_conn_create(conn->bhc_handle, addrs.peer_id_addr); + } else { + conn->bhc_gatt_svr.aware_state = true; + conn->bhc_gatt_svr.half_aware = 0; + /* This is also done when bonding is restored, so `conn` and `ble_gatts_conn_aware_states` need to be kept in sync */ + ble_hs_conn_addrs(conn, &addrs); + for (int i = 0; i < MYNEWT_VAL(BLE_STORE_MAX_BONDS); i++) { + if (memcmp(ble_gatts_conn_aware_states[i].peer_id_addr, + addrs.peer_id_addr.val, sizeof addrs.peer_id_addr.val)) { + conn->bhc_gatt_svr.half_aware = ble_gatts_conn_aware_states[i].half_aware; + conn->bhc_gatt_svr.aware_state = ble_gatts_conn_aware_states[i].aware; + } + } } #endif ble_gap_event_listener_call(&event); diff --git a/nimble/host/src/ble_gattc_cache.c b/nimble/host/src/ble_gattc_cache.c index 7f0bf3108..33c09bb2e 100644 --- a/nimble/host/src/ble_gattc_cache.c +++ b/nimble/host/src/ble_gattc_cache.c @@ -388,6 +388,7 @@ ble_gattc_cache_addr_save(uint8_t *out_index, ble_addr_t addr, uint8_t * hash_ke address list. */ if(num > MYNEWT_VAL(BLE_GATT_CACHING_MAX_CONNS)) { + nimble_platform_mem_free(p_buf); return BLE_HS_ENOMEM; } BLE_HS_LOG(DEBUG, "BD addr not present"); @@ -396,6 +397,7 @@ ble_gattc_cache_addr_save(uint8_t *out_index, ble_addr_t addr, uint8_t * hash_ke } else { BLE_HS_LOG(DEBUG, "Hash key not present, saving new data"); if(num > MYNEWT_VAL(BLE_GATT_CACHING_MAX_CONNS)) { + nimble_platform_mem_free(p_buf); return BLE_HS_ENOMEM; } insert_ind = num - 1; @@ -485,6 +487,7 @@ ble_gattc_cache_save(struct ble_gattc_cache_conn *peer, size_t num_attr) if(rc != 0) { /* cannot save address, return */ BLE_HS_LOG(ERROR, "Failed to save cache %d", rc); + nimble_platform_mem_free(nv_attr); return; } @@ -530,6 +533,7 @@ static int ble_gattc_add_svc_from_cache(ble_addr_t peer_addr, struct ble_gatt_nv_attr nv_attr) { struct ble_gatt_svc *gatt_svc; + int rc; gatt_svc = (struct ble_gatt_svc *)nimble_platform_mem_malloc(sizeof(struct ble_gatt_svc)); if (gatt_svc == NULL) { @@ -540,13 +544,16 @@ ble_gattc_add_svc_from_cache(ble_addr_t peer_addr, struct ble_gatt_nv_attr nv_at gatt_svc->end_handle = nv_attr.e_handle; ble_uuid_copy(&gatt_svc->uuid, &nv_attr.uuid.u); - return ble_gattc_cache_conn_svc_add(peer_addr, gatt_svc); + rc = ble_gattc_cache_conn_svc_add(peer_addr, gatt_svc); + nimble_platform_mem_free(gatt_svc); + return rc; } static int ble_gattc_add_inc_from_cache(ble_addr_t peer_addr, struct ble_gatt_nv_attr nv_attr) { struct ble_gatt_svc *gatt_svc; + int rc; gatt_svc = (struct ble_gatt_svc *)nimble_platform_mem_malloc(sizeof(struct ble_gatt_svc)); if (gatt_svc == NULL) { @@ -556,13 +563,16 @@ ble_gattc_add_inc_from_cache(ble_addr_t peer_addr, struct ble_gatt_nv_attr nv_at gatt_svc->start_handle = nv_attr.s_handle; gatt_svc->end_handle = nv_attr.e_handle; ble_uuid_copy(&gatt_svc->uuid, &nv_attr.uuid.u); - return ble_gattc_cache_conn_inc_add(peer_addr, gatt_svc); + rc = ble_gattc_cache_conn_inc_add(peer_addr, gatt_svc); + nimble_platform_mem_free(gatt_svc); + return rc; } static int ble_gattc_add_chr_from_cache(ble_addr_t peer_addr, struct ble_gatt_nv_attr nv_attr) { struct ble_gatt_chr *gatt_chr; + int rc; gatt_chr = (struct ble_gatt_chr *)nimble_platform_mem_malloc(sizeof(struct ble_gatt_chr)); if (gatt_chr == NULL) { return BLE_HS_ENOMEM; @@ -573,13 +583,16 @@ ble_gattc_add_chr_from_cache(ble_addr_t peer_addr, struct ble_gatt_nv_attr nv_at ble_uuid_copy(&gatt_chr->uuid, &nv_attr.uuid.u); gatt_chr->properties = nv_attr.properties; - return ble_gattc_cache_conn_chr_add(peer_addr, 0, gatt_chr); + rc = ble_gattc_cache_conn_chr_add(peer_addr, 0, gatt_chr); + nimble_platform_mem_free(gatt_chr); + return rc; } static int ble_gattc_add_dsc_from_cache(ble_addr_t peer_addr, struct ble_gatt_nv_attr nv_attr) { struct ble_gatt_dsc *gatt_dsc; + int rc; gatt_dsc = (struct ble_gatt_dsc *)nimble_platform_mem_malloc(sizeof(struct ble_gatt_dsc)); if (gatt_dsc == NULL) { return BLE_HS_ENOMEM; @@ -588,7 +601,9 @@ ble_gattc_add_dsc_from_cache(ble_addr_t peer_addr, struct ble_gatt_nv_attr nv_at gatt_dsc->handle = nv_attr.s_handle; ble_uuid_copy(&gatt_dsc->uuid, &nv_attr.uuid.u); - return ble_gattc_cache_conn_dsc_add(peer_addr, 0, gatt_dsc); + rc = ble_gattc_cache_conn_dsc_add(peer_addr, 0, gatt_dsc); + nimble_platform_mem_free(gatt_dsc); + return rc; } int diff --git a/nimble/host/src/ble_gattc_cache_conn.c b/nimble/host/src/ble_gattc_cache_conn.c index 8ec8fdaa9..5db4af433 100644 --- a/nimble/host/src/ble_gattc_cache_conn.c +++ b/nimble/host/src/ble_gattc_cache_conn.c @@ -718,6 +718,7 @@ ble_gattc_cache_conn_bonding_established(uint16_t conn_handle) BLE_HS_DBG_ASSERT(conn != NULL); ble_hs_conn_addrs(conn, &addrs); peer->ble_gattc_cache_conn_addr = conn->bhc_peer_addr; + peer->ble_gattc_cache_conn_addr.type = ble_hs_misc_peer_addr_type_to_id(conn->bhc_peer_addr.type); @@ -811,6 +812,7 @@ ble_gattc_cache_conn_disc_complete(struct ble_gattc_cache_conn *peer, int rc) BLE_UUID16_DECLARE(BLE_SVC_GATT_CHR_DATABASE_HASH_UUID16)); if (bonded || chr != NULL) { /* persist the cache */ + ble_gattc_cacheReset(&hs_conn->bhc_peer_addr); ble_gattc_cache_conn_cache_peer(peer); /* TODO */ } } else { @@ -860,9 +862,15 @@ ble_gattc_cache_conn_disc_complete(struct ble_gattc_cache_conn *peer, int rc) } } -static void -ble_gattc_cache_conn_undisc_all(struct ble_gattc_cache_conn *peer) +void +ble_gattc_cache_conn_undisc_all(ble_addr_t peer_addr) { + struct ble_gattc_cache_conn * peer = NULL; + + peer = ble_gattc_cache_conn_find_by_addr(peer_addr); + if (peer == NULL) { + return; + } ble_gattc_cacheReset(&peer->ble_gattc_cache_conn_addr); struct ble_gattc_cache_conn_svc *svc; @@ -946,7 +954,7 @@ ble_gattc_cache_conn_disc(struct ble_gattc_cache_conn *peer) { int rc; - ble_gattc_cache_conn_undisc_all(peer); + ble_gattc_cache_conn_undisc_all(peer->ble_gattc_cache_conn_addr); peer->disc_prev_chr_val = 1; @@ -964,6 +972,11 @@ ble_gattc_cache_conn_on_read(uint16_t conn_handle, { uint16_t res; + if (error->status == BLE_HS_EDONE) { + /* Ignore Read by UUID follow-up callback */ + return 0; + } + if (error->status != 0) { res = error->status; ble_gattc_cache_conn_disc_complete((struct ble_gattc_cache_conn *)arg, res); @@ -975,9 +988,10 @@ ble_gattc_cache_conn_on_read(uint16_t conn_handle, BLE_HS_LOG(INFO, "Hash value up to date, skipping Discovery"); ble_gattc_cache_conn_disc_complete((struct ble_gattc_cache_conn *)arg, res); return 0; + } else { + res = ble_gattc_cache_conn_disc((struct ble_gattc_cache_conn *)arg); + return res; } - return 0; - } int @@ -1226,6 +1240,10 @@ ble_gattc_cache_conn_update(uint16_t conn_handle, uint16_t start_handle, uint16_ } peer->cache_state = CACHE_INVALID; + if (MYNEWT_VAL(BLE_GATT_CACHING_DISABLE_AUTO)) { + /* Do not automatically re-discover and correct cache */ + return; + } rc = ble_gattc_cache_conn_disc(peer); if (rc != 0) { peer->cache_state = CACHE_INVALID; @@ -1385,7 +1403,6 @@ ble_gattc_cache_error(int status, uint16_t att_handle) static int ble_gattc_cache_conn_verify(struct ble_gattc_cache_conn *conn) { struct ble_hs_conn *gap_conn; - const struct ble_gattc_cache_conn_chr *chr; int rc; if (conn->cache_state == CACHE_VERIFIED) { @@ -1404,16 +1421,10 @@ static int ble_gattc_cache_conn_verify(struct ble_gattc_cache_conn *conn) conn->cache_state = CACHE_VERIFIED; return 0; } - chr = ble_gattc_cache_conn_chr_find_uuid(conn, - BLE_UUID16_DECLARE(BLE_GATT_SVC_UUID16), - BLE_UUID16_DECLARE(BLE_SVC_GATT_CHR_DATABASE_HASH_UUID16)); - if (chr == NULL) { - /* no way to verify */ - conn->cache_state = CACHE_INVALID; - return 0; - } - rc = ble_gattc_read(conn->conn_handle, chr->chr.val_handle, - ble_gattc_cache_conn_on_read, conn); + + rc = ble_gattc_read_by_uuid(conn->conn_handle, 0x0001, 0xFFFF, + BLE_UUID16_DECLARE(BLE_SVC_GATT_CHR_DATABASE_HASH_UUID16), + ble_gattc_cache_conn_on_read, conn); if (rc != 0) { /* no way to verify */ conn->cache_state = CACHE_INVALID; @@ -1449,6 +1460,8 @@ static void ble_gattc_cache_search_all_svcs_cb(struct ble_npl_event *ev) } status = BLE_HS_EDONE; dcb(conn->conn_handle, ble_gattc_cache_error(status, 0), &svc->svc, op->cb_arg); + + ble_npl_event_deinit(&conn->disc_ev); return; } @@ -1522,6 +1535,7 @@ ble_gattc_cache_conn_search_svc_by_uuid_cb(struct ble_npl_event *ev) } status = BLE_HS_EDONE; dcb(conn_handle, ble_gattc_cache_error(status, 0), &svc->svc, op->cb_arg); + ble_npl_event_deinit(&conn->disc_ev); return; } @@ -1580,6 +1594,7 @@ ble_gattc_cache_conn_search_inc_svcs_cb(struct ble_npl_event *ev) } status = BLE_HS_EDONE; dcb(conn->conn_handle, ble_gattc_cache_error(status, 0), &svc->svc, op->cb_arg); + ble_npl_event_deinit(&conn->disc_ev); return; } int @@ -1638,6 +1653,8 @@ ble_gattc_cache_conn_search_all_chrs_cb(struct ble_npl_event *ev) } status = BLE_HS_EDONE; dcb(conn_handle, ble_gattc_cache_error(status, 0), &chr->chr, op->cb_arg); + + ble_npl_event_deinit(&conn->disc_ev); return; } @@ -1699,6 +1716,8 @@ ble_gattc_cache_conn_search_chrs_by_uuid_cb(struct ble_npl_event *ev) } status = BLE_HS_EDONE; dcb(conn_handle, ble_gattc_cache_error(status, 0), &chr->chr, op->cb_arg); + + ble_npl_event_deinit(&conn->disc_ev); return; } @@ -1758,6 +1777,7 @@ ble_gattc_cache_conn_search_all_dscs_cb(struct ble_npl_event *ev) } status = BLE_HS_EDONE; dcb(conn_handle, ble_gattc_cache_error(status, 0), chr->chr.val_handle, &dsc->dsc, op->cb_arg); + ble_npl_event_deinit(&conn->disc_ev); return; } diff --git a/nimble/host/src/ble_gatts.c b/nimble/host/src/ble_gatts.c index a2b93e0e1..7c7f9fa8d 100644 --- a/nimble/host/src/ble_gatts.c +++ b/nimble/host/src/ble_gatts.c @@ -613,16 +613,20 @@ ble_gatts_calculate_hash(uint8_t *out_hash_key) rc = ble_att_fill_database_info(buf); if(rc != 0) { - return rc; + goto done; } rc = ble_sm_alg_aes_cmac(key, buf, size, out_hash_key); if(rc != 0) { - return rc; + goto done; } swap_in_place(out_hash_key, 16); - return 0; + + rc = 0; +done: + nimble_platform_mem_free(buf); + return rc; } #endif @@ -2449,6 +2453,8 @@ ble_gatts_bonding_established(uint16_t conn_handle) sizeof(struct ble_gatts_aware_state)); memcpy(ble_gatts_conn_aware_states[new_idx].peer_id_addr, addrs.peer_id_addr.val, sizeof addrs.peer_id_addr.val); + ble_gatts_conn_aware_states[new_idx].aware = conn->bhc_gatt_svr.aware_state; + ble_gatts_conn_aware_states[new_idx].half_aware = conn->bhc_gatt_svr.half_aware; last_conn_aware_state_index = new_idx; #endif ble_hs_unlock();