Re: [PATCH] gatt: Fix double att_disconnected issue on disconnection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Yunhan,

On Thu, Oct 25, 2018 at 9:24 PM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Yunhan,
>
> We might be better of removing the handler altogether and just leave device.c handler instead then.
>
> On Thu, 25 Oct 2018, 20:19 Yunhan Wang, <yunhanw@xxxxxxxxxx> wrote:
>>
>> Hi, Luiz
>>
>> Actually before I submit my patch, I tried your way to unregister the
>> handler, it is failing. That is why I use random address check in
>> btd_gatt_database_att_disconnected to workaround this issue.
>>
>> Thanks
>> Best wishes
>> Yunhan
>>
>>
>>
>> On Thu, Oct 25, 2018 at 10:49 AM Yunhan Wang <yunhanw@xxxxxxxxxx> wrote:
>> >
>> > Hi, Luiz
>> >
>> > Just have a test with your patch in master branch, both crashes are
>> > still there, and att_disconnected has been called for two times even
>> > though unregistering the handler....

Actually you may be missing the following patch:

commit 261cf78db4be79a0f7d44798a57730b159c9be91
Author: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
Date:   Mon Oct 23 14:13:59 2017 +0300

    shared/att: Fix crash when calling disconnect handlers

This is quite old btw, what version is Chrome OS shipping?

