Skip to content

Commit 864cf79

Browse files
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.
1 parent b7590dc commit 864cf79

File tree

3 files changed

+58
-40
lines changed

3 files changed

+58
-40
lines changed

nimble/host/src/ble_att_clt.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,13 +431,20 @@ ble_att_clt_rx_read_type(uint16_t conn_handle, uint16_t cid, struct os_mbuf **rx
431431
adata.value_len = data_len - sizeof(*data);
432432
adata.value = data->value;
433433

434-
ble_gattc_rx_read_type_adata(conn_handle, cid, &adata);
434+
rc = ble_gattc_rx_read_type_adata(conn_handle, cid, &adata);
435+
if (rc != 0) {
436+
/* We should not call complete callback if this returned an error
437+
* since proc is not added to the proc list.
438+
*/
439+
return rc;
440+
}
435441
os_mbuf_adj(*rxom, data_len);
436442
}
437443

438444
done:
439445
/* Notify GATT that the response is done being parsed. */
440446
ble_gattc_rx_read_type_complete(conn_handle, cid, rc);
447+
441448
return rc;
442449

443450
}

nimble/host/src/ble_gatt_priv.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ void ble_gatts_indicate_fail_notconn(uint16_t conn_handle);
119119

120120
void ble_gattc_rx_err(uint16_t conn_handle, uint16_t cid, uint16_t handle, uint16_t status);
121121
void ble_gattc_rx_mtu(uint16_t conn_handle, uint16_t cid, int status, uint16_t chan_mtu);
122-
void ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
123-
struct ble_att_read_type_adata *adata);
122+
int ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
123+
struct ble_att_read_type_adata *adata);
124124
void ble_gattc_rx_read_type_complete(uint16_t conn_handle, uint16_t cid, int status);
125125
void ble_gattc_rx_read_rsp(uint16_t conn_handle, uint16_t cid, int status,
126126
struct os_mbuf **rxom);

nimble/host/src/ble_gattc.c

Lines changed: 48 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -754,12 +754,16 @@ ble_gattc_proc_free(struct ble_gattc_proc *proc)
754754
}
755755

756756
static void
757-
ble_gattc_proc_insert(struct ble_gattc_proc *proc)
757+
ble_gattc_proc_insert(struct ble_gattc_proc *proc, bool insert_head)
758758
{
759759
ble_gattc_dbg_assert_proc_not_inserted(proc);
760760

761761
ble_hs_lock();
762-
STAILQ_INSERT_TAIL(&ble_gattc_procs, proc, next);
762+
if (insert_head) {
763+
STAILQ_INSERT_HEAD(&ble_gattc_procs, proc, next);
764+
} else {
765+
STAILQ_INSERT_TAIL(&ble_gattc_procs, proc, next);
766+
}
763767
ble_hs_unlock();
764768
}
765769

@@ -790,15 +794,15 @@ ble_gattc_proc_set_resume_timer(struct ble_gattc_proc *proc)
790794
}
791795

792796
static void
793-
ble_gattc_process_status(struct ble_gattc_proc *proc, int status)
797+
ble_gattc_process_status(struct ble_gattc_proc *proc, int status, bool insert_head)
794798
{
795799
switch (status) {
796800
case 0:
797801
if (!(proc->flags & BLE_GATTC_PROC_F_STALLED)) {
798802
ble_gattc_proc_set_exp_timer(proc);
799803
}
800804

801-
ble_gattc_proc_insert(proc);
805+
ble_gattc_proc_insert(proc, insert_head);
802806
ble_hs_timer_resched();
803807
break;
804808

@@ -1187,7 +1191,7 @@ ble_gattc_resume_procs(void)
11871191

11881192
proc->flags &= ~BLE_GATTC_PROC_F_STALLED;
11891193
rc = resume_cb(proc);
1190-
ble_gattc_process_status(proc, rc);
1194+
ble_gattc_process_status(proc, rc, false);
11911195
}
11921196
}
11931197

@@ -1398,7 +1402,7 @@ ble_gattc_exchange_mtu(uint16_t conn_handle, ble_gatt_mtu_fn *cb, void *cb_arg)
13981402
STATS_INC(ble_gattc_stats, mtu_fail);
13991403
}
14001404

1401-
ble_gattc_process_status(proc, rc);
1405+
ble_gattc_process_status(proc, rc, false);
14021406
return rc;
14031407
}
14041408

@@ -1623,7 +1627,7 @@ ble_gattc_disc_all_svcs(uint16_t conn_handle, ble_gatt_disc_svc_fn *cb,
16231627
STATS_INC(ble_gattc_stats, disc_all_svcs_fail);
16241628
}
16251629

