linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ 1/3] shared/att: Fix crash when calling disconnect handlers
@ 2017-10-23 11:17 Luiz Augusto von Dentz
  2017-10-23 11:17 ` [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired Luiz Augusto von Dentz
  2017-10-23 11:17 ` [PATCH BlueZ 3/3] test/example-gatt-server: Don't change measuments if not notifying Luiz Augusto von Dentz
  0 siblings, 2 replies; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-23 11:17 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

When calling disconnect handlers the callback itself may remove items
from the queue causing the following crash:

Invalid read of size 8
  at 0x4D1D3C: queue_foreach (queue.c:219)
  by 0x4D8369: disconnect_cb (att.c:590)
  by 0x4E4FAA: watch_callback (io-glib.c:170)
  by 0x50CD246: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.5200.3)
  by 0x50CD5E7: ??? (in /usr/lib64/libglib-2.0.so.0.5200.3)
  by 0x50CD901: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.5200.3)
  by 0x40CCC0: main (main.c:770)
Address 0x888a888 is 8 bytes inside a block of size 16 free'd
  at 0x4C2FD18: free (vg_replace_malloc.c:530)
  by 0x4D1F9B: queue_remove_if (queue.c:302)
  by 0x4D763B: bt_att_unregister_disconnect (att.c:1206)
  by 0x4DC11E: bt_gatt_client_free (gatt-client.c:1762)
  by 0x4DC270: bt_gatt_client_unref (gatt-client.c:1903)
  by 0x4A316F: gatt_client_cleanup (device.c:573)
  by 0x4A326E: attio_cleanup (device.c:598)
  by 0x4A5EB9: att_disconnected_cb (device.c:4679)
  by 0x4D66D5: disconn_handler (att.c:538)
  by 0x4D1D4F: queue_foreach (queue.c:220)
  by 0x4D8369: disconnect_cb (att.c:590)
  by 0x4E4FAA: watch_callback (io-glib.c:170)
---
 src/shared/att.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index 4670de74f..8d58156c1 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -1203,6 +1203,17 @@ bool bt_att_unregister_disconnect(struct bt_att *att, unsigned int id)
 	if (!att || !id)
 		return false;
 
+	/* Check if disconnect is running */
+	if (!att->io) {
+		disconn = queue_find(att->disconn_list, match_disconn_id,
+							UINT_TO_PTR(id));
+		if (!disconn)
+			return false;
+
+		disconn->removed = true;
+		return true;
+	}
+
 	disconn = queue_remove_if(att->disconn_list, match_disconn_id,
 							UINT_TO_PTR(id));
 	if (!disconn)
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired
  2017-10-23 11:17 [PATCH BlueZ 1/3] shared/att: Fix crash when calling disconnect handlers Luiz Augusto von Dentz