>> > Thanks
>> > Best wishes
>> > Yunhan
>> >
>> > Program received signal SIGSEGV, Segmentation fault.
>> > btd_adapter_find_device (adapter=0x72657664612f6372,
>> > dst=dst@entry=0x555555872998, bdaddr_type=0 '\000')
>> >     at bluez/repo/src/adapter.c:845
>> > 845 list = g_slist_find_custom(adapter->devices, &addr,
>> > (gdb) bt
>> > #0  btd_adapter_find_device (adapter=0x72657664612f6372,
>> > dst=dst@entry=0x555555872998, bdaddr_type=0 '\000')
>> >     at bluez/repo/src/adapter.c:845
>> > #1  0x00005555555ab890 in att_disconnected (err=<optimized out>,
>> > user_data=0x555555872990)
>> >     at bluez/repo/src/gatt-database.c:329
>> > #2  0x00005555555eaba8 in queue_foreach (queue=0x55555585de60,
>> > function=function@entry=0x5555555ee5f0 <disconn_handler>,
>> > user_data=0x68)
>> >     at bluez/repo/src/shared/queue.c:220
>> > #3  0x00005555555ef819 in disconnect_cb (io=<optimized out>,
>> > user_data=0x555555869d50)
>> >     at bluez/repo/src/shared/att.c:592
>> > #4  0x00005555555f89a3 in watch_callback (channel=<optimized out>,
>> > cond=<optimized out>, user_data=<optimized out>)
>> >     at bluez/repo/src/shared/io-glib.c:170
>> > #5  0x00007ffff7b0fe35 in g_main_context_dispatch () from
>> > /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> > #6  0x00007ffff7b10200 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> > #7  0x00007ffff7b10512 in g_main_loop_run () from
>> > /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> > #8  0x0000555555572238 in main (argc=<optimized out>, argv=<optimized
>> > out>) at bluez/repo/src/main.c:808
>> >
>> >
>> > Program received signal SIGSEGV, Segmentation fault.
>> > queue_remove (queue=0x30, data=data@entry=0x555555873740) at
>> > bluez/repo/src/shared/queue.c:256
>> > 256 for (entry = queue->head, prev = NULL; entry;
>> > (gdb) bt
>> > #0  queue_remove (queue=0x30, data=data@entry=0x555555873740) at
>> > bluez/repo/src/shared/queue.c:256
>> > #1  0x00005555555ab8c5 in att_disconnected (err=<optimized out>,
>> > user_data=0x555555873740)
>> >     at bluez/repo/src/gatt-database.c:350
>> > #2  0x00005555555eabb8 in queue_foreach (queue=0x55555586e670,
>> > function=function@entry=0x5555555ee600 <disconn_handler>,
>> > user_data=0x68)
>> >     at bluez/repo/src/shared/queue.c:220
>> > #3  0x00005555555ef829 in disconnect_cb (io=<optimized out>,
>> > user_data=0x555555865f50)
>> >     at bluez/repo/src/shared/att.c:592
>> > #4  0x00005555555f89b3 in watch_callback (channel=<optimized out>,
>> > cond=<optimized out>, user_data=<optimized out>)
>> >     at bluez/repo/src/shared/io-glib.c:170
>> > #5  0x00007ffff7b0fe35 in g_main_context_dispatch () from
>> > /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> > #6  0x00007ffff7b10200 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> > #7  0x00007ffff7b10512 in g_main_loop_run () from
>> > /lib/x86_64-linux-gnu/libglib-2.0.so.0
>> > #8  0x0000555555572238 in main (argc=<optimized out>, argv=<optimized
>> > out>) at  bluez/repo/src/main.c:808
>> > On Thu, Oct 25, 2018 at 2:20 AM Luiz Augusto von Dentz
>> > <luiz.dentz@xxxxxxxxx> wrote:
>> > >
>> > > Hi Yunhan,
>> > >
>> > > On Thu, Oct 25, 2018 at 4:47 AM Yunhan Wang <yunhanw@xxxxxxxxxx> wrote:
>> > > >
>> > > > Hi, Luiz
>> > > >
>> > > > I am observing the multiple crashes when doing BLE disconnection using
>> > > > latest bluez master..It looks like the two att_disconnect are
>> > > > triggered from your last gatt commit.. Please help take a look at this
>> > > > workaround and comments.. the better solution might be to figure out
>> > > > how to handle the disconnection along with random address and public
>> > > > address together regarding the previous issue, Gatt: Subscriptions are
>> > > > not cleared after disconnection from a temporary device
>> > >
>> > > Ive pushed a similar fix, it should remove the handler before calling
>> > > att_disconnected.
>> > >
>> > > > Thanks
>> > > > Best wishes
>> > > > Yunhan
>> > > > On Wed, Oct 24, 2018 at 5:42 PM yunhanw <yunhanw@xxxxxxxxxx> wrote:
>> > > > >
>> > > > > When BLE disconnection happens, att_disconnect is triggered from two locations, the new added location is gatt_server_cleanup, it would cause several blueetoothd crashes. This bus is introduced from commit 634f0a6e1125af8d5959bff119d9336a8d81c028, where gatt fix, gatt subscriptions are not cleared after disconnection from a temporary device with private/random address. In order to workaround this issue, btd_gatt_database_att_disconnected can only be triggered when address type is random, and for others, it can continue to use original disconnect code path.
>> > > > >
>> > > > >     crash 1
>> > > > >     Program received signal SIGSEGV, Segmentation fault.
>> > > > >     queue_remove (queue=0x30, data=data@entry=0x555555872a40) at /repo/src/shared/queue.c:256
>> > > > >     256     for (entry = queue->head, prev = NULL; entry;
>> > > > >     (gdb) backtrace
>> > > > >         at /bluez/repo/src/gatt-database.c:350
>> > > > >         at bluez/repo/src/shared/queue.c:220
>> > > > >         at bluez/repo/src/shared/att.c:592
>> > > > >         at bluez/repo/src/shared/io-glib.c:170
>> > > > >
>> > > > >     crash 2
>> > > > >         at bluez/repo/src/shared/queue.c:220
>> > > > >         at bluez/repo/src/shared/att.c:592
>> > > > >         at bluez/repo/src/shared/io-glib.c:170
>> > > > >
>> > > > >     (gdb) print state->db->adapter
>> > > > >     Cannot access memory at address 0x61672f6269727474
>> > > > > ---
>> > > > >  src/gatt-database.c | 2 ++
>> > > > >  1 file changed, 2 insertions(+)
>> > > > >
>> > > > > diff --git a/src/gatt-database.c b/src/gatt-database.c
>> > > > > index 783b692d5..2f0eb83b5 100644
>> > > > > --- a/src/gatt-database.c
>> > > > > +++ b/src/gatt-database.c
>> > > > > @@ -3365,6 +3365,8 @@ void btd_gatt_database_att_disconnected(struct btd_gatt_database *database,
>> > > > >
>> > > > >         addr = device_get_address(device);
>> > > > >         type = btd_device_get_bdaddr_type(device);
>> > > > > +    if (type != BDADDR_LE_RANDOM)
>> > > > > +        return;
>> > > > >
>> > > > >         state = find_device_state(database, addr, type);
>> > > > >         if (!state)
>> > > > > --
>> > > > > 2.19.1.568.g152ad8e336-goog
>> > > > >
>> > >
>> > >
>> > >
>> > > --
>> > > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux