From cf1de33bc6181894e60bbe2002777c6727c75eed Mon Sep 17 00:00:00 2001 From: Simon Mikuda Date: Tue, 12 Nov 2019 14:56:34 +0100 Subject: [PATCH] core/advertising: Fix crash when unregistering advertisement too fast MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When advertisement is unregistered during MGMT_OP_ADD_ADVERTISING it will crash in add_adv_callback because struct btd_adv_client no longer exist. This is seen also in debug log from bluetoothd: bluetoothd[29698]: src/advertising.c:register_advertisement() RegisterAdvertisement bluetoothd[29698]: src/advertising.c:client_create() Adding proxy for /org/bluez/example/advertisement0 bluetoothd[29698]: src/advertising.c:register_advertisement() Registered advertisement at path /org/bluez/example/advertisement0 bluetoothd[29698]: src/advertising.c:parse_service_uuids() Adding ServiceUUID: 180D bluetoothd[29698]: src/advertising.c:parse_service_uuids() Adding ServiceUUID: 180F bluetoothd[29698]: src/advertising.c:parse_manufacturer_data() Adding ManufacturerData for ffff bluetoothd[29698]: src/advertising.c:parse_service_data() Adding ServiceData for 9999 bluetoothd[29698]: src/advertising.c:parse_data() Adding Data for type 0x26 len 3 bluetoothd[29698]: src/advertising.c:refresh_adv() Refreshing advertisement: /org/bluez/example/advertisement0 bluetoothd[29698]: src/advertising.c:unregister_advertisement() UnregisterAdvertisement bluetoothd[29698]: src/advertising.c:add_adv_callback() Advertisement registered: � Segmentation fault (core dumped) Signed-off-by: Anuj Jain Signed-off-by: Ayush Garg --- src/advertising.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/advertising.c b/src/advertising.c index 27cbad6..f39be7f 100644 --- a/src/advertising.c +++ b/src/advertising.c @@ -68,6 +68,7 @@ struct btd_adv_client { uint16_t discoverable_to; unsigned int to_id; unsigned int disc_to_id; + unsigned int add_adv_id; GDBusClient *client; GDBusProxy *proxy; DBusMessage *reg; @@ -112,6 +113,15 @@ static void client_free(void *data) g_dbus_client_unref(client->client); } + if (client->reg) { + g_dbus_send_message(btd_get_dbus_connection(), + dbus_message_new_method_return(client->reg)); + dbus_message_unref(client->reg); + } + + if (client->add_adv_id) + mgmt_cancel(client->manager->mgmt, client->add_adv_id); + if (client->instance) util_clear_uid(&client->manager->instance_bitmap, client->instance); @@ -760,7 +770,8 @@ static uint8_t *generate_scan_rsp(struct btd_adv_client *client, return bt_ad_generate(client->scan, len); } -static int refresh_adv(struct btd_adv_client *client, mgmt_request_func_t func) +static int refresh_adv(struct btd_adv_client *client, mgmt_request_func_t func, + unsigned int *mgmt_id) { struct mgmt_cp_add_advertising *cp; uint8_t param_len; @@ -769,6 +780,7 @@ static int refresh_adv(struct btd_adv_client *client, mgmt_request_func_t func) uint8_t *scan_rsp; size_t scan_rsp_len = -1; uint32_t flags = 0; + unsigned int mgmt_ret; DBG("Refreshing advertisement: %s", client->path); @@ -817,13 +829,17 @@ static int refresh_adv(struct btd_adv_client *client, mgmt_request_func_t func) free(adv_data); free(scan_rsp); - if (!mgmt_send(client->manager->mgmt, MGMT_OP_ADD_ADVERTISING, - client->manager->mgmt_index, param_len, cp, - func, client, NULL)) { + mgmt_ret = mgmt_send(client->manager->mgmt, MGMT_OP_ADD_ADVERTISING, + client->manager->mgmt_index, param_len, cp, + func, client, NULL); + + if (!mgmt_ret) { error("Failed to add Advertising Data"); free(cp); return -EINVAL; } + if (mgmt_id) + *mgmt_id = mgmt_ret; free(cp); @@ -840,7 +856,7 @@ static gboolean client_discoverable_timeout(void *user_data) bt_ad_clear_flags(client->data); - refresh_adv(client, NULL); + refresh_adv(client, NULL, NULL); return FALSE; } @@ -943,7 +959,7 @@ static void properties_changed(GDBusProxy *proxy, const char *name, continue; if (parser->func(iter, client)) { - refresh_adv(client, NULL); + refresh_adv(client, NULL, NULL); break; } } @@ -975,6 +991,8 @@ static void add_adv_callback(uint8_t status, uint16_t length, struct btd_adv_client *client = user_data; const struct mgmt_rp_add_advertising *rp = param; + client->add_adv_id = 0; + if (status) goto done; @@ -1054,7 +1072,7 @@ static DBusMessage *parse_advertisement(struct btd_adv_client *client) goto fail; } - err = refresh_adv(client, add_adv_callback); + err = refresh_adv(client, add_adv_callback, &client->add_adv_id); if (!err) return NULL; @@ -1444,7 +1462,7 @@ void btd_adv_manager_destroy(struct btd_adv_manager *manager) static void manager_refresh(void *data, void *user_data) { - refresh_adv(data, user_data); + refresh_adv(data, user_data, NULL); } void btd_adv_manager_refresh(struct btd_adv_manager *manager) -- 2.7.4