fix(nimble): Fix unpair_oldest_peer deleting wrong device

Sort bond entries by bond_count after in-place updates to maintain correct
eviction order, and log IRK resolving-list failures instead of failing the bond.
This commit is contained in:
Rahul Tank
2026-08-18 12:42:58 +05:30
parent 3e30c9f70b
commit 83794b7c5f
7 changed files with 893 additions and 213 deletions
+24 -26
View File
@@ -2401,13 +2401,13 @@ ble_gap_conn_broken(uint16_t conn_handle, int reason)
}
}
ble_hs_unlock();
post_connect_fail = conn != NULL && !send &&
!(conn->bhc_flags & BLE_HS_CONN_F_MASTER);
ble_hs_unlock();
/* If we never posted a connect event for a slave (send == 0), treat this as a
* connection failure and post a connect-failed event instead of a
/* If we never posted a connect event for a slave (send == 0), treat this as
* a connection failure and post a connect-failed event instead of a
* disconnect. This allows applications to restart advertising.
*/
if (post_connect_fail) {
@@ -10630,11 +10630,6 @@ ble_gap_unpair(const ble_addr_t *peer_addr)
return BLE_HS_EINVAL;
}
/* Resolving-list removal is handled at the actual IRK removal site:
* controller privacy preempts GAP procedures, while host privacy updates
* the software resolving list directly.
*/
ble_hs_lock();
conn = ble_hs_conn_find_by_addr(peer_addr);
@@ -10663,11 +10658,15 @@ ble_gap_unpair(const ble_addr_t *peer_addr)
if (!rc) {
new_addr = &(value.rpa_rec.peer_addr);
#if MYNEWT_VAL(BLE_HOST_BASED_PRIVACY)
/* Host privacy may lack peer_dev_rec while the identity still sits on
* the software RL (e.g. partial bond). Remove by resolved identity
* here so delete_peer cannot miss it.
*/
ble_hs_pvcy_remove_entry(new_addr->type, new_addr->val);
#endif
} else {
#if MYNEWT_VAL(BLE_HOST_BASED_PRIVACY)
/* If no rpa_rec, remove original addr directly */
/* No rpa_rec remove using the address the app passed. */
ble_hs_pvcy_remove_entry(peer_addr->type, peer_addr->val);
#endif
}
@@ -10677,7 +10676,9 @@ ble_gap_unpair(const ble_addr_t *peer_addr)
rc = ble_store_read(BLE_STORE_OBJ_TYPE_PEER_SEC, &key, &value);
// Checking if the device is in ble_store
/* Prefer peer_sec for IRK removal; fall back to our_sec existence check.
* Always attempt delete_peer so partial/asymmetric bond records are cleared.
*/
if (!rc) {
if (value.sec.irk_present) {
#if MYNEWT_VAL(BLE_HS_PVCY)
@@ -10708,23 +10709,20 @@ ble_gap_unpair(const ble_addr_t *peer_addr)
}
#endif
}
// Delete the Peer record from store as LTK is present
rc = ble_store_util_delete_peer(&key.sec.peer_addr);
if (rc != 0) {
BLE_HS_LOG(ERROR, "Error while removing LTK , rc = %x\n",rc);
err = rc;
}
} else {
rc = ble_store_read(BLE_STORE_OBJ_TYPE_OUR_SEC, &key, &value);
if (!rc) {
rc = ble_store_util_delete_peer(&key.sec.peer_addr);
if (rc != 0) {
err = rc;
}
} else {
BLE_HS_LOG(ERROR,"No record found for the given address in ble store , rc = %x\n",rc);
err = rc ;
if (rc != 0) {
BLE_HS_LOG(ERROR, "No record found for the given address in ble store, rc = %x\n",
rc);
err = rc;
}
}
rc = ble_store_util_delete_peer(&key.sec.peer_addr);
if (rc != 0) {
BLE_HS_LOG(ERROR, "Error while removing peer records , rc = %x\n", rc);
if (err == 0) {
err = rc;
}
}
+1
View File
@@ -37,6 +37,7 @@ typedef uint8_t ble_hs_conn_flags_t;
#define BLE_HS_CONN_F_MASTER 0x01
#define BLE_HS_CONN_F_TERMINATING 0x02
#define BLE_HS_CONN_F_TX_FRAG 0x04 /* Cur ACL packet partially txed. */
#define BLE_HS_CONN_F_CONN_FAIL_POSTED 0x08
#if MYNEWT_VAL(BLE_DEFER_CONN_EVENTS)
#include "host/ble_gap.h"
+6 -1
View File
@@ -1580,7 +1580,12 @@ ble_sm_ltk_restore_exec(struct ble_sm_proc *proc, struct ble_sm_result *res,
res->app_status = rc;
res->enc_cb = 1; /* Notify application of failure, similar to reply branch */
} else {
res->app_status = 0;
/* Neg-reply sent: do not treat as restore success. app_status=0
* would enter ENC_RESTORE and wait for an enc-change that never
* arrives. PINKEY_MISSING also drives BLE_RESTART_PAIR.
*/
res->app_status = BLE_HS_HCI_ERR(BLE_ERR_PINKEY_MISSING);
res->enc_cb = 1;
}
}
+69 -5
View File
@@ -207,6 +207,44 @@ ble_store_persist_sec(int obj_type,
rc = ble_store_write(obj_type, store_value);
return rc;
}
#if MYNEWT_VAL(BLE_HS_PVCY) && MYNEWT_VAL(BLE_SM_SC)
struct ble_store_pvcy_irk_cleanup_entry {
const uint8_t *irk;
ble_addr_t peer_addr;
};
static int
ble_store_remove_stale_pvcy_entry_cb(int obj_type,
union ble_store_value *val,
void *arg)
{
struct ble_store_pvcy_irk_cleanup_entry *cleanup = arg;
BLE_HS_DBG_ASSERT(obj_type == BLE_STORE_OBJ_TYPE_PEER_SEC);
/* Current peer is handled by add/replace below; only purge other peers
* that still hold this IRK in the controller resolving list.
*/
if (ble_addr_cmp(&val->sec.peer_addr, &cleanup->peer_addr) == 0) {
return 0;
}
if (!val->sec.irk_present ||
memcmp(val->sec.irk, cleanup->irk, sizeof(val->sec.irk)) != 0) {
return 0;
}
#if !MYNEWT_VAL(BLE_HOST_BASED_PRIVACY)
ble_gap_preempt();
#endif
ble_hs_pvcy_remove_entry(val->sec.peer_addr.type, val->sec.peer_addr.val);
#if !MYNEWT_VAL(BLE_HOST_BASED_PRIVACY)
ble_gap_preempt_done();
#endif
return 0;
}
#endif
#endif
int
@@ -331,9 +369,36 @@ ble_store_write_peer_sec(const struct ble_store_value_sec *value_sec)
}
ble_hs_unlock();
#endif
#endif /* MYNEWT_VAL(BLE_DEFER_CONN_EVENTS) */
/* Write the peer IRK to the controller keycache
* There is not much to do here if it fails */
#if MYNEWT_VAL(BLE_SM_SC)
/* Drop stale resolving-list entries that share this IRK (or this peer)
* before programming the controller. Skip when the add was deferred
* above (already returned). */
{
struct ble_store_pvcy_irk_cleanup_entry cleanup = {
.irk = value_sec->irk,
.peer_addr = value_sec->peer_addr,
};
int rc_iter;
rc_iter = ble_store_iterate(BLE_STORE_OBJ_TYPE_PEER_SEC,
ble_store_remove_stale_pvcy_entry_cb,
&cleanup);
if (rc_iter != 0) {
BLE_HS_LOG(WARN, "failed to clean stale IRK entries for peer; rc=%d\n",
rc_iter);
}
}
#endif
/* Write the peer IRK to the controller keycache.
* The bond is already persisted at this point, so if programming the
* controller resolving list fails (e.g. error 0x07 - Memory Capacity
* Exceeded), log the failure but do not fail the bond. This matches
* the best-effort handling in ble_hs_misc_restore_one_irk().
*/
#if MYNEWT_VAL(BLE_DEFER_CONN_EVENTS)
if (replace_entry) {
rc = ble_hs_pvcy_replace_entry(value_sec->peer_addr.val,
value_sec->peer_addr.type,
@@ -355,14 +420,13 @@ ble_store_write_peer_sec(const struct ble_store_value_sec *value_sec)
}
}
#else
/* Write the peer IRK to the controller keycache
* There is not much to do here if it fails */
rc = ble_hs_pvcy_add_entry(value_sec->peer_addr.val,
value_sec->peer_addr.type,
value_sec->irk);
#endif /* MYNEWT_VAL(BLE_DEFER_CONN_EVENTS) */
if (rc != 0) {
return rc;
BLE_HS_LOG(ERROR, "%s pvcy_add_entry failed; rc=%d\n",
__func__, rc);
}
#endif
}
+40 -21
View File
@@ -163,26 +163,23 @@ ble_store_util_delete_peer(const ble_addr_t *peer_id_addr)
union ble_store_key key;
int rc;
int err;
memset(&key, 0, sizeof key);
key.sec.peer_addr = *peer_id_addr;
rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_OUR_SEC, &key);
if (rc != 0) {
return rc;
}
err = 0;
rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_PEER_SEC, &key);
if (rc != 0) {
return rc;
if (rc != 0 && err == 0) {
err = rc;
}
memset(&key, 0, sizeof key);
key.cccd.peer_addr = *peer_id_addr;
rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
if (rc != 0) {
return rc;
if (rc != 0 && err == 0) {
err = rc;
}
#if MYNEWT_VAL(ENC_ADV_DATA)
@@ -190,8 +187,8 @@ ble_store_util_delete_peer(const ble_addr_t *peer_id_addr)
key.ead.peer_addr = *peer_id_addr;
rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_ENC_ADV_DATA, &key);
if (rc != 0) {
return rc;
if (rc != 0 && err == 0) {
err = rc;
}
#endif
@@ -199,16 +196,27 @@ ble_store_util_delete_peer(const ble_addr_t *peer_id_addr)
key.rpa_rec.peer_rpa_addr = *peer_id_addr;
rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_PEER_ADDR, &key);
if (rc != 0) {
return rc;
if (rc != 0 && err == 0) {
err = rc;
}
memset(&key, 0, sizeof key);
key.csfc.peer_addr = *peer_id_addr;
rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CSFC, &key);
if (rc != 0) {
return rc;
if (rc != 0 && err == 0) {
err = rc;
}
memset(&key, 0, sizeof key);
key.sec.peer_addr = *peer_id_addr;
/* Delete our_sec last so a partial failure does not hide the bond from
* eviction logic that walks the our_sec list.
*/
rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_OUR_SEC, &key);
if (rc != 0 && err == 0) {
err = rc;
}
#if MYNEWT_VAL(BLE_HOST_BASED_PRIVACY)
@@ -230,11 +238,8 @@ ble_store_util_delete_peer(const ble_addr_t *peer_id_addr)
}
rc = ble_rpa_remove_peer_dev_rec(peer_rec);
if (rc != 0) {
if (needs_unlock) {
ble_hs_unlock();
}
return rc;
if (rc != 0 && err == 0) {
err = rc;
}
}
if (needs_unlock) {
@@ -242,7 +247,21 @@ ble_store_util_delete_peer(const ble_addr_t *peer_id_addr)
}
#endif
return 0;
#if MYNEWT_VAL(BLE_HS_PVCY) && !MYNEWT_VAL(BLE_HOST_BASED_PRIVACY)
/* Direct callers (delete_oldest_peer, repeat-pairing) may not go through
* ble_gap_unpair. Preempt so HCI remove is legal while advertising/scanning.
* Unpair already removes when irk_present (with defer-add skip); a second
* remove here is best-effort and may return an error — that is fine.
*/
ble_gap_preempt();
rc = ble_hs_pvcy_remove_entry(key.sec.peer_addr.type, key.sec.peer_addr.val);
ble_gap_preempt_done();
if (rc != 0) {
BLE_HS_LOG(DEBUG, "Peer Device was not removed from RL \n");
}
#endif
return err;
#else
return BLE_HS_ENOTSUP;
#endif
@@ -309,6 +309,10 @@ ble_store_config_write_our_sec(const struct ble_store_value_sec *value_sec)
ble_store_config_our_secs[idx].bond_count = ++ble_store_config_our_bond_count;
/* Ensure entries are sorted at all times */
qsort(ble_store_config_our_secs, ble_store_config_num_our_secs,
sizeof(struct ble_store_value_sec), ble_store_config_compare_bond_count);
rc = ble_store_config_persist_our_secs();
if (rc != 0) {
return rc;
@@ -465,6 +469,10 @@ ble_store_config_write_peer_sec(const struct ble_store_value_sec *value_sec)
ble_store_config_peer_secs[idx].bond_count = ++ble_store_config_peer_bond_count;
/* Ensure entries are sorted at all times */
qsort(ble_store_config_peer_secs, ble_store_config_num_peer_secs,
sizeof(struct ble_store_value_sec), ble_store_config_compare_bond_count);
rc = ble_store_config_persist_peer_secs();
if (rc != 0) {
return rc;
@@ -881,6 +889,22 @@ ble_store_config_find_rpa_rec(const struct ble_store_key_rpa_rec *key)
}
return -1;
}
static int
ble_store_config_find_rpa_rec_by_peer_addr(const ble_addr_t *peer_addr)
{
struct ble_store_value_rpa_rec *rpa_rec;
int i;
for (i = 0; i < ble_store_config_num_rpa_recs; i++) {
rpa_rec = ble_store_config_rpa_recs + i;
if (ble_addr_cmp(&rpa_rec->peer_addr, peer_addr) == 0) {
return i;
}
}
return -1;
}
#endif
static int
@@ -907,7 +931,10 @@ ble_store_config_write_rpa_rec(const struct ble_store_value_rpa_rec *value_rpa_r
int idx;
int rc;
ble_store_key_from_value_rpa_rec(&key_rpa_rec, value_rpa_rec);
idx = ble_store_config_find_rpa_rec(&key_rpa_rec);
idx = ble_store_config_find_rpa_rec_by_peer_addr(&value_rpa_rec->peer_addr);
if (idx == -1) {
idx = ble_store_config_find_rpa_rec(&key_rpa_rec);
}
if (idx == -1) {
if (ble_store_config_num_rpa_recs >= MYNEWT_VAL(BLE_STORE_MAX_BONDS)) {
File diff suppressed because it is too large Load Diff