1626-
ble_gattc_process_status(proc, rc);
1630+
ble_gattc_process_status(proc, rc, false);
16271631
return rc;
16281632
}
16291633

@@ -1837,7 +1841,7 @@ ble_gattc_disc_svc_by_uuid(uint16_t conn_handle, const ble_uuid_t *uuid,
18371841
STATS_INC(ble_gattc_stats, disc_svc_uuid_fail);
18381842
}
18391843

1840-
ble_gattc_process_status(proc, rc);
1844+
ble_gattc_process_status(proc, rc, false);
18411845
return rc;
18421846
}
18431847

@@ -2153,7 +2157,7 @@ ble_gattc_find_inc_svcs(uint16_t conn_handle, uint16_t start_handle,
21532157
STATS_INC(ble_gattc_stats, find_inc_svcs_fail);
21542158
}
21552159

2156-
ble_gattc_process_status(proc, rc);
2160+
ble_gattc_process_status(proc, rc, false);
21572161
return rc;
21582162
}
21592163

@@ -2386,7 +2390,7 @@ ble_gattc_disc_all_chrs(uint16_t conn_handle, uint16_t start_handle,
23862390
STATS_INC(ble_gattc_stats, disc_all_chrs_fail);
23872391
}
23882392

2389-
ble_gattc_process_status(proc, rc);
2393+
ble_gattc_process_status(proc, rc, false);
23902394
return rc;
23912395
}
23922396

@@ -2626,7 +2630,7 @@ ble_gattc_disc_chrs_by_uuid(uint16_t conn_handle, uint16_t start_handle,
26262630
STATS_INC(ble_gattc_stats, disc_chrs_uuid_fail);
26272631
}
26282632

2629-
ble_gattc_process_status(proc, rc);
2633+
ble_gattc_process_status(proc, rc, false);
26302634
return rc;
26312635
}
26322636

@@ -2836,7 +2840,7 @@ ble_gattc_disc_all_dscs(uint16_t conn_handle, uint16_t start_handle,
28362840
STATS_INC(ble_gattc_stats, disc_all_dscs_fail);
28372841
}
28382842

2839-
ble_gattc_process_status(proc, rc);
2843+
ble_gattc_process_status(proc, rc, false);
28402844
return rc;
28412845
}
28422846

@@ -2971,7 +2975,7 @@ ble_gattc_read(uint16_t conn_handle, uint16_t attr_handle,
29712975
STATS_INC(ble_gattc_stats, read_fail);
29722976
}
29732977

2974-
ble_gattc_process_status(proc, rc);
2978+
ble_gattc_process_status(proc, rc, false);
29752979
return rc;
29762980
}
29772981

@@ -3135,7 +3139,7 @@ ble_gattc_read_by_uuid(uint16_t conn_handle, uint16_t start_handle,
31353139
STATS_INC(ble_gattc_stats, read_uuid_fail);
31363140
}
31373141

3138-
ble_gattc_process_status(proc, rc);
3142+
ble_gattc_process_status(proc, rc, false);
31393143
return rc;
31403144
}
31413145

@@ -3331,7 +3335,7 @@ ble_gattc_read_long(uint16_t conn_handle, uint16_t handle, uint16_t offset,
33313335
STATS_INC(ble_gattc_stats, read_long_fail);
33323336
}
33333337

3334-
ble_gattc_process_status(proc, rc);
3338+
ble_gattc_process_status(proc, rc, false);
33353339
return rc;
33363340
}
33373341

@@ -3555,7 +3559,7 @@ ble_gattc_read_mult_internal(uint16_t conn_handle, const uint16_t *handles,
35553559
STATS_INC(ble_gattc_stats, read_mult_fail);
35563560
}
35573561

3558-
ble_gattc_process_status(proc, rc);
3562+
ble_gattc_process_status(proc, rc, false);
35593563
return rc;
35603564
}
35613565

@@ -3730,7 +3734,7 @@ ble_gattc_write(uint16_t conn_handle, uint16_t attr_handle,
37303734
/* Free the mbuf in case the send failed. */
37313735
os_mbuf_free_chain(txom);
37323736

3733-
ble_gattc_process_status(proc, rc);
3737+
ble_gattc_process_status(proc, rc, false);
37343738
return rc;
37353739
}
37363740

@@ -4048,7 +4052,7 @@ ble_gattc_write_long(uint16_t conn_handle, uint16_t attr_handle,
40484052
/* Free the mbuf in case of failure. */
40494053
os_mbuf_free_chain(txom);
40504054

4051-
ble_gattc_process_status(proc, rc);
4055+
ble_gattc_process_status(proc, rc, false);
40524056
return rc;
40534057
}
40544058

@@ -4337,7 +4341,7 @@ ble_gattc_write_reliable(uint16_t conn_handle,
43374341
attrs[i].om = NULL;
43384342
}
43394343

4340-
ble_gattc_process_status(proc, rc);
4344+
ble_gattc_process_status(proc, rc, false);
43414345
return rc;
43424346
}
43434347

@@ -4709,7 +4713,7 @@ ble_gatts_indicate_custom(uint16_t conn_handle, uint16_t chr_val_handle,
47094713
/* Tell the application that an indication transmission was attempted. */
47104714
ble_gap_notify_tx_event(rc, conn_handle, chr_val_handle, 1);
47114715

4712-
ble_gattc_process_status(proc, rc);
4716+
ble_gattc_process_status(proc, rc, false);
47134717
os_mbuf_free_chain(txom);
47144718
return rc;
47154719
}
@@ -4777,7 +4781,7 @@ ble_gattc_rx_mtu(uint16_t conn_handle, uint16_t cid, int status, uint16_t chan_m
47774781
proc = ble_gattc_extract_first_by_conn_cid_op(conn_handle, BLE_L2CAP_CID_ATT, BLE_GATT_OP_MTU);
47784782
if (proc != NULL) {
47794783
ble_gattc_mtu_cb(proc, status, 0, chan_mtu);
4780-
ble_gattc_process_status(proc, BLE_HS_EDONE);
4784+
ble_gattc_process_status(proc, BLE_HS_EDONE, false);
47814785
}
47824786
}
47834787

@@ -4800,7 +4804,7 @@ ble_gattc_rx_find_info_idata(uint16_t conn_handle, uint16_t cid,
48004804
BLE_GATT_OP_DISC_ALL_DSCS);
48014805
if (proc != NULL) {
48024806
rc = ble_gattc_disc_all_dscs_rx_idata(proc, idata);
4803-
ble_gattc_process_status(proc, rc);
4807+
ble_gattc_process_status(proc, rc, false);
48044808
}
48054809
}
48064810

@@ -4822,7 +4826,7 @@ ble_gattc_rx_find_info_complete(uint16_t conn_handle, uint16_t cid, int status)
48224826
BLE_GATT_OP_DISC_ALL_DSCS);
48234827
if (proc != NULL) {
48244828
rc = ble_gattc_disc_all_dscs_rx_complete(proc, status);
4825-
ble_gattc_process_status(proc, rc);
4829+
ble_gattc_process_status(proc, rc, false);
48264830
}
48274831
}
48284832

@@ -4845,7 +4849,7 @@ ble_gattc_rx_find_type_value_hinfo(uint16_t conn_handle, uint16_t cid,
48454849
BLE_GATT_OP_DISC_SVC_UUID);
48464850
if (proc != NULL) {
48474851
rc = ble_gattc_disc_svc_uuid_rx_hinfo(proc, hinfo);
4848-
ble_gattc_process_status(proc, rc);
4852+
ble_gattc_process_status(proc, rc, false);
48494853
}
48504854
}
48514855

@@ -4867,15 +4871,15 @@ ble_gattc_rx_find_type_value_complete(uint16_t conn_handle, uint16_t cid, int st
48674871
BLE_GATT_OP_DISC_SVC_UUID);
48684872
if (proc != NULL) {
48694873
rc = ble_gattc_disc_svc_uuid_rx_complete(proc, status);
4870-
ble_gattc_process_status(proc, rc);
4874+
ble_gattc_process_status(proc, rc, false);
48714875
}
48724876
}
48734877

48744878
/**
48754879
* Dispatches an incoming "attribute data" entry from a read-by-type-response
48764880
* to the appropriate active GATT procedure.
48774881
*/
4878-
void
4882+
int
48794883
ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
48804884
struct ble_att_read_type_adata *adata)
48814885
{
@@ -4892,8 +4896,15 @@ ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
48924896
&rx_entry);
48934897
if (proc != NULL) {
48944898
rc = rx_entry->cb(proc, adata);
4895-
ble_gattc_process_status(proc, rc);
4899+
/* We need to put procedure on head of the queue since caller expects
4900+
* to process the same proc again.
4901+
*/
4902+
ble_gattc_process_status(proc, rc, true);
4903+
} else {
4904+
rc = -1;
48964905
}
4906+
4907+
return rc;
48974908
}
48984909

48994910
/**
@@ -4916,7 +4927,7 @@ ble_gattc_rx_read_type_complete(uint16_t conn_handle, uint16_t cid, int status)
49164927
&rx_entry);
49174928
if (proc != NULL) {
49184929
rc = rx_entry->cb(proc, status);
4919-
ble_gattc_process_status(proc, rc);
4930+
ble_gattc_process_status(proc, rc, false);
49204931
}
49214932
}
49224933

@@ -4939,7 +4950,7 @@ ble_gattc_rx_read_group_type_adata(uint16_t conn_handle, uint16_t cid,
49394950
BLE_GATT_OP_DISC_ALL_SVCS);
49404951
if (proc != NULL) {
49414952
rc = ble_gattc_disc_all_svcs_rx_adata(proc, adata);
4942-
ble_gattc_process_status(proc, rc);
4953+
ble_gattc_process_status(proc, rc, false);
49434954
}
49444955
}
49454956

@@ -4961,7 +4972,7 @@ ble_gattc_rx_read_group_type_complete(uint16_t conn_handle, uint16_t cid, int st
49614972
BLE_GATT_OP_DISC_ALL_SVCS);
49624973
if (proc != NULL) {
49634974
rc = ble_gattc_disc_all_svcs_rx_complete(proc, status);
4964-
ble_gattc_process_status(proc, rc);
4975+
ble_gattc_process_status(proc, rc, false);
49654976
}
49664977
}
49674978

@@ -4985,7 +4996,7 @@ ble_gattc_rx_read_rsp(uint16_t conn_handle, uint16_t cid, int status, struct os_
49854996
&rx_entry);
49864997
if (proc != NULL) {
49874998
rc = rx_entry->cb(proc, status, om);
4988-
ble_gattc_process_status(proc, rc);
4999+
ble_gattc_process_status(proc, rc, false);
49895000
}
49905001
}
49915002

@@ -5008,7 +5019,7 @@ ble_gattc_rx_read_blob_rsp(uint16_t conn_handle, uint16_t cid, int status,
50085019
BLE_GATT_OP_READ_LONG);
50095020
if (proc != NULL) {
50105021
rc = ble_gattc_read_long_rx_read_rsp(proc, status, om);
5011-
ble_gattc_process_status(proc, rc);
5022+
ble_gattc_process_status(proc, rc, false);
50125023
}
50135024
}
50145025

@@ -5032,7 +5043,7 @@ ble_gattc_rx_read_mult_rsp(uint16_t conn_handle, uint16_t cid, int status,
50325043
proc = ble_gattc_extract_first_by_conn_cid_op(conn_handle, cid, op);
50335044
if (proc != NULL) {
50345045
ble_gattc_read_mult_cb(proc, status, 0, om);
5035-
ble_gattc_process_status(proc, BLE_HS_EDONE);
5046+
ble_gattc_process_status(proc, BLE_HS_EDONE, false);
50365047
}
50375048
}
50385049

@@ -5053,7 +5064,7 @@ ble_gattc_rx_write_rsp(uint16_t conn_handle, uint16_t cid)
50535064
BLE_GATT_OP_WRITE);
50545065
if (proc != NULL) {
50555066
ble_gattc_write_cb(proc, 0, 0);
5056-
ble_gattc_process_status(proc, BLE_HS_EDONE);
5067+
ble_gattc_process_status(proc, BLE_HS_EDONE, false);
50575068
}
50585069
}
50595070

@@ -5079,7 +5090,7 @@ ble_gattc_rx_prep_write_rsp(uint16_t conn_handle, uint16_t cid, int status,
50795090
&rx_entry);
50805091
if (proc != NULL) {
50815092
rc = rx_entry->cb(proc, status, handle, offset, om);
5082-
ble_gattc_process_status(proc, rc);
5093+
ble_gattc_process_status(proc, rc, false);
50835094
}
50845095
}
50855096

@@ -5102,7 +5113,7 @@ ble_gattc_rx_exec_write_rsp(uint16_t conn_handle, uint16_t cid, int status)
51025113
ble_gattc_rx_exec_entries, &rx_entry);
51035114
if (proc != NULL) {
51045115
rc = rx_entry->cb(proc, status);
5105-
ble_gattc_process_status(proc, rc);
5116+
ble_gattc_process_status(proc, rc, false);
51065117
}
51075118
}
51085119

@@ -5123,7 +5134,7 @@ ble_gatts_rx_indicate_rsp(uint16_t conn_handle, uint16_t cid)
51235134
BLE_GATT_OP_INDICATE);
51245135
if (proc != NULL) {
51255136
ble_gatts_indicate_rx_rsp(proc);
5126-
ble_gattc_process_status(proc, BLE_HS_EDONE);
5137+
ble_gattc_process_status(proc, BLE_HS_EDONE, false);
51275138
}
51285139
}
51295140

0 commit comments

Comments
 (0)