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 Fri, Oct 26, 2018 at 12:06 AM Yunhan Wang <yunhanw@xxxxxxxxxx> wrote:
>
> Hi, Luiz
>
> I am using latest bluez master without any change for this issue, I
> think I am not missing any changes....The issue is there.

Then we have a problem on bt_att, but that is tracking if the handler
is removed so I wonder how it is still reproducible for you.

> It is not for chrome os. I am currently trying bring up bluez version
> from commit in Jan 19 11:37:07 2018 to latest master in Open weave
> project(https://github.com/openweave/openweave-core/blob/master/repos.conf),
> where we are using BLE for weave pairing in iot products, and create
> two GATT characteristics for Tx and Rx and the TCP-like control
> protocol  to control BLE packet flow. Periodically I would sync Bluez
> revision in openweave against Bluez Upstream.

All major mobile OS support LE L2CAP CoC channels, no idea why
companies want to keep using GATT for emulating serial like
communication special when L2CAP does have support for fragmentation
and flow control.

> Thanks
> Best wishes
> Yunhan
>
> On Thu, Oct 25, 2018 at 1:22 PM Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
> >
> > 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



-- 
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