From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> If there are pending request while disconnecting they would be notified but clients may endup being freed in the proccess which will then be calling bt_att_cancel to cancal its requests causing the following trace: Invalid read of size 4 at 0x1D894C: enable_ccc_callback (gatt-client.c:1627) by 0x1D247B: disc_att_send_op (att.c:417) by 0x1CCC17: queue_remove_all (queue.c:354) by 0x1D47B7: disconnect_cb (att.c:635) by 0x1E0707: watch_callback (io-glib.c:170) by 0x48E963B: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.6400.4) by 0x48E9AC7: ??? (in /usr/lib/libglib-2.0.so.0.6400.4) by 0x48E9ECF: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.6400.4) by 0x1E0E97: mainloop_run (mainloop-glib.c:79) by 0x1E13B3: mainloop_run_with_signal (mainloop-notify.c:201) by 0x12BC3B: main (main.c:770) Address 0x7d40a28 is 24 bytes inside a block of size 32 free'd at 0x484A2E0: free (vg_replace_malloc.c:540) by 0x1CCC17: queue_remove_all (queue.c:354) by 0x1CCC83: queue_destroy (queue.c:73) by 0x1D7DD7: bt_gatt_client_free (gatt-client.c:2209) by 0x16497B: batt_free (battery.c:77) by 0x16497B: batt_remove (battery.c:286) by 0x1A0013: service_remove (service.c:176) by 0x1A9B7B: device_remove_gatt_service (device.c:3691) by 0x1A9B7B: gatt_service_removed (device.c:3805) by 0x1CC90B: queue_foreach (queue.c:220) by 0x1DE27B: notify_service_changed.isra.0.part.0 (gatt-db.c:369) by 0x1DE387: notify_service_changed (gatt-db.c:361) by 0x1DE387: gatt_db_service_destroy (gatt-db.c:385) by 0x1DE3EF: gatt_db_remove_service (gatt-db.c:519) by 0x1D674F: discovery_op_complete (gatt-client.c:388) by 0x1D6877: discover_primary_cb (gatt-client.c:1260) by 0x1E220B: discovery_op_complete (gatt-helpers.c:628) by 0x1E249B: read_by_grp_type_cb (gatt-helpers.c:730) by 0x1D247B: disc_att_send_op (att.c:417) by 0x1CCC17: queue_remove_all (queue.c:354) by 0x1D47B7: disconnect_cb (att.c:635) --- src/shared/att.c | 46 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/src/shared/att.c b/src/shared/att.c index ed3af2920..58f23dfcb 100644 --- a/src/shared/att.c +++ b/src/shared/att.c @@ -84,6 +84,7 @@ struct bt_att { struct queue *req_queue; /* Queued ATT protocol requests */ struct queue *ind_queue; /* Queued ATT protocol indications */ struct queue *write_queue; /* Queue of PDUs ready to send */ + bool in_disc; /* Cleanup queues on disconnect_cb */ bt_att_timeout_func_t timeout_callback; bt_att_destroy_func_t timeout_destroy; @@ -222,8 +223,10 @@ static void destroy_att_send_op(void *data) free(op); } -static void cancel_att_send_op(struct att_send_op *op) +static void cancel_att_send_op(void *data) { + struct att_send_op *op = data; + if (op->destroy) op->destroy(op->user_data); @@ -631,11 +634,6 @@ static bool disconnect_cb(struct io *io, void *user_data) /* Dettach channel */ queue_remove(att->chans, chan); - /* Notify request callbacks */ - queue_remove_all(att->req_queue, NULL, NULL, disc_att_send_op); - queue_remove_all(att->ind_queue, NULL, NULL, disc_att_send_op); - queue_remove_all(att->write_queue, NULL, NULL, disc_att_send_op); - if (chan->pending_req) { disc_att_send_op(chan->pending_req); chan->pending_req = NULL; @@ -654,6 +652,15 @@ static bool disconnect_cb(struct io *io, void *user_data) bt_att_ref(att); + att->in_disc = true; + + /* Notify request callbacks */ + queue_remove_all(att->req_queue, NULL, NULL, disc_att_send_op); + queue_remove_all(att->ind_queue, NULL, NULL, disc_att_send_op); + queue_remove_all(att->write_queue, NULL, NULL, disc_att_send_op); + + att->in_disc = false; + queue_foreach(att->disconn_list, disconn_handler, INT_TO_PTR(err)); bt_att_unregister_all(att); @@ -1574,6 +1581,30 @@ bool bt_att_chan_cancel(struct bt_att_chan *chan, unsigned int id) return true; } +static bool bt_att_disc_cancel(struct bt_att *att, unsigned int id) +{ + struct att_send_op *op; + + op = queue_find(att->req_queue, match_op_id, UINT_TO_PTR(id)); + if (op) + goto done; + + op = queue_find(att->ind_queue, match_op_id, UINT_TO_PTR(id)); + if (op) + goto done; + + op = queue_find(att->write_queue, match_op_id, UINT_TO_PTR(id)); + +done: + if (!op) + return false; + + /* Just cancel since disconnect_cb will be cleaning up */ + cancel_att_send_op(op); + + return true; +} + bool bt_att_cancel(struct bt_att *att, unsigned int id) { const struct queue_entry *entry; @@ -1591,6 +1622,9 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id) return true; } + if (att->in_disc) + return bt_att_disc_cancel(att, id); + op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id)); if (op) goto done; -- 2.25.3