Hi, Luiz I have tried this, I think the issue is still there with your patch. thanks best wishes yunhan On Mon, Oct 23, 2017 at 1:29 PM, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > Hi Yunhan, > > On Mon, Oct 23, 2017 at 11:26 PM, Yunhan Wang <yunhanw@xxxxxxxxxxxx> wrote: >> Hi Luiz >> >> WIth latest bluez master, I am seeing segmentation fault in bluetoothd >> when ble disconnection happens. I think it related to this patch. >> >> The backtract is as below, >> #0 0x0000000000000000 in ?? () >> #1 0x0000000000481b4d in queue_foreach (queue=0x6fe4e0, >> function=function@entry=0x442d40 <clear_ccc_state>, >> user_data=0x6ee630) at src/shared/queue.c:220 >> #2 0x00000000004436c9 in att_disconnected (err=<optimized out>, >> user_data=0x6fe4b0) at src/gatt-database.c:310 >> #3 0x0000000000481b4d in queue_foreach (queue=0x705b70, >> function=function@entry=0x485280 <disconn_handler>, user_data=0x68) at >> src/shared/queue.c:220 >> #4 0x0000000000486632 in disconnect_cb (io=<optimized out>, >> user_data=0x70b140) at src/shared/att.c:590 >> #5 0x000000000048fbc5 in watch_callback (channel=<optimized out>, >> cond=<optimized out>, user_data=<optimized out>) at >> src/shared/io-glib.c:170 >> #6 0x00007ffff7b1ace5 in g_main_context_dispatch () from >> /lib/x86_64-linux-gnu/libglib-2.0.so.0 >> #7 0x00007ffff7b1b048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0 >> #8 0x00007ffff7b1b30a in g_main_loop_run () from >> /lib/x86_64-linux-gnu/libglib-2.0.so.0 >> #9 0x000000000040b51b in main (argc=1, argv=0x7fffffffe618) at src/main.c:770 > > There is a patch for this issue: > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=261cf78db4be79a0f7d44798a57730b159c9be91 > >> thanks >> best wishes >> yunhan >> >> On Mon, Oct 23, 2017 at 4:17 AM, Luiz Augusto von Dentz >> <luiz.dentz@xxxxxxxxx> wrote: >>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> >>> >>> If the device is no longer valid or is not considered bonded anymore >>> clear its CCC states before removing otherwise application may continue >>> to notify when there are no devices listening. >>> --- >>> src/gatt-database.c | 155 ++++++++++++++++++++++++++++++++++------------------ >>> 1 file changed, 103 insertions(+), 52 deletions(-) >>> >>> diff --git a/src/gatt-database.c b/src/gatt-database.c >>> index 47304704a..6784998c3 100644 >>> --- a/src/gatt-database.c >>> +++ b/src/gatt-database.c >>> @@ -150,8 +150,10 @@ struct pending_op { >>> }; >>> >>> struct device_state { >>> + struct btd_gatt_database *db; >>> bdaddr_t bdaddr; >>> uint8_t bdaddr_type; >>> + unsigned int disc_id; >>> struct queue *ccc_states; >>> }; >>> >>> @@ -245,12 +247,14 @@ static struct ccc_state *find_ccc_state(struct device_state *dev_state, >>> UINT_TO_PTR(handle)); >>> } >>> >>> -static struct device_state *device_state_create(bdaddr_t *bdaddr, >>> +static struct device_state *device_state_create(struct btd_gatt_database *db, >>> + bdaddr_t *bdaddr, >>> uint8_t bdaddr_type) >>> { >>> struct device_state *dev_state; >>> >>> dev_state = new0(struct device_state, 1); >>> + dev_state->db = db; >>> dev_state->ccc_states = queue_new(); >>> bacpy(&dev_state->bdaddr, bdaddr); >>> dev_state->bdaddr_type = bdaddr_type; >>> @@ -258,36 +262,117 @@ static struct device_state *device_state_create(bdaddr_t *bdaddr, >>> return dev_state; >>> } >>> >>> +static void device_state_free(void *data) >>> +{ >>> + struct device_state *state = data; >>> + >>> + queue_destroy(state->ccc_states, free); >>> + free(state); >>> +} >>> + >>> +static void clear_ccc_state(void *data, void *user_data) >>> +{ >>> + struct ccc_state *ccc = data; >>> + struct btd_gatt_database *db = user_data; >>> + struct ccc_cb_data *ccc_cb; >>> + >>> + if (!ccc->value[0]) >>> + return; >>> + >>> + ccc_cb = queue_find(db->ccc_callbacks, ccc_cb_match_handle, >>> + UINT_TO_PTR(ccc->handle)); >>> + if (!ccc_cb) >>> + return; >>> + >>> + ccc_cb->callback(NULL, 0, ccc_cb->user_data); >>> +} >>> + >>> +static void att_disconnected(int err, void *user_data) >>> +{ >>> + struct device_state *state = user_data; >>> + struct btd_device *device; >>> + >>> + DBG(""); >>> + >>> + state->disc_id = 0; >>> + >>> + device = btd_adapter_get_device(state->db->adapter, &state->bdaddr, >>> + state->bdaddr_type); >>> + if (!device) >>> + goto remove; >>> + >>> + if (device_is_paired(device, state->bdaddr_type)) >>> + return; >>> + >>> +remove: >>> + /* Remove device state if device no longer exists or is not paired */ >>> + if (queue_remove(state->db->device_states, state)) { >>> + queue_foreach(state->ccc_states, clear_ccc_state, state->db); >>> + device_state_free(state); >>> + } >>> +} >>> + >>> +static bool get_dst_info(struct bt_att *att, bdaddr_t *dst, uint8_t *dst_type) >>> +{ >>> + GIOChannel *io = NULL; >>> + GError *gerr = NULL; >>> + >>> + io = g_io_channel_unix_new(bt_att_get_fd(att)); >>> + if (!io) >>> + return false; >>> + >>> + bt_io_get(io, &gerr, BT_IO_OPT_DEST_BDADDR, dst, >>> + BT_IO_OPT_DEST_TYPE, dst_type, >>> + BT_IO_OPT_INVALID); >>> + if (gerr) { >>> + error("gatt: bt_io_get: %s", gerr->message); >>> + g_error_free(gerr); >>> + g_io_channel_unref(io); >>> + return false; >>> + } >>> + >>> + g_io_channel_unref(io); >>> + return true; >>> +} >>> + >>> static struct device_state *get_device_state(struct btd_gatt_database *database, >>> - bdaddr_t *bdaddr, >>> - uint8_t bdaddr_type) >>> + struct bt_att *att) >>> { >>> struct device_state *dev_state; >>> + bdaddr_t bdaddr; >>> + uint8_t bdaddr_type; >>> + >>> + if (!get_dst_info(att, &bdaddr, &bdaddr_type)) >>> + return NULL; >>> >>> /* >>> * Find and return a device state. If a matching state doesn't exist, >>> * then create a new one. >>> */ >>> - dev_state = find_device_state(database, bdaddr, bdaddr_type); >>> + dev_state = find_device_state(database, &bdaddr, bdaddr_type); >>> if (dev_state) >>> - return dev_state; >>> + goto done; >>> >>> - dev_state = device_state_create(bdaddr, bdaddr_type); >>> + dev_state = device_state_create(database, &bdaddr, bdaddr_type); >>> >>> queue_push_tail(database->device_states, dev_state); >>> >>> +done: >>> + dev_state->disc_id = bt_att_register_disconnect(att, att_disconnected, >>> + dev_state, NULL); >>> + >>> return dev_state; >>> } >>> >>> static struct ccc_state *get_ccc_state(struct btd_gatt_database *database, >>> - bdaddr_t *bdaddr, >>> - uint8_t bdaddr_type, >>> - uint16_t handle) >>> + struct bt_att *att, uint16_t handle) >>> { >>> struct device_state *dev_state; >>> struct ccc_state *ccc; >>> >>> - dev_state = get_device_state(database, bdaddr, bdaddr_type); >>> + dev_state = get_device_state(database, att); >>> + if (!dev_state) >>> + return NULL; >>> >>> ccc = find_ccc_state(dev_state, handle); >>> if (ccc) >>> @@ -300,14 +385,6 @@ static struct ccc_state *get_ccc_state(struct btd_gatt_database *database, >>> return ccc; >>> } >>> >>> -static void device_state_free(void *data) >>> -{ >>> - struct device_state *state = data; >>> - >>> - queue_destroy(state->ccc_states, free); >>> - free(state); >>> -} >>> - >>> static void cancel_pending_read(void *data) >>> { >>> struct pending_op *op = data; >>> @@ -690,29 +767,6 @@ static void populate_gap_service(struct btd_gatt_database *database) >>> gatt_db_service_set_active(service, true); >>> } >>> >>> -static bool get_dst_info(struct bt_att *att, bdaddr_t *dst, uint8_t *dst_type) >>> -{ >>> - GIOChannel *io = NULL; >>> - GError *gerr = NULL; >>> - >>> - io = g_io_channel_unix_new(bt_att_get_fd(att)); >>> - if (!io) >>> - return false; >>> - >>> - bt_io_get(io, &gerr, BT_IO_OPT_DEST_BDADDR, dst, >>> - BT_IO_OPT_DEST_TYPE, dst_type, >>> - BT_IO_OPT_INVALID); >>> - if (gerr) { >>> - error("gatt: bt_io_get: %s", gerr->message); >>> - g_error_free(gerr); >>> - g_io_channel_unref(io); >>> - return false; >>> - } >>> - >>> - g_io_channel_unref(io); >>> - return true; >>> -} >>> - >>> static void gatt_ccc_read_cb(struct gatt_db_attribute *attrib, >>> unsigned int id, uint16_t offset, >>> uint8_t opcode, struct bt_att *att, >>> @@ -724,8 +778,6 @@ static void gatt_ccc_read_cb(struct gatt_db_attribute *attrib, >>> uint8_t ecode = 0; >>> const uint8_t *value = NULL; >>> size_t len = 0; >>> - bdaddr_t bdaddr; >>> - uint8_t bdaddr_type; >>> >>> handle = gatt_db_attribute_get_handle(attrib); >>> >>> @@ -736,13 +788,12 @@ static void gatt_ccc_read_cb(struct gatt_db_attribute *attrib, >>> goto done; >>> } >>> >>> - if (!get_dst_info(att, &bdaddr, &bdaddr_type)) { >>> + ccc = get_ccc_state(database, att, handle); >>> + if (!ccc) { >>> ecode = BT_ATT_ERROR_UNLIKELY; >>> goto done; >>> } >>> >>> - ccc = get_ccc_state(database, &bdaddr, bdaddr_type, handle); >>> - >>> len = 2 - offset; >>> value = len ? &ccc->value[offset] : NULL; >>> >>> @@ -761,8 +812,6 @@ static void gatt_ccc_write_cb(struct gatt_db_attribute *attrib, >>> struct ccc_cb_data *ccc_cb; >>> uint16_t handle; >>> uint8_t ecode = 0; >>> - bdaddr_t bdaddr; >>> - uint8_t bdaddr_type; >>> >>> handle = gatt_db_attribute_get_handle(attrib); >>> >>> @@ -778,13 +827,12 @@ static void gatt_ccc_write_cb(struct gatt_db_attribute *attrib, >>> goto done; >>> } >>> >>> - if (!get_dst_info(att, &bdaddr, &bdaddr_type)) { >>> + ccc = get_ccc_state(database, att, handle); >>> + if (!ccc) { >>> ecode = BT_ATT_ERROR_UNLIKELY; >>> goto done; >>> } >>> >>> - ccc = get_ccc_state(database, &bdaddr, bdaddr_type, handle); >>> - >>> ccc_cb = queue_find(database->ccc_callbacks, ccc_cb_match_handle, >>> UINT_TO_PTR(gatt_db_attribute_get_handle(attrib))); >>> if (!ccc_cb) { >>> @@ -940,8 +988,11 @@ static void send_notification_to_device(void *data, void *user_data) >>> >>> remove: >>> /* Remove device state if device no longer exists or is not paired */ >>> - if (queue_remove(notify->database->device_states, device_state)) >>> + if (queue_remove(notify->database->device_states, device_state)) { >>> + queue_foreach(device_state->ccc_states, clear_ccc_state, >>> + notify->database); >>> device_state_free(device_state); >>> + } >>> } >>> >>> static void send_notification_to_devices(struct btd_gatt_database *database, >>> -- >>> 2.13.6 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in >>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Luiz Augusto von Dentz -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
