This is an automated email from the ASF dual-hosted git repository.
janc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-nimble.git
The following commit(s) were added to refs/heads/master by this push:
new e9b014e53 nimble/host: Fix ATT Read By Type Response handling
e9b014e53 is described below
commit e9b014e5350daef048683e6face55b52e065542f
Author: Andrzej Kaczmarek <[email protected]>
AuthorDate: Tue Oct 7 10:55:12 2025 +0200
nimble/host: Fix ATT Read By Type Response handling
ATT_READ_BY_TYPE_RSP handler will extract proc from the proc list and
then call the callback for each handle-value pair in the PDU. However,
this means each time proc is extracted it will be then added to the
tail of the proc list. If there are more than 1 matching procesures on
the list, subsequent extracts when handling *the same PDU* may extract
invalid proc from the list and break the procedure.
This fixes the problem by adding proc back to list at the head instead
of the tail of proc list. This way subsequent iterations of handle-value
pairs handling will extract proper proc as it will be the first matching
on the list.
---
nimble/host/src/ble_att_clt.c | 9 ++++-
nimble/host/src/ble_gatt_priv.h | 4 +-
nimble/host/src/ble_gattc.c | 87 +++++++++++++++++++++++------------------
3 files changed, 59 insertions(+), 41 deletions(-)
diff --git a/nimble/host/src/ble_att_clt.c b/nimble/host/src/ble_att_clt.c
index 2b60f8b53..ddd7fc8a9 100644
--- a/nimble/host/src/ble_att_clt.c
+++ b/nimble/host/src/ble_att_clt.c
@@ -431,13 +431,20 @@ ble_att_clt_rx_read_type(uint16_t conn_handle, uint16_t
cid, struct os_mbuf **rx
adata.value_len = data_len - sizeof(*data);
adata.value = data->value;
- ble_gattc_rx_read_type_adata(conn_handle, cid, &adata);
+ rc = ble_gattc_rx_read_type_adata(conn_handle, cid, &adata);
+ if (rc != 0) {
+ /* We should not call complete callback if this returned an error
+ * since proc is not added to the proc list.
+ */
+ return 0;
+ }
os_mbuf_adj(*rxom, data_len);
}
done:
/* Notify GATT that the response is done being parsed. */
ble_gattc_rx_read_type_complete(conn_handle, cid, rc);
+
return rc;
}
diff --git a/nimble/host/src/ble_gatt_priv.h b/nimble/host/src/ble_gatt_priv.h
index 50e9a7d82..9a622788b 100644
--- a/nimble/host/src/ble_gatt_priv.h
+++ b/nimble/host/src/ble_gatt_priv.h
@@ -119,8 +119,8 @@ void ble_gatts_indicate_fail_notconn(uint16_t conn_handle);
void ble_gattc_rx_err(uint16_t conn_handle, uint16_t cid, uint16_t handle,
uint16_t status);
void ble_gattc_rx_mtu(uint16_t conn_handle, uint16_t cid, int status, uint16_t
chan_mtu);
-void ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
- struct ble_att_read_type_adata *adata);
+int ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
+ struct ble_att_read_type_adata *adata);
void ble_gattc_rx_read_type_complete(uint16_t conn_handle, uint16_t cid, int
status);
void ble_gattc_rx_read_rsp(uint16_t conn_handle, uint16_t cid, int status,
struct os_mbuf **rxom);
diff --git a/nimble/host/src/ble_gattc.c b/nimble/host/src/ble_gattc.c
index 0ff4ec28e..a19ed8325 100644
--- a/nimble/host/src/ble_gattc.c
+++ b/nimble/host/src/ble_gattc.c
@@ -754,12 +754,16 @@ ble_gattc_proc_free(struct ble_gattc_proc *proc)
}
static void
-ble_gattc_proc_insert(struct ble_gattc_proc *proc)
+ble_gattc_proc_insert(struct ble_gattc_proc *proc, bool insert_head)
{
ble_gattc_dbg_assert_proc_not_inserted(proc);
ble_hs_lock();
- STAILQ_INSERT_TAIL(&ble_gattc_procs, proc, next);
+ if (insert_head) {
+ STAILQ_INSERT_HEAD(&ble_gattc_procs, proc, next);
+ } else {
+ STAILQ_INSERT_TAIL(&ble_gattc_procs, proc, next);
+ }
ble_hs_unlock();
}
@@ -790,7 +794,7 @@ ble_gattc_proc_set_resume_timer(struct ble_gattc_proc *proc)
}
static void
-ble_gattc_process_status(struct ble_gattc_proc *proc, int status)
+ble_gattc_process_status(struct ble_gattc_proc *proc, int status, bool
insert_head)
{
switch (status) {
case 0:
@@ -798,7 +802,7 @@ ble_gattc_process_status(struct ble_gattc_proc *proc, int
status)
ble_gattc_proc_set_exp_timer(proc);
}
- ble_gattc_proc_insert(proc);
+ ble_gattc_proc_insert(proc, insert_head);
ble_hs_timer_resched();
break;
@@ -1187,7 +1191,7 @@ ble_gattc_resume_procs(void)
proc->flags &= ~BLE_GATTC_PROC_F_STALLED;
rc = resume_cb(proc);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -1398,7 +1402,7 @@ done:
STATS_INC(ble_gattc_stats, mtu_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -1623,7 +1627,7 @@ done:
STATS_INC(ble_gattc_stats, disc_all_svcs_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -1837,7 +1841,7 @@ done:
STATS_INC(ble_gattc_stats, disc_svc_uuid_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -2153,7 +2157,7 @@ done:
STATS_INC(ble_gattc_stats, find_inc_svcs_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -2386,7 +2390,7 @@ done:
STATS_INC(ble_gattc_stats, disc_all_chrs_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -2626,7 +2630,7 @@ done:
STATS_INC(ble_gattc_stats, disc_chrs_uuid_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -2836,7 +2840,7 @@ done:
STATS_INC(ble_gattc_stats, disc_all_dscs_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -2971,7 +2975,7 @@ done:
STATS_INC(ble_gattc_stats, read_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -3135,7 +3139,7 @@ done:
STATS_INC(ble_gattc_stats, read_uuid_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -3331,7 +3335,7 @@ done:
STATS_INC(ble_gattc_stats, read_long_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -3555,7 +3559,7 @@ done:
STATS_INC(ble_gattc_stats, read_mult_fail);
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -3730,7 +3734,7 @@ done:
/* Free the mbuf in case the send failed. */
os_mbuf_free_chain(txom);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -4048,7 +4052,7 @@ done:
/* Free the mbuf in case of failure. */
os_mbuf_free_chain(txom);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -4337,7 +4341,7 @@ done:
attrs[i].om = NULL;
}
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
return rc;
}
@@ -4709,7 +4713,7 @@ done:
/* Tell the application that an indication transmission was attempted. */
ble_gap_notify_tx_event(rc, conn_handle, chr_val_handle, 1);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
os_mbuf_free_chain(txom);
return rc;
}
@@ -4777,7 +4781,7 @@ ble_gattc_rx_mtu(uint16_t conn_handle, uint16_t cid, int
status, uint16_t chan_m
proc = ble_gattc_extract_first_by_conn_cid_op(conn_handle,
BLE_L2CAP_CID_ATT, BLE_GATT_OP_MTU);
if (proc != NULL) {
ble_gattc_mtu_cb(proc, status, 0, chan_mtu);
- ble_gattc_process_status(proc, BLE_HS_EDONE);
+ ble_gattc_process_status(proc, BLE_HS_EDONE, false);
}
}
@@ -4800,7 +4804,7 @@ ble_gattc_rx_find_info_idata(uint16_t conn_handle,
uint16_t cid,
BLE_GATT_OP_DISC_ALL_DSCS);
if (proc != NULL) {
rc = ble_gattc_disc_all_dscs_rx_idata(proc, idata);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -4822,7 +4826,7 @@ ble_gattc_rx_find_info_complete(uint16_t conn_handle,
uint16_t cid, int status)
BLE_GATT_OP_DISC_ALL_DSCS);
if (proc != NULL) {
rc = ble_gattc_disc_all_dscs_rx_complete(proc, status);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -4845,7 +4849,7 @@ ble_gattc_rx_find_type_value_hinfo(uint16_t conn_handle,
uint16_t cid,
BLE_GATT_OP_DISC_SVC_UUID);
if (proc != NULL) {
rc = ble_gattc_disc_svc_uuid_rx_hinfo(proc, hinfo);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -4867,7 +4871,7 @@ ble_gattc_rx_find_type_value_complete(uint16_t
conn_handle, uint16_t cid, int st
BLE_GATT_OP_DISC_SVC_UUID);
if (proc != NULL) {
rc = ble_gattc_disc_svc_uuid_rx_complete(proc, status);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -4875,12 +4879,12 @@ ble_gattc_rx_find_type_value_complete(uint16_t
conn_handle, uint16_t cid, int st
* Dispatches an incoming "attribute data" entry from a read-by-type-response
* to the appropriate active GATT procedure.
*/
-void
+int
ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
struct ble_att_read_type_adata *adata)
{
#if !NIMBLE_BLE_ATT_CLT_READ_TYPE
- return;
+ return BLE_HS_ENOTSUP;
#endif
const struct ble_gattc_rx_adata_entry *rx_entry;
@@ -4892,8 +4896,15 @@ ble_gattc_rx_read_type_adata(uint16_t conn_handle,
uint16_t cid,
&rx_entry);
if (proc != NULL) {
rc = rx_entry->cb(proc, adata);
- ble_gattc_process_status(proc, rc);
+ /* We need to put procedure on head of the queue since caller expects
+ * to process the same proc again.
+ */
+ ble_gattc_process_status(proc, rc, true);
+ } else {
+ rc = -1;
}
+
+ return rc;
}
/**
@@ -4916,7 +4927,7 @@ ble_gattc_rx_read_type_complete(uint16_t conn_handle,
uint16_t cid, int status)
&rx_entry);
if (proc != NULL) {
rc = rx_entry->cb(proc, status);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -4939,7 +4950,7 @@ ble_gattc_rx_read_group_type_adata(uint16_t conn_handle,
uint16_t cid,
BLE_GATT_OP_DISC_ALL_SVCS);
if (proc != NULL) {
rc = ble_gattc_disc_all_svcs_rx_adata(proc, adata);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -4961,7 +4972,7 @@ ble_gattc_rx_read_group_type_complete(uint16_t
conn_handle, uint16_t cid, int st
BLE_GATT_OP_DISC_ALL_SVCS);
if (proc != NULL) {
rc = ble_gattc_disc_all_svcs_rx_complete(proc, status);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -4985,7 +4996,7 @@ ble_gattc_rx_read_rsp(uint16_t conn_handle, uint16_t cid,
int status, struct os_
&rx_entry);
if (proc != NULL) {
rc = rx_entry->cb(proc, status, om);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -5008,7 +5019,7 @@ ble_gattc_rx_read_blob_rsp(uint16_t conn_handle, uint16_t
cid, int status,
BLE_GATT_OP_READ_LONG);
if (proc != NULL) {
rc = ble_gattc_read_long_rx_read_rsp(proc, status, om);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -5032,7 +5043,7 @@ ble_gattc_rx_read_mult_rsp(uint16_t conn_handle, uint16_t
cid, int status,
proc = ble_gattc_extract_first_by_conn_cid_op(conn_handle, cid, op);
if (proc != NULL) {
ble_gattc_read_mult_cb(proc, status, 0, om);
- ble_gattc_process_status(proc, BLE_HS_EDONE);
+ ble_gattc_process_status(proc, BLE_HS_EDONE, false);
}
}
@@ -5053,7 +5064,7 @@ ble_gattc_rx_write_rsp(uint16_t conn_handle, uint16_t cid)
BLE_GATT_OP_WRITE);
if (proc != NULL) {
ble_gattc_write_cb(proc, 0, 0);
- ble_gattc_process_status(proc, BLE_HS_EDONE);
+ ble_gattc_process_status(proc, BLE_HS_EDONE, false);
}
}
@@ -5079,7 +5090,7 @@ ble_gattc_rx_prep_write_rsp(uint16_t conn_handle,
uint16_t cid, int status,
&rx_entry);
if (proc != NULL) {
rc = rx_entry->cb(proc, status, handle, offset, om);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -5102,7 +5113,7 @@ ble_gattc_rx_exec_write_rsp(uint16_t conn_handle,
uint16_t cid, int status)
ble_gattc_rx_exec_entries, &rx_entry);
if (proc != NULL) {
rc = rx_entry->cb(proc, status);
- ble_gattc_process_status(proc, rc);
+ ble_gattc_process_status(proc, rc, false);
}
}
@@ -5123,7 +5134,7 @@ ble_gatts_rx_indicate_rsp(uint16_t conn_handle, uint16_t
cid)
BLE_GATT_OP_INDICATE);
if (proc != NULL) {
ble_gatts_indicate_rx_rsp(proc);
- ble_gattc_process_status(proc, BLE_HS_EDONE);
+ ble_gattc_process_status(proc, BLE_HS_EDONE, false);
}
}
