Skip to content

Commit 1cd644d

Browse files
committed
shared/att: Fix possible crash on disconnect
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)
1 parent 1d1577c commit 1cd644d

File tree

1 file changed

+40
-6
lines changed

1 file changed

+40
-6
lines changed

Diff for: src/shared/att.c

+40-6
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ struct bt_att {
8484
struct queue *req_queue; /* Queued ATT protocol requests */
8585
struct queue *ind_queue; /* Queued ATT protocol indications */
8686
struct queue *write_queue; /* Queue of PDUs ready to send */
87+
bool in_disc; /* Cleanup queues on disconnect_cb */
8788

8889
bt_att_timeout_func_t timeout_callback;
8990
bt_att_destroy_func_t timeout_destroy;
@@ -222,8 +223,10 @@ static void destroy_att_send_op(void *data)
222223
free(op);
223224
}
224225

225-
static void cancel_att_send_op(struct att_send_op *op)
226+
static void cancel_att_send_op(void *data)
226227
{
228+
struct att_send_op *op = data;
229+
227230
if (op->destroy)
228231
op->destroy(op->user_data);
229232

@@ -631,11 +634,6 @@ static bool disconnect_cb(struct io *io, void *user_data)
631634
/* Dettach channel */
632635
queue_remove(att->chans, chan);
633636

634-
/* Notify request callbacks */
635-
queue_remove_all(att->req_queue, NULL, NULL, disc_att_send_op);
636-
queue_remove_all(att->ind_queue, NULL, NULL, disc_att_send_op);
637-
queue_remove_all(att->write_queue, NULL, NULL, disc_att_send_op);
638-
639637
if (chan->pending_req) {
640638
disc_att_send_op(chan->pending_req);
641639
chan->pending_req = NULL;
@@ -654,6 +652,15 @@ static bool disconnect_cb(struct io *io, void *user_data)
654652

655653
bt_att_ref(att);
656654

655+
att->in_disc = true;
656+
657+
/* Notify request callbacks */
658+
queue_remove_all(att->req_queue, NULL, NULL, disc_att_send_op);
659+
queue_remove_all(att->ind_queue, NULL, NULL, disc_att_send_op);
660+
queue_remove_all(att->write_queue, NULL, NULL, disc_att_send_op);
661+
662+
att->in_disc = false;
663+
657664
queue_foreach(att->disconn_list, disconn_handler, INT_TO_PTR(err));
658665

659666
bt_att_unregister_all(att);
@@ -1574,6 +1581,30 @@ bool bt_att_chan_cancel(struct bt_att_chan *chan, unsigned int id)
15741581
return true;
15751582
}
15761583

1584+
static bool bt_att_disc_cancel(struct bt_att *att, unsigned int id)
1585+
{
1586+
struct att_send_op *op;
1587+
1588+
op = queue_find(att->req_queue, match_op_id, UINT_TO_PTR(id));
1589+
if (op)
1590+
goto done;
1591+
1592+
op = queue_find(att->ind_queue, match_op_id, UINT_TO_PTR(id));
1593+
if (op)
1594+
goto done;
1595+
1596+
op = queue_find(att->write_queue, match_op_id, UINT_TO_PTR(id));
1597+
1598+
done:
1599+
if (!op)
1600+
return false;
1601+
1602+
/* Just cancel since disconnect_cb will be cleaning up */
1603+
cancel_att_send_op(op);
1604+
1605+
return true;
1606+
}
1607+
15771608
bool bt_att_cancel(struct bt_att *att, unsigned int id)
15781609
{
15791610
const struct queue_entry *entry;
@@ -1591,6 +1622,9 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id)
15911622
return true;
15921623
}
15931624

1625+
if (att->in_disc)
1626+
return bt_att_disc_cancel(att, id);
1627+
15941628
op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id));
15951629
if (op)
15961630
goto done;

0 commit comments

Comments
 (0)