@ 2017-10-23 11:17 ` Luiz Augusto von Dentz
  2017-10-23 20:26   ` Yunhan Wang
  2017-10-23 11:17 ` [PATCH BlueZ 3/3] test/example-gatt-server: Don't change measuments if not notifying Luiz Augusto von Dentz
  1 sibling, 1 reply; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-23 11:17 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH BlueZ 3/3] test/example-gatt-server: Don't change measuments if not notifying
  2017-10-23 11:17 [PATCH BlueZ 1/3] shared/att: Fix crash when calling disconnect handlers Luiz Augusto von Dentz
  2017-10-23 11:17 ` [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired Luiz Augusto von Dentz
@ 2017-10-23 11:17 ` Luiz Augusto von Dentz
  1 sibling, 0 replies; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-23 11:17 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes it clearer when notifications are actually in effect.
---
 test/example-gatt-server | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/example-gatt-server b/test/example-gatt-server
index 24aaff973..689e86ff7 100755
--- a/test/example-gatt-server
+++ b/test/example-gatt-server
@@ -401,6 +401,8 @@ class BatteryLevelCharacteristic(Characteristic):
                 { 'Value': [dbus.Byte(self.battery_lvl)] }, [])
 
     def drain_battery(self):
+        if not self.notifying:
+            return True
         if self.battery_lvl > 0:
             self.battery_lvl -= 2
             if self.battery_lvl < 0:
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired
  2017-10-23 11:17 ` [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired Luiz Augusto von Dentz
@ 2017-10-23 20:26   ` Yunhan Wang
  2017-10-23 20:29     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 12+ messages in thread
From: Yunhan Wang @ 2017-10-23 20:26 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

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

thanks
best wishes
yunhan

On Mon, Oct 23, 2017 at 4:17 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired
  2017-10-23 20:26   ` Yunhan Wang
@ 2017-10-23 20:29     ` Luiz Augusto von Dentz
  2017-10-23 20:59       ` Yunhan Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-23 20:29 UTC (permalink / raw)
  To: Yunhan Wang; +Cc: linux-bluetooth@vger.kernel.org

Hi Yunhan,

On Mon, Oct 23, 2017 at 11:26 PM, Yunhan Wang <yunhanw@nestlabs.com> 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@gmail.com> wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> 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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired
  2017-10-23 20:29     ` Luiz Augusto von Dentz
@ 2017-10-23 20:59       ` Yunhan Wang
  2017-10-24  8:10         ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 12+ messages in thread
From: Yunhan Wang @ 2017-10-23 20:59 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org

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@gmail.com> wrote:
> Hi Yunhan,
>
> On Mon, Oct 23, 2017 at 11:26 PM, Yunhan Wang <yunhanw@nestlabs.com> 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@gmail.com> wrote:
>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>>
>>> 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@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired
  2017-10-23 20:59       ` Yunhan Wang
@ 2017-10-24  8:10         ` Luiz Augusto von Dentz
  2017-10-25  7:58           ` Yunhan Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-24  8:10 UTC (permalink / raw)
  To: Yunhan Wang; +Cc: linux-bluetooth@vger.kernel.org

Hi Yunhan,

On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <yunhanw@nestlabs.com> wrote:
> Hi, Luiz
>
> I have tried this, I think the issue is still there with your patch.

I don't see any problem:

bluetoothd[13968]: src/device.c:att_disconnected_cb() Software caused
connection abort (103)
bluetoothd[13968]: src/gatt-client.c:btd_gatt_client_disconnected()
Device disconnected. Cleaning up.
bluetoothd[13968]: src/device.c:att_disconnected_cb() Automatic
connection disabled
bluetoothd[13968]: attrib/gattrib.c:g_attrib_unref() 0x8897600: g_attrib_unref=0
bluetoothd[13968]: src/gatt-database.c:att_disconnected()
bluetoothd[13968]: src/gatt-database.c:ccc_write_cb() External CCC
write received with value: 0x0000

Could you try running under valgrind?

> thanks
> best wishes
> yunhan
>
> On Mon, Oct 23, 2017 at 1:29 PM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Yunhan,
>>
>> On Mon, Oct 23, 2017 at 11:26 PM, Yunhan Wang <yunhanw@nestlabs.com> 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@gmail.com> wrote:
>>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>>>
>>>> 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@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired
  2017-10-24  8:10         ` Luiz Augusto von Dentz
@ 2017-10-25  7:58           ` Yunhan Wang
  2017-10-25 13:27             ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 12+ messages in thread
From: Yunhan Wang @ 2017-10-25  7:58 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org

Hi, Luiz

On Tue, Oct 24, 2017 at 1:10 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Yunhan,
>
> On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <yunhanw@nestlabs.com> wrote:
>> Hi, Luiz
>>
>> I have tried this, I think the issue is still there with your patch.
>
> I don't see any problem:
>
> bluetoothd[13968]: src/device.c:att_disconnected_cb() Software caused
> connection abort (103)
> bluetoothd[13968]: src/gatt-client.c:btd_gatt_client_disconnected()
> Device disconnected. Cleaning up.
> bluetoothd[13968]: src/device.c:att_disconnected_cb() Automatic
> connection disabled
> bluetoothd[13968]: attrib/gattrib.c:g_attrib_unref() 0x8897600: g_attrib_unref=0
> bluetoothd[13968]: src/gatt-database.c:att_disconnected()
> bluetoothd[13968]: src/gatt-database.c:ccc_write_cb() External CCC
> write received with value: 0x0000
>
> Could you try running under valgrind?
>
Yes, just tried under valgrind, the issue is constantly reprodcued.
Btw, I am currently using btvirt -L -l2, one client, one server, then
bluetoothd crash after client issue ble disconnect. I am using bluez
lateset master.

sudo valgrind ./src/bluetoothd --experimental --debug
==31609== Memcheck, a memory error detector
==31609== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==31609== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==31609== Command: ./src/bluetoothd --experimental --debug
==31609==
==31609== Syscall param socketcall.bind(my_addr.rc_bdaddr) points to
uninitialised byte(s)
==31609==    at 0x588EDA7: bind (syscall-template.S:81)
==31609==    by 0x43A805: logging_open (log.c:76)
==31609==    by 0x43A805: __btd_log_init (log.c:314)
==31609==    by 0x40ADCD: main (main.c:688)
==31609==  Address 0xfff0003d6 is on thread 1's stack
==31609==  in frame #1, created by __btd_log_init (log.c:306)
==31609==
==31609== Syscall param socketcall.bind(my_addr.rc_channel) points to
uninitialised byte(s)
==31609==    at 0x588EDA7: bind (syscall-template.S:81)
==31609==    by 0x43A805: logging_open (log.c:76)
==31609==    by 0x43A805: __btd_log_init (log.c:314)
==31609==    by 0x40ADCD: main (main.c:688)
==31609==  Address 0xfff0003d8 is on thread 1's stack
==31609==  in frame #1, created by __btd_log_init (log.c:306)
==31609==
==31609== Jump to the invalid address stated on the next line
==31609==    at 0x0: ???
==31609==    by 0x481B4C: queue_foreach (queue.c:220)
==31609==    by 0x4436C8: att_disconnected (gatt-database.c:310)
==31609==    by 0x481B4C: queue_foreach (queue.c:220)
==31609==    by 0x486631: disconnect_cb (att.c:590)
==31609==    by 0x48FBC4: watch_callback (io-glib.c:170)
==31609==    by 0x4E7FCE4: g_main_context_dispatch (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
==31609==    by 0x4E80047: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
==31609==    by 0x4E80309: g_main_loop_run (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
==31609==    by 0x40B51A: main (main.c:770)
==31609==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==31609==
==31609==
==31609== Process terminating with default action of signal 11 (SIGSEGV)
==31609==  Bad permissions for mapped region at address 0x0
==31609==    at 0x0: ???
==31609==    by 0x481B4C: queue_foreach (queue.c:220)
==31609==    by 0x4436C8: att_disconnected (gatt-database.c:310)
==31609==    by 0x481B4C: queue_foreach (queue.c:220)
==31609==    by 0x486631: disconnect_cb (att.c:590)
==31609==    by 0x48FBC4: watch_callback (io-glib.c:170)
==31609==    by 0x4E7FCE4: g_main_context_dispatch (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
==31609==    by 0x4E80047: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
==31609==    by 0x4E80309: g_main_loop_run (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
==31609==    by 0x40B51A: main (main.c:770)
==31609==
==31609== HEAP SUMMARY:
==31609==     in use at exit: 131,062 bytes in 2,228 blocks
==31609==   total heap usage: 16,067 allocs, 13,839 frees, 4,489,304
bytes allocated
==31609==
==31609== LEAK SUMMARY:
==31609==    definitely lost: 0 bytes in 0 blocks
==31609==    indirectly lost: 0 bytes in 0 blocks
==31609==      possibly lost: 0 bytes in 0 blocks
==31609==    still reachable: 131,062 bytes in 2,228 blocks
==31609==         suppressed: 0 bytes in 0 blocks
==31609== Rerun with --leak-check=full to see details of leaked memory
==31609==
==31609== For counts of detected and suppressed errors, rerun with: -v
==31609== Use --track-origins=yes to see where uninitialised values come from
==31609== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

thanks
best wishes
yunhan
>> thanks
>> best wishes
>> yunhan
>>
>> On Mon, Oct 23, 2017 at 1:29 PM, Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>>> Hi Yunhan,
>>>
>>> On Mon, Oct 23, 2017 at 11:26 PM, Yunhan Wang <yunhanw@nestlabs.com> 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@gmail.com> wrote:
>>>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>>>>
>>>>> 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@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>>
>>> --
>>> Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired
  2017-10-25  7:58           ` Yunhan Wang
@ 2017-10-25 13:27             ` Luiz Augusto von Dentz
  2017-10-30 21:44               ` Yunhan Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-25 13:27 UTC (permalink / raw)
  To: Yunhan Wang; +Cc: linux-bluetooth@vger.kernel.org

Hi Yunhan,

On Wed, Oct 25, 2017 at 10:58 AM, Yunhan Wang <yunhanw@nestlabs.com> wrote:
> Hi, Luiz
>
> On Tue, Oct 24, 2017 at 1:10 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Yunhan,
>>
>> On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <yunhanw@nestlabs.com> wrote:
>>> Hi, Luiz
>>>
>>> I have tried this, I think the issue is still there with your patch.
>>
>> I don't see any problem:
>>
>> bluetoothd[13968]: src/device.c:att_disconnected_cb() Software caused
>> connection abort (103)
>> bluetoothd[13968]: src/gatt-client.c:btd_gatt_client_disconnected()
>> Device disconnected. Cleaning up.
>> bluetoothd[13968]: src/device.c:att_disconnected_cb() Automatic
>> connection disabled
>> bluetoothd[13968]: attrib/gattrib.c:g_attrib_unref() 0x8897600: g_attrib_unref=0
>> bluetoothd[13968]: src/gatt-database.c:att_disconnected()
>> bluetoothd[13968]: src/gatt-database.c:ccc_write_cb() External CCC
>> write received with value: 0x0000
>>
>> Could you try running under valgrind?
>>
> Yes, just tried under valgrind, the issue is constantly reprodcued.
> Btw, I am currently using btvirt -L -l2, one client, one server, then
> bluetoothd crash after client issue ble disconnect. I am using bluez
> lateset master.
>
> sudo valgrind ./src/bluetoothd --experimental --debug
> ==31609== Jump to the invalid address stated on the next line
> ==31609==    at 0x0: ???
> ==31609==    by 0x481B4C: queue_foreach (queue.c:220)
> ==31609==    by 0x4436C8: att_disconnected (gatt-database.c:310)
> ==31609==    by 0x481B4C: queue_foreach (queue.c:220)
> ==31609==    by 0x486631: disconnect_cb (att.c:590)
> ==31609==    by 0x48FBC4: watch_callback (io-glib.c:170)
> ==31609==    by 0x4E7FCE4: g_main_context_dispatch (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
> ==31609==    by 0x4E80047: ??? (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
> ==31609==    by 0x4E80309: g_main_loop_run (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
> ==31609==    by 0x40B51A: main (main.c:770)
> ==31609==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

It turns out the ccc_cb->callback could be NULL in case of
svc_changed, though I not sure why in our case it did not have the
last call pointing to clear_ccc_state:

      at 0x0: ???
      by 0x475C7C: clear_ccc_state (gatt-database.c:287)
      by 0x4D28CF: queue_foreach (queue.c:220)
      by 0x475FE7: att_disconnected (gatt-database.c:310)
      by 0x4D7255: disconn_handler (att.c:538)
      by 0x4D28CF: queue_foreach (queue.c:220)
      by 0x4D8F39: disconnect_cb (att.c:590)
      by 0x4E6B3A: watch_callback (io-glib.c:170)
      by 0x50CD246: g_main_context_dispatch (in
/usr/lib64/libglib-2.0.so.0.5200.3)
      by 0x50CD5E7: ??? (in /usr/lib64/libglib-2.0.so.0.5200.3)
      by 0x50CD901: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.5200.3)
      by 0x40CD90: main (main.c:770)
    Address 0x0 is not stack'd, malloc'd or (recently) free'd

Anyway now this should fixed upstream, thanks for the report.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired
  2017-10-25 13:27             ` Luiz Augusto von Dentz
@ 2017-10-30 21:44               ` Yunhan Wang
  2017-11-02 17:45                 ` Yunhan Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Yunhan Wang @ 2017-10-30 21:44 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org

Hi, Luiz

I am still consistently seeing multiple segmentation faults happening
on this feature via bluetoothctl. I am using latest bluez master.

Here are the full reproduced details using bluetoothctl
Terminal 1(Bluetoothd):
sudo gdb --args ./src/bluetoothd --experimental --debug
run

Terminal 2(btvirt 2 interface):
sudo ./emulator/btvirt -L -l2

Terminal 3(Peripheral):
select 00:AA:01:00:00:23
power on
register-service 00001820-0000-1000-8000-00805f9b34fb
register-characteristic 00002a06-0000-1000-8000-00805f9b34fb read,indicate
register-application
advertise on

Terminal 4(Central):
select 00:AA:01:01:00:24
power on
scan on
scan off
connect 00:AA:01:00:00:23
select-attribute /org/bluez/hci1/dev_00_AA_01_00_00_23/service000a/char000b
notify on
disconnect

Then one of segmentation faults happen

Program received signal SIGSEGV, Segmentation fault.
queue_remove (queue=0x30, data=data@entry=0x70b6c0) at src/shared/queue.c:256
256 for (entry = queue->head, prev = NULL; entry;
(gdb) bt
#0  queue_remove (queue=0x30, data=data@entry=0x70b6c0)
    at src/shared/queue.c:256
#1  0x00000000004437bb in att_disconnected (err=<optimized out>,
    user_data=0x70b6c0) at src/gatt-database.c:310
#2  0x0000000000481d6d in queue_foreach (queue=0x6f1d50,
    function=function@entry=0x4854a0 <disconn_handler>, user_data=0x68)
    at src/shared/queue.c:220
#3  0x0000000000486852 in disconnect_cb (io=<optimized out>,
    user_data=0x704ce0) at src/shared/att.c:590
#4  0x000000000048fde5 in watch_callback (channel=<optimized out>,
    cond=<optimized out>, user_data=<optimized out>)
    at src/shared/io-glib.c:170
#5  0x00007ffff7b1ace5 in g_main_context_dispatch ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#6  0x00007ffff7b1b048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#7  0x00007ffff7b1b30a in g_main_loop_run ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#8  0x000000000040b56b in main (argc=1, argv=0x7fffffffe618) at src/main.c:770

some of other faults:
1.
Program received signal SIGSEGV, Segmentation fault.
att_disconnected (err=<optimized out>, user_data=0x703270)
    at src/gatt-database.c:300
300 if (!state->db->adapter)
(gdb)
(gdb) bt
#0  att_disconnected (err=<optimized out>, user_data=0x703270)
    at src/gatt-database.c:300
#1  0x0000000000481ccd in queue_foreach (queue=0x6fd7e0,
    function=function@entry=0x485400 <disconn_handler>, user_data=0x68)
    at src/shared/queue.c:220
#2  0x00000000004867b2 in disconnect_cb (io=<optimized out>,
    user_data=0x6ee310) at src/shared/att.c:590
#3  0x000000000048fd45 in watch_callback (channel=<optimized out>,
    cond=<optimized out>, user_data=<optimized out>)
    at src/shared/io-glib.c:170
#4  0x00007ffff7b1ace5 in g_main_context_dispatch ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#5  0x00007ffff7b1b048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#6  0x00007ffff7b1b30a in g_main_loop_run ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#7  0x000000000040b56b in main (argc=1, argv=0x7fffffffe618) at src/main.c:770
(gdb) print state
$1 = (struct device_state *) 0x703270
(gdb) print state->db
$2 = (struct btd_gatt_database *) 0x0

2.
Program received signal SIGSEGV, Segmentation fault.
btd_adapter_find_device (adapter=0x2a, dst=dst@entry=0x70a2e8,
    bdaddr_type=0 '\000') at src/adapter.c:821
821 list = g_slist_find_custom(adapter->devices, &addr,
(gdb) quit


Thanks
Best wishes
Yunhan

On Wed, Oct 25, 2017 at 6:27 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Yunhan,
>
> On Wed, Oct 25, 2017 at 10:58 AM, Yunhan Wang <yunhanw@nestlabs.com> wrote:
>> Hi, Luiz
>>
>> On Tue, Oct 24, 2017 at 1:10 AM, Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>>> Hi Yunhan,
>>>
>>> On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <yunhanw@nestlabs.com> wrote:
>>>> Hi, Luiz
>>>>
>>>> I have tried this, I think the issue is still there with your patch.
>>>
>>> I don't see any problem:
>>>
>>> bluetoothd[13968]: src/device.c:att_disconnected_cb() Software caused
>>> connection abort (103)
>>> bluetoothd[13968]: src/gatt-client.c:btd_gatt_client_disconnected()
>>> Device disconnected. Cleaning up.
>>> bluetoothd[13968]: src/device.c:att_disconnected_cb() Automatic
>>> connection disabled
>>> bluetoothd[13968]: attrib/gattrib.c:g_attrib_unref() 0x8897600: g_attrib_unref=0
>>> bluetoothd[13968]: src/gatt-database.c:att_disconnected()
>>> bluetoothd[13968]: src/gatt-database.c:ccc_write_cb() External CCC
>>> write received with value: 0x0000
>>>
>>> Could you try running under valgrind?
>>>
>> Yes, just tried under valgrind, the issue is constantly reprodcued.
>> Btw, I am currently using btvirt -L -l2, one client, one server, then
>> bluetoothd crash after client issue ble disconnect. I am using bluez
>> lateset master.
>>
>> sudo valgrind ./src/bluetoothd --experimental --debug
>> ==31609== Jump to the invalid address stated on the next line
>> ==31609==    at 0x0: ???
>> ==31609==    by 0x481B4C: queue_foreach (queue.c:220)
>> ==31609==    by 0x4436C8: att_disconnected (gatt-database.c:310)
>> ==31609==    by 0x481B4C: queue_foreach (queue.c:220)
>> ==31609==    by 0x486631: disconnect_cb (att.c:590)
>> ==31609==    by 0x48FBC4: watch_callback (io-glib.c:170)
>> ==31609==    by 0x4E7FCE4: g_main_context_dispatch (in
>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
>> ==31609==    by 0x4E80047: ??? (in
>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
>> ==31609==    by 0x4E80309: g_main_loop_run (in
>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
>> ==31609==    by 0x40B51A: main (main.c:770)
>> ==31609==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
>
> It turns out the ccc_cb->callback could be NULL in case of
> svc_changed, though I not sure why in our case it did not have the
> last call pointing to clear_ccc_state:
>
>       at 0x0: ???
>       by 0x475C7C: clear_ccc_state (gatt-database.c:287)
>       by 0x4D28CF: queue_foreach (queue.c:220)
>       by 0x475FE7: att_disconnected (gatt-database.c:310)
>       by 0x4D7255: disconn_handler (att.c:538)
>       by 0x4D28CF: queue_foreach (queue.c:220)
>       by 0x4D8F39: disconnect_cb (att.c:590)
>       by 0x4E6B3A: watch_callback (io-glib.c:170)
>       by 0x50CD246: g_main_context_dispatch (in
> /usr/lib64/libglib-2.0.so.0.5200.3)
>       by 0x50CD5E7: ??? (in /usr/lib64/libglib-2.0.so.0.5200.3)
>       by 0x50CD901: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.5200.3)
>       by 0x40CD90: main (main.c:770)
>     Address 0x0 is not stack'd, malloc'd or (recently) free'd
>
> Anyway now this should fixed upstream, thanks for the report.
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired
  2017-10-30 21:44               ` Yunhan Wang
@ 2017-11-02 17:45                 ` Yunhan Wang
  2017-11-02 18:07                   ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 12+ messages in thread
From: Yunhan Wang @ 2017-11-02 17:45 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org

Hi Luiz

It is working now with your latest fix.

Thanks
Best wishes
Yunhan

On Mon, Oct 30, 2017 at 2:44 PM, Yunhan Wang <yunhanw@google.com> wrote:
> Hi, Luiz
>
> I am still consistently seeing multiple segmentation faults happening
> on this feature via bluetoothctl. I am using latest bluez master.
>
> Here are the full reproduced details using bluetoothctl
> Terminal 1(Bluetoothd):
> sudo gdb --args ./src/bluetoothd --experimental --debug
> run
>
> Terminal 2(btvirt 2 interface):
> sudo ./emulator/btvirt -L -l2
>
> Terminal 3(Peripheral):
> select 00:AA:01:00:00:23
> power on
> register-service 00001820-0000-1000-8000-00805f9b34fb
> register-characteristic 00002a06-0000-1000-8000-00805f9b34fb read,indicate
> register-application
> advertise on
>
> Terminal 4(Central):
> select 00:AA:01:01:00:24
> power on
> scan on
> scan off
> connect 00:AA:01:00:00:23
> select-attribute /org/bluez/hci1/dev_00_AA_01_00_00_23/service000a/char000b
> notify on
> disconnect
>
> Then one of segmentation faults happen
>
> Program received signal SIGSEGV, Segmentation fault.
> queue_remove (queue=0x30, data=data@entry=0x70b6c0) at src/shared/queue.c:256
> 256 for (entry = queue->head, prev = NULL; entry;
> (gdb) bt
> #0  queue_remove (queue=0x30, data=data@entry=0x70b6c0)
>     at src/shared/queue.c:256
> #1  0x00000000004437bb in att_disconnected (err=<optimized out>,
>     user_data=0x70b6c0) at src/gatt-database.c:310
> #2  0x0000000000481d6d in queue_foreach (queue=0x6f1d50,
>     function=function@entry=0x4854a0 <disconn_handler>, user_data=0x68)
>     at src/shared/queue.c:220
> #3  0x0000000000486852 in disconnect_cb (io=<optimized out>,
>     user_data=0x704ce0) at src/shared/att.c:590
> #4  0x000000000048fde5 in watch_callback (channel=<optimized out>,
>     cond=<optimized out>, user_data=<optimized out>)
>     at src/shared/io-glib.c:170
> #5  0x00007ffff7b1ace5 in g_main_context_dispatch ()
>    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #6  0x00007ffff7b1b048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #7  0x00007ffff7b1b30a in g_main_loop_run ()
>    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #8  0x000000000040b56b in main (argc=1, argv=0x7fffffffe618) at src/main.c:770
>
> some of other faults:
> 1.
> Program received signal SIGSEGV, Segmentation fault.
> att_disconnected (err=<optimized out>, user_data=0x703270)
>     at src/gatt-database.c:300
> 300 if (!state->db->adapter)
> (gdb)
> (gdb) bt
> #0  att_disconnected (err=<optimized out>, user_data=0x703270)
>     at src/gatt-database.c:300
> #1  0x0000000000481ccd in queue_foreach (queue=0x6fd7e0,
>     function=function@entry=0x485400 <disconn_handler>, user_data=0x68)
>     at src/shared/queue.c:220
> #2  0x00000000004867b2 in disconnect_cb (io=<optimized out>,
>     user_data=0x6ee310) at src/shared/att.c:590
> #3  0x000000000048fd45 in watch_callback (channel=<optimized out>,
>     cond=<optimized out>, user_data=<optimized out>)
>     at src/shared/io-glib.c:170
> #4  0x00007ffff7b1ace5 in g_main_context_dispatch ()
>    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #5  0x00007ffff7b1b048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #6  0x00007ffff7b1b30a in g_main_loop_run ()
>    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #7  0x000000000040b56b in main (argc=1, argv=0x7fffffffe618) at src/main.c:770
> (gdb) print state
> $1 = (struct device_state *) 0x703270
> (gdb) print state->db
> $2 = (struct btd_gatt_database *) 0x0
>
> 2.
> Program received signal SIGSEGV, Segmentation fault.
> btd_adapter_find_device (adapter=0x2a, dst=dst@entry=0x70a2e8,
>     bdaddr_type=0 '\000') at src/adapter.c:821
> 821 list = g_slist_find_custom(adapter->devices, &addr,
> (gdb) quit
>
>
> Thanks
> Best wishes
> Yunhan
>
> On Wed, Oct 25, 2017 at 6:27 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Yunhan,
>>
>> On Wed, Oct 25, 2017 at 10:58 AM, Yunhan Wang <yunhanw@nestlabs.com> wrote:
>>> Hi, Luiz
>>>
>>> On Tue, Oct 24, 2017 at 1:10 AM, Luiz Augusto von Dentz
>>> <luiz.dentz@gmail.com> wrote:
>>>> Hi Yunhan,
>>>>
>>>> On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <yunhanw@nestlabs.com> wrote:
>>>>> Hi, Luiz
>>>>>
>>>>> I have tried this, I think the issue is still there with your patch.
>>>>
>>>> I don't see any problem:
>>>>
>>>> bluetoothd[13968]: src/device.c:att_disconnected_cb() Software caused
>>>> connection abort (103)
>>>> bluetoothd[13968]: src/gatt-client.c:btd_gatt_client_disconnected()
>>>> Device disconnected. Cleaning up.
>>>> bluetoothd[13968]: src/device.c:att_disconnected_cb() Automatic
>>>> connection disabled
>>>> bluetoothd[13968]: attrib/gattrib.c:g_attrib_unref() 0x8897600: g_attrib_unref=0
>>>> bluetoothd[13968]: src/gatt-database.c:att_disconnected()
>>>> bluetoothd[13968]: src/gatt-database.c:ccc_write_cb() External CCC
>>>> write received with value: 0x0000
>>>>
>>>> Could you try running under valgrind?
>>>>
>>> Yes, just tried under valgrind, the issue is constantly reprodcued.
>>> Btw, I am currently using btvirt -L -l2, one client, one server, then
>>> bluetoothd crash after client issue ble disconnect. I am using bluez
>>> lateset master.
>>>
>>> sudo valgrind ./src/bluetoothd --experimental --debug
>>> ==31609== Jump to the invalid address stated on the next line
>>> ==31609==    at 0x0: ???
>>> ==31609==    by 0x481B4C: queue_foreach (queue.c:220)
>>> ==31609==    by 0x4436C8: att_disconnected (gatt-database.c:310)
>>> ==31609==    by 0x481B4C: queue_foreach (queue.c:220)
>>> ==31609==    by 0x486631: disconnect_cb (att.c:590)
>>> ==31609==    by 0x48FBC4: watch_callback (io-glib.c:170)
>>> ==31609==    by 0x4E7FCE4: g_main_context_dispatch (in
>>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
>>> ==31609==    by 0x4E80047: ??? (in
>>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
>>> ==31609==    by 0x4E80309: g_main_loop_run (in
>>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
>>> ==31609==    by 0x40B51A: main (main.c:770)
>>> ==31609==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
>>
>> It turns out the ccc_cb->callback could be NULL in case of
>> svc_changed, though I not sure why in our case it did not have the
>> last call pointing to clear_ccc_state:
>>
>>       at 0x0: ???
>>       by 0x475C7C: clear_ccc_state (gatt-database.c:287)
>>       by 0x4D28CF: queue_foreach (queue.c:220)
>>       by 0x475FE7: att_disconnected (gatt-database.c:310)
>>       by 0x4D7255: disconn_handler (att.c:538)
>>       by 0x4D28CF: queue_foreach (queue.c:220)
>>       by 0x4D8F39: disconnect_cb (att.c:590)
>>       by 0x4E6B3A: watch_callback (io-glib.c:170)
>>       by 0x50CD246: g_main_context_dispatch (in
>> /usr/lib64/libglib-2.0.so.0.5200.3)
>>       by 0x50CD5E7: ??? (in /usr/lib64/libglib-2.0.so.0.5200.3)
>>       by 0x50CD901: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.5200.3)
>>       by 0x40CD90: main (main.c:770)
>>     Address 0x0 is not stack'd, malloc'd or (recently) free'd
>>
>> Anyway now this should fixed upstream, thanks for the report.
>>
>> --
>> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired
  2017-11-02 17:45                 ` Yunhan Wang
@ 2017-11-02 18:07                   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2017-11-02 18:07 UTC (permalink / raw)
  To: Yunhan Wang; +Cc: linux-bluetooth@vger.kernel.org

Hi Yunhan,

On Thu, Nov 2, 2017 at 7:45 PM, Yunhan Wang <yunhanw@google.com> wrote:
> Hi Luiz
>
> It is working now with your latest fix.

Great, hopefully this will settle things for server notifications.

> Thanks
> Best wishes
> Yunhan
>
> On Mon, Oct 30, 2017 at 2:44 PM, Yunhan Wang <yunhanw@google.com> wrote:
>> Hi, Luiz
>>
>> I am still consistently seeing multiple segmentation faults happening
>> on this feature via bluetoothctl. I am using latest bluez master.
>>
>> Here are the full reproduced details using bluetoothctl
>> Terminal 1(Bluetoothd):
>> sudo gdb --args ./src/bluetoothd --experimental --debug
>> run
>>
>> Terminal 2(btvirt 2 interface):
>> sudo ./emulator/btvirt -L -l2
>>
>> Terminal 3(Peripheral):
>> select 00:AA:01:00:00:23
>> power on
>> register-service 00001820-0000-1000-8000-00805f9b34fb
>> register-characteristic 00002a06-0000-1000-8000-00805f9b34fb read,indicate
>> register-application
>> advertise on
>>
>> Terminal 4(Central):
>> select 00:AA:01:01:00:24
>> power on
>> scan on
>> scan off
>> connect 00:AA:01:00:00:23
>> select-attribute /org/bluez/hci1/dev_00_AA_01_00_00_23/service000a/char000b
>> notify on
>> disconnect
>>
>> Then one of segmentation faults happen
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> queue_remove (queue=0x30, data=data@entry=0x70b6c0) at src/shared/queue.c:256
>> 256 for (entry = queue->head, prev = NULL; entry;
>> (gdb) bt
>> #0  queue_remove (queue=0x30, data=data@entry=0x70b6c0)
>>     at src/shared/queue.c:256
>> #1  0x00000000004437bb in att_disconnected (err=<optimized out>,
>>     user_data=0x70b6c0) at src/gatt-database.c:310
>> #2  0x0000000000481d6d in queue_foreach (queue=0x6f1d50,
>>     function=function@entry=0x4854a0 <disconn_handler>, user_data=0x68)
>>     at src/shared/queue.c:220
>> #3  0x0000000000486852 in disconnect_cb (io=<optimized out>,
>>     user_data=0x704ce0) at src/shared/att.c:590
>> #4  0x000000000048fde5 in watch_callback (channel=<optimized out>,
>>     cond=<optimized out>, user_data=<optimized out>)
>>     at src/shared/io-glib.c:170
>> #5  0x00007ffff7b1ace5 in g_main_context_dispatch ()
>>    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> #6  0x00007ffff7b1b048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> #7  0x00007ffff7b1b30a in g_main_loop_run ()
>>    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> #8  0x000000000040b56b in main (argc=1, argv=0x7fffffffe618) at src/main.c:770
>>
>> some of other faults:
>> 1.
>> Program received signal SIGSEGV, Segmentation fault.
>> att_disconnected (err=<optimized out>, user_data=0x703270)
>>     at src/gatt-database.c:300
>> 300 if (!state->db->adapter)
>> (gdb)
>> (gdb) bt
>> #0  att_disconnected (err=<optimized out>, user_data=0x703270)
>>     at src/gatt-database.c:300
>> #1  0x0000000000481ccd in queue_foreach (queue=0x6fd7e0,
>>     function=function@entry=0x485400 <disconn_handler>, user_data=0x68)
>>     at src/shared/queue.c:220
>> #2  0x00000000004867b2 in disconnect_cb (io=<optimized out>,
>>     user_data=0x6ee310) at src/shared/att.c:590
>> #3  0x000000000048fd45 in watch_callback (channel=<optimized out>,
>>     cond=<optimized out>, user_data=<optimized out>)
>>     at src/shared/io-glib.c:170
>> #4  0x00007ffff7b1ace5 in g_main_context_dispatch ()
>>    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> #5  0x00007ffff7b1b048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> #6  0x00007ffff7b1b30a in g_main_loop_run ()
>>    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> #7  0x000000000040b56b in main (argc=1, argv=0x7fffffffe618) at src/main.c:770
>> (gdb) print state
>> $1 = (struct device_state *) 0x703270
>> (gdb) print state->db
>> $2 = (struct btd_gatt_database *) 0x0
>>
>> 2.
>> Program received signal SIGSEGV, Segmentation fault.
>> btd_adapter_find_device (adapter=0x2a, dst=dst@entry=0x70a2e8,
>>     bdaddr_type=0 '\000') at src/adapter.c:821
>> 821 list = g_slist_find_custom(adapter->devices, &addr,
>> (gdb) quit
>>
>>
>> Thanks
>> Best wishes
>> Yunhan
>>
>> On Wed, Oct 25, 2017 at 6:27 AM, Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>>> Hi Yunhan,
>>>
>>> On Wed, Oct 25, 2017 at 10:58 AM, Yunhan Wang <yunhanw@nestlabs.com> wrote:
>>>> Hi, Luiz
>>>>
>>>> On Tue, Oct 24, 2017 at 1:10 AM, Luiz Augusto von Dentz
>>>> <luiz.dentz@gmail.com> wrote:
>>>>> Hi Yunhan,
>>>>>
>>>>> On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <yunhanw@nestlabs.com> wrote:
>>>>>> Hi, Luiz
>>>>>>
>>>>>> I have tried this, I think the issue is still there with your patch.
>>>>>
>>>>> I don't see any problem:
>>>>>
>>>>> bluetoothd[13968]: src/device.c:att_disconnected_cb() Software caused
>>>>> connection abort (103)
>>>>> bluetoothd[13968]: src/gatt-client.c:btd_gatt_client_disconnected()
>>>>> Device disconnected. Cleaning up.
>>>>> bluetoothd[13968]: src/device.c:att_disconnected_cb() Automatic
>>>>> connection disabled
>>>>> bluetoothd[13968]: attrib/gattrib.c:g_attrib_unref() 0x8897600: g_attrib_unref=0
>>>>> bluetoothd[13968]: src/gatt-database.c:att_disconnected()
>>>>> bluetoothd[13968]: src/gatt-database.c:ccc_write_cb() External CCC
>>>>> write received with value: 0x0000
>>>>>
>>>>> Could you try running under valgrind?
>>>>>
>>>> Yes, just tried under valgrind, the issue is constantly reprodcued.
>>>> Btw, I am currently using btvirt -L -l2, one client, one server, then
>>>> bluetoothd crash after client issue ble disconnect. I am using bluez
>>>> lateset master.
>>>>
>>>> sudo valgrind ./src/bluetoothd --experimental --debug
>>>> ==31609== Jump to the invalid address stated on the next line
>>>> ==31609==    at 0x0: ???
>>>> ==31609==    by 0x481B4C: queue_foreach (queue.c:220)
>>>> ==31609==    by 0x4436C8: att_disconnected (gatt-database.c:310)
>>>> ==31609==    by 0x481B4C: queue_foreach (queue.c:220)
>>>> ==31609==    by 0x486631: disconnect_cb (att.c:590)
>>>> ==31609==    by 0x48FBC4: watch_callback (io-glib.c:170)
>>>> ==31609==    by 0x4E7FCE4: g_main_context_dispatch (in
>>>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
>>>> ==31609==    by 0x4E80047: ??? (in
>>>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
>>>> ==31609==    by 0x4E80309: g_main_loop_run (in
>>>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
>>>> ==31609==    by 0x40B51A: main (main.c:770)
>>>> ==31609==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
>>>
>>> It turns out the ccc_cb->callback could be NULL in case of
>>> svc_changed, though I not sure why in our case it did not have the
>>> last call pointing to clear_ccc_state:
>>>
>>>       at 0x0: ???
>>>       by 0x475C7C: clear_ccc_state (gatt-database.c:287)
>>>       by 0x4D28CF: queue_foreach (queue.c:220)
>>>       by 0x475FE7: att_disconnected (gatt-database.c:310)
>>>       by 0x4D7255: disconn_handler (att.c:538)
>>>       by 0x4D28CF: queue_foreach (queue.c:220)
>>>       by 0x4D8F39: disconnect_cb (att.c:590)
>>>       by 0x4E6B3A: watch_callback (io-glib.c:170)
>>>       by 0x50CD246: g_main_context_dispatch (in
>>> /usr/lib64/libglib-2.0.so.0.5200.3)
>>>       by 0x50CD5E7: ??? (in /usr/lib64/libglib-2.0.so.0.5200.3)
>>>       by 0x50CD901: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.5200.3)
>>>       by 0x40CD90: main (main.c:770)
>>>     Address 0x0 is not stack'd, malloc'd or (recently) free'd
>>>
>>> Anyway now this should fixed upstream, thanks for the report.
>>>
>>> --
>>> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-11-02 18:07 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-23 11:17 [PATCH BlueZ 1/3] shared/att: Fix crash when calling disconnect handlers Luiz Augusto von Dentz
2017-10-23 11:17 ` [PATCH BlueZ 2/3] gatt: Clear subscriptions for device not paired Luiz Augusto von Dentz
2017-10-23 20:26   ` Yunhan Wang
2017-10-23 20:29     ` Luiz Augusto von Dentz
2017-10-23 20:59       ` Yunhan Wang
2017-10-24  8:10         ` Luiz Augusto von Dentz
2017-10-25  7:58           ` Yunhan Wang
2017-10-25 13:27             ` Luiz Augusto von Dentz
2017-10-30 21:44               ` Yunhan Wang
2017-11-02 17:45                 ` Yunhan Wang
2017-11-02 18:07                   ` Luiz Augusto von Dentz
2017-10-23 11:17 ` [PATCH BlueZ 3/3] test/example-gatt-server: Don't change measuments if not notifying Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).