From b12d1984c6747f7d3bf08d3eee13ee92e31e02bd Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Mon, 19 Nov 2018 17:19:39 +0200 Subject: [PATCH] gatt: Fix invalid read when disconnecting In case there is a client of AcquireNotify and a disconnect happens the code not only have to free the client object but also destroy the io associated with it, for this reason the client object cannot be freed until the io is destroyed otherwise it may lead to the following error: Invalid read of size 4 at 0x63920: notify_io_destroy (gatt-client.c:1461) by 0x63EDB: pipe_io_destroy (gatt-client.c:1082) by 0x6405B: characteristic_free (gatt-client.c:1663) by 0x81F33: remove_interface (object.c:667) by 0x826CB: g_dbus_unregister_interface (object.c:1391) by 0x85D2B: queue_remove_all (queue.c:354) by 0x635F7: unregister_service (gatt-client.c:1893) by 0x85CF7: queue_remove_all (queue.c:339) by 0x661DF: btd_gatt_client_service_removed (gatt-client.c:2199) by 0x695CB: gatt_service_removed (device.c:3747) by 0x85B17: queue_foreach (queue.c:220) by 0x91283: notify_service_changed (gatt-db.c:280) by 0x91283: gatt_db_service_destroy (gatt-db.c:291) Address 0x515ed48 is 0 bytes inside a block of size 20 free'd at 0x483EAD0: free (vg_replace_malloc.c:530) by 0x85D2B: queue_remove_all (queue.c:354) by 0x636D3: unregister_characteristic (gatt-client.c:1741) by 0x85D2B: queue_remove_all (queue.c:354) by 0x635F7: unregister_service (gatt-client.c:1893) by 0x85CF7: queue_remove_all (queue.c:339) by 0x661DF: btd_gatt_client_service_removed (gatt-client.c:2199) by 0x695CB: gatt_service_removed (device.c:3747) by 0x85B17: queue_foreach (queue.c:220) by 0x91283: notify_service_changed (gatt-db.c:280) by 0x91283: gatt_db_service_destroy (gatt-db.c:291) by 0x85D2B: queue_remove_all (queue.c:354) by 0x91387: gatt_db_clear_range (gatt-db.c:475) --- src/gatt-client.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/gatt-client.c b/src/gatt-client.c index 234f46ed7..55aa5e423 100644 --- a/src/gatt-client.c +++ b/src/gatt-client.c @@ -1645,13 +1645,22 @@ static const GDBusMethodTable characteristic_methods[] = { { } }; +static void remove_client(void *data) +{ + struct notify_client *ntfy_client = data; + struct btd_gatt_client *client = ntfy_client->chrc->service->client; + + queue_remove(client->all_notify_clients, ntfy_client); + + notify_client_unref(ntfy_client); +} + static void characteristic_free(void *data) { struct characteristic *chrc = data; /* List should be empty here */ queue_destroy(chrc->descs, NULL); - queue_destroy(chrc->notify_clients, NULL); if (chrc->write_io) { queue_remove(chrc->service->client->ios, chrc->write_io->io); @@ -1663,6 +1672,8 @@ static void characteristic_free(void *data) pipe_io_destroy(chrc->notify_io); } + queue_destroy(chrc->notify_clients, remove_client); + g_free(chrc->path); free(chrc); } @@ -1715,16 +1726,6 @@ static struct characteristic *characteristic_create( return chrc; } -static void remove_client(void *data) -{ - struct notify_client *ntfy_client = data; - struct btd_gatt_client *client = ntfy_client->chrc->service->client; - - queue_remove(client->all_notify_clients, ntfy_client); - - notify_client_unref(ntfy_client); -} - static void unregister_characteristic(void *data) { struct characteristic *chrc = data; @@ -1738,7 +1739,6 @@ static void unregister_characteristic(void *data) if (chrc->write_op) bt_gatt_client_cancel(gatt, chrc->write_op->id); - queue_remove_all(chrc->notify_clients, NULL, NULL, remove_client); queue_remove_all(chrc->descs, NULL, NULL, unregister_descriptor); g_dbus_unregister_interface(btd_get_dbus_connection(), chrc->path, -- 2.47.3