From: Luiz Augusto von Dentz <[email protected]>
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
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
<[email protected]> wrote:
> Hi Yunhan,
>
> On Wed, Oct 25, 2017 at 10:58 AM, Yunhan Wang <[email protected]> wrote:
>> Hi, Luiz
>>
>> On Tue, Oct 24, 2017 at 1:10 AM, Luiz Augusto von Dentz
>> <[email protected]> wrote:
>>> Hi Yunhan,
>>>
>>> On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <[email protected]> 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
Hi Yunhan,
On Wed, Oct 25, 2017 at 10:58 AM, Yunhan Wang <[email protected]> wrote:
> Hi, Luiz
>
> On Tue, Oct 24, 2017 at 1:10 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Yunhan,
>>
>> On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <[email protected]> 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
Hi, Luiz
On Tue, Oct 24, 2017 at 1:10 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Yunhan,
>
> On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <[email protected]> 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
>> <[email protected]> wrote:
>>> Hi Yunhan,
>>>
>>> On Mon, Oct 23, 2017 at 11:26 PM, Yunhan Wang <[email protected]> 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
>>>> <[email protected]> wrote:
>>>>> From: Luiz Augusto von Dentz <[email protected]>
>>>>>
>>>>> 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 [email protected]
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>>
>>> --
>>> Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz
Hi Yunhan,
On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <[email protected]> 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
> <[email protected]> wrote:
>> Hi Yunhan,
>>
>> On Mon, Oct 23, 2017 at 11:26 PM, Yunhan Wang <[email protected]> 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
>>> <[email protected]> wrote:
>>>> From: Luiz Augusto von Dentz <[email protected]>
>>>>
>>>> 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 [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
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
<[email protected]> wrote:
> Hi Yunhan,
>
> On Mon, Oct 23, 2017 at 11:26 PM, Yunhan Wang <[email protected]> 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
>> <[email protected]> wrote:
>>> From: Luiz Augusto von Dentz <[email protected]>
>>>
>>> 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 [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz
Hi Yunhan,
On Mon, Oct 23, 2017 at 11:26 PM, Yunhan Wang <[email protected]> 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
> <[email protected]> wrote:
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
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
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Luiz Augusto von Dentz <[email protected]>
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
From: Luiz Augusto von Dentz <[email protected]>
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
Hi Yunhan,
On Thu, Nov 2, 2017 at 7:45 PM, Yunhan Wang <[email protected]> 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 <[email protected]> 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
>> <[email protected]> wrote:
>>> Hi Yunhan,
>>>
>>> On Wed, Oct 25, 2017 at 10:58 AM, Yunhan Wang <[email protected]> wrote:
>>>> Hi, Luiz
>>>>
>>>> On Tue, Oct 24, 2017 at 1:10 AM, Luiz Augusto von Dentz
>>>> <[email protected]> wrote:
>>>>> Hi Yunhan,
>>>>>
>>>>> On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <[email protected]> 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
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 <[email protected]> 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
> <[email protected]> wrote:
>> Hi Yunhan,
>>
>> On Wed, Oct 25, 2017 at 10:58 AM, Yunhan Wang <[email protected]> wrote:
>>> Hi, Luiz
>>>
>>> On Tue, Oct 24, 2017 at 1:10 AM, Luiz Augusto von Dentz
>>> <[email protected]> wrote:
>>>> Hi Yunhan,
>>>>
>>>> On Mon, Oct 23, 2017 at 11:59 PM, Yunhan Wang <[email protected]> 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