fix(nimble): stop GATTC temp-list crash on coex

STAILQ_REMOVE on emptied temp_proc_list faulted during
service discovery when BLE host preempted 802.15.4. Insert
the proc on the active list before ATT TX and defer free
while the public API still holds the pointer.
This commit is contained in:
Rahul Tank
2026-09-23 17:44:56 +05:30
parent b6e2f93db2
commit 544bb668ad
+126 -78
View File
@@ -112,6 +112,14 @@
/** Procedure stalled due to resource exhaustion. */
#define BLE_GATTC_PROC_F_STALLED 0x01
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
/** Procedure is still referenced by its public API caller. */
#define BLE_GATTC_PROC_F_API_LOCKED 0x02
/** Procedure free was deferred until its public API caller returns. */
#define BLE_GATTC_PROC_F_FREE_PENDING 0x04
#endif
/** Represents an in-progress GATT procedure. */
struct ble_gattc_proc {
STAILQ_ENTRY(ble_gattc_proc) next;
@@ -244,9 +252,6 @@ struct ble_gattc_proc {
};
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
static struct ble_gattc_proc_list temp_proc_list;
#endif
STAILQ_HEAD(ble_gattc_proc_list, ble_gattc_proc);
/**
@@ -872,6 +877,16 @@ ble_gattc_proc_free(struct ble_gattc_proc *proc)
int i;
if (proc != NULL) {
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
ble_hs_lock_nested();
if (proc->flags & BLE_GATTC_PROC_F_API_LOCKED) {
proc->flags |= BLE_GATTC_PROC_F_FREE_PENDING;
ble_hs_unlock_nested();
return;
}
ble_hs_unlock_nested();
#endif
ble_gattc_dbg_assert_proc_not_inserted(proc);
switch (proc->op) {
@@ -931,6 +946,35 @@ ble_gattc_proc_insert(struct ble_gattc_proc *proc)
ble_hs_unlock();
}
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
static int
ble_gattc_proc_remove_locked(struct ble_gattc_proc *proc)
{
struct ble_gattc_proc *cur;
struct ble_gattc_proc *prev;
BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
prev = NULL;
cur = STAILQ_FIRST(&ble_gattc_procs);
while (cur != NULL) {
if (cur == proc) {
if (prev == NULL) {
STAILQ_REMOVE_HEAD(&ble_gattc_procs, next);
} else {
STAILQ_REMOVE_AFTER(&ble_gattc_procs, prev, next);
}
return 1;
}
prev = cur;
cur = STAILQ_NEXT(cur, next);
}
return 0;
}
#endif
static void
ble_gattc_proc_set_exp_timer(struct ble_gattc_proc *proc)
{
@@ -938,6 +982,48 @@ ble_gattc_proc_set_exp_timer(struct ble_gattc_proc *proc)
ble_npl_time_ms_to_ticks32(BLE_GATTC_UNRESPONSIVE_TIMEOUT_MS);
}
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
static void
ble_gattc_proc_preempt_insert(struct ble_gattc_proc *proc)
{
if (!(proc->flags & BLE_GATTC_PROC_F_STALLED)) {
ble_gattc_proc_set_exp_timer(proc);
}
ble_hs_lock();
proc->flags |= BLE_GATTC_PROC_F_API_LOCKED;
STAILQ_INSERT_TAIL(&ble_gattc_procs, proc, next);
ble_hs_unlock();
ble_hs_timer_resched();
}
static void
ble_gattc_proc_preempt_complete(struct ble_gattc_proc *proc, int status)
{
int free_proc;
free_proc = 0;
ble_hs_lock();
if (status != 0) {
free_proc = ble_gattc_proc_remove_locked(proc);
}
if (proc->flags & BLE_GATTC_PROC_F_FREE_PENDING) {
proc->flags &= ~BLE_GATTC_PROC_F_FREE_PENDING;
free_proc = 1;
}
proc->flags &= ~BLE_GATTC_PROC_F_API_LOCKED;
ble_hs_unlock();
if (free_proc) {
ble_gattc_proc_free(proc);
}
}
#endif
#if MYNEWT_VAL(BLE_GATTC)
static void
ble_gattc_proc_set_resume_timer(struct ble_gattc_proc *proc)
@@ -1182,13 +1268,11 @@ ble_gattc_extract(ble_gattc_match_fn *cb, void *arg, int max_procs,
struct ble_gattc_proc *proc;
struct ble_gattc_proc *prev;
struct ble_gattc_proc *next;
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
struct ble_gattc_proc *cur;
uint8_t flag = 0;
#endif
int num_extracted;
/* Only the parent task is allowed to remove entries from the list. */
/* Parent task performs normal extraction. The preemption-protect TX
* failure path may also remove an active proc while holding ble_hs_lock.
*/
BLE_HS_DBG_ASSERT(ble_hs_is_parent_task());
STAILQ_INIT(dst_list);
@@ -1198,27 +1282,6 @@ ble_gattc_extract(ble_gattc_match_fn *cb, void *arg, int max_procs,
prev = NULL;
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
proc = STAILQ_FIRST(&temp_proc_list);
while (proc != NULL) {
next = STAILQ_NEXT(proc, next);
STAILQ_FOREACH(cur, &ble_gattc_procs, next) {
if (proc == cur) {
flag = 1;
break;
}
}
if (!flag) {
/* Detected a preemption case */
STAILQ_INSERT_TAIL(&ble_gattc_procs, proc, next);
}
flag = 0;
proc = next;
}
/* Clear the temp proc list */
STAILQ_INIT(&temp_proc_list);
#endif
proc = STAILQ_FIRST(&ble_gattc_procs);
while (proc != NULL) {
next = STAILQ_NEXT(proc, next);
@@ -1812,7 +1875,9 @@ ble_gattc_disc_all_svcs_tx(struct ble_gattc_proc *proc)
ble_uuid16_t uuid = BLE_UUID16_INIT(BLE_ATT_UUID_PRIMARY_SERVICE);
int rc;
#if !MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
ble_gattc_dbg_assert_proc_not_inserted(proc);
#endif
rc = ble_att_clt_tx_read_group_type(proc->conn_handle, proc->cid,
proc->disc_all_svcs.prev_handle + 1,
@@ -1977,9 +2042,7 @@ ble_gattc_disc_all_svcs(uint16_t conn_handle, ble_gatt_disc_svc_fn *cb,
proc->disc_all_svcs.cb_arg = cb_arg;
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
ble_hs_lock();
STAILQ_INSERT_TAIL(&temp_proc_list, proc, next);
ble_hs_unlock();
ble_gattc_proc_preempt_insert(proc);
#endif
ble_gattc_log_proc_init("discover all services\n");
@@ -1989,21 +2052,17 @@ ble_gattc_disc_all_svcs(uint16_t conn_handle, ble_gatt_disc_svc_fn *cb,
}
done:
/* One ble_gattc_proc cannot be part of multiple linked lists.
* Hence it needs to be removed before going into ble_gattc_process_status.
*/
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
if (proc != NULL) {
ble_hs_lock();
STAILQ_REMOVE(&temp_proc_list, proc, ble_gattc_proc, next);
ble_hs_unlock();
}
#endif
if (rc != 0) {
STATS_INC(ble_gattc_stats, disc_all_svcs_fail);
}
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
if (proc != NULL) {
ble_gattc_proc_preempt_complete(proc, rc);
return rc;
}
#endif
ble_gattc_process_status(proc, rc);
return rc;
}
@@ -2645,7 +2704,9 @@ ble_gattc_disc_all_chrs_tx(struct ble_gattc_proc *proc)
ble_uuid16_t uuid = BLE_UUID16_INIT(BLE_ATT_UUID_CHARACTERISTIC);
int rc;
#if !MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
ble_gattc_dbg_assert_proc_not_inserted(proc);
#endif
rc = ble_att_clt_tx_read_type(proc->conn_handle, proc->cid,
proc->disc_all_chrs.prev_handle + 1,
@@ -2809,12 +2870,10 @@ ble_gattc_disc_all_chrs(uint16_t conn_handle, uint16_t start_handle,
proc->disc_all_chrs.cb = cb;
proc->disc_all_chrs.cb_arg = cb_arg;
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
ble_hs_lock();
STAILQ_INSERT_TAIL(&temp_proc_list, proc, next);
ble_hs_unlock();
#endif
ble_gattc_log_disc_all_chrs(proc);
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
ble_gattc_proc_preempt_insert(proc);
#endif
rc = ble_gattc_disc_all_chrs_tx(proc);
if (rc != 0) {
@@ -2822,21 +2881,17 @@ ble_gattc_disc_all_chrs(uint16_t conn_handle, uint16_t start_handle,
}
done:
/* One ble_gattc_proc cannot be part of multiple linked lists.
* Hence it needs to be removed before going into ble_gattc_process_status.
*/
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
if (proc != NULL) {
ble_hs_lock();
STAILQ_REMOVE(&temp_proc_list, proc, ble_gattc_proc, next);
ble_hs_unlock();
}
#endif
if (rc != 0) {
STATS_INC(ble_gattc_stats, disc_all_chrs_fail);
}
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
if (proc != NULL) {
ble_gattc_proc_preempt_complete(proc, rc);
return rc;
}
#endif
ble_gattc_process_status(proc, rc);
return rc;
}
@@ -3149,7 +3204,9 @@ ble_gattc_disc_all_dscs_tx(struct ble_gattc_proc *proc)
{
int rc;
#if !MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
ble_gattc_dbg_assert_proc_not_inserted(proc);
#endif
rc = ble_att_clt_tx_find_info(proc->conn_handle, proc->cid,
proc->disc_all_dscs.prev_handle + 1,
@@ -3307,12 +3364,10 @@ ble_gattc_disc_all_dscs(uint16_t conn_handle, uint16_t start_handle,
proc->disc_all_dscs.cb = cb;
proc->disc_all_dscs.cb_arg = cb_arg;
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
ble_hs_lock();
STAILQ_INSERT_TAIL(&temp_proc_list, proc, next);
ble_hs_unlock();
#endif
ble_gattc_log_disc_all_dscs(proc);
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
ble_gattc_proc_preempt_insert(proc);
#endif
rc = ble_gattc_disc_all_dscs_tx(proc);
if (rc != 0) {
@@ -3320,21 +3375,17 @@ ble_gattc_disc_all_dscs(uint16_t conn_handle, uint16_t start_handle,
}
done:
/* One ble_gattc_proc cannot be part of multiple linked lists.
* Hence it needs to be removed before going into ble_gattc_process_status.
*/
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
if (proc != NULL) {
ble_hs_lock();
STAILQ_REMOVE(&temp_proc_list,proc,ble_gattc_proc, next);
ble_hs_unlock();
}
#endif
if (rc != 0) {
STATS_INC(ble_gattc_stats, disc_all_dscs_fail);
}
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
if (proc != NULL) {
ble_gattc_proc_preempt_complete(proc, rc);
return rc;
}
#endif
ble_gattc_process_status(proc, rc);
return rc;
}
@@ -6829,9 +6880,6 @@ ble_gattc_init(void)
}
#endif
#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT)
STAILQ_INIT(&temp_proc_list);
#endif
STAILQ_INIT(&ble_gattc_procs);
#if MYNEWT_VAL(BLE_GATTC_AUTO_PAIR)
STAILQ_INIT(&ble_gattc_cached_procs);