Hi Howard,
On Mon, Mar 23, 2020 at 2:43 AM Yun-hao Chung <howardchung@xxxxxxxxxx> wrote:
>
> Hi Luiz,
>
> I can trigger the crash by running a connect disconnect loop. What I've found is that crashes always happened when sep is assigned to stream->lsep but stream is NULL. If I apply the change above, there are no such crashes anymore. So I'm still believing it is caused by stream being NULL instead of stream->lsep being NULL. Do I miss something?
>
> One thing to add:
> Perhaps it would be more clear if we use stream->lsep directly when calling avdtp_sep_set_state.
>
> Here is the stack trace:
>
> Crash reason: SIGSEGV /0x00000000
> Crash address: 0x18
> Process uptime: not available
>
> Thread 0 (crashed)
> 0 bluetoothd!avdtp_abort [avdtp.c : 3487 + 0x0]
> 1 bluetoothd!a2dp_cancel [a2dp.c : 2180 + 0x5]
> 2 bluetoothd!sink_disconnect [sink.c : 404 + 0x5]
> 3 bluetoothd!btd_service_disconnect [service.c : 293 + 0x5]
> 4 libglib-2.0.so.0!g_list_foreach [glist.c : 1013 + 0x6]
> 5 bluetoothd!device_request_disconnect [device.c : 1500 + 0xe]
> 6 bluetoothd!dev_disconnect [device.c : 1652 + 0xb]
> 7 bluetoothd!generic_message [object.c : 259 + 0x8]
> 8 libdbus-1.so.3!_dbus_object_tree_dispatch_and_unlock [dbus-object-tree.c : 1020 + 0x5]
> 9 libdbus-1.so.3!dbus_connection_dispatch [dbus-connection.c : 4750 + 0x8]
> 10 bluetoothd!message_dispatch [mainloop.c : 72 + 0x8]
> 11 libglib-2.0.so.0!g_main_context_dispatch [gmain.c : 3182 + 0x2]
> 12 libglib-2.0.so.0!g_main_context_iterate [gmain.c : 3920 + 0x8]
> 13 libglib-2.0.so.0!g_main_loop_run [gmain.c : 4116 + 0xf]
> 14 bluetoothd!main [main.c : 800 + 0x5]
> 15 libc.so.6!__libc_start_main [libc-start.c : 308 + 0x1a]
> 16 bluetoothd!_start + 0x2a
I see so the setup->stream is stil NULL in this case so trying to
access stream->lsep will fail.
>
> Thanks,
> Howard
>
> On Fri, Mar 6, 2020 at 3:17 AM Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote:
>>
>> Hi Howard,
>>
>> On Thu, Mar 5, 2020 at 3:06 AM Howard Chung <howardchung@xxxxxxxxxx> wrote:
>> >
>> > Initialized avdtp_local_sep later since stream could be NULL.
>> > ---
>> >
>> > profiles/audio/avdtp.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
>> > index 0e075f9ff..12d984866 100644
>> > --- a/profiles/audio/avdtp.c
>> > +++ b/profiles/audio/avdtp.c
>> > @@ -3566,7 +3566,7 @@ int avdtp_abort(struct avdtp *session, struct avdtp_stream *stream)
>> > {
>> > struct seid_req req;
>> > int ret;
>> > - struct avdtp_local_sep *sep = stream->lsep;
>> > + struct avdtp_local_sep *sep;
Lets just remove this variable for here.
>> > if (!stream && session->discover) {
>> > /* Don't call cb since it being aborted */
>> > @@ -3581,6 +3581,7 @@ int avdtp_abort(struct avdtp *session, struct avdtp_stream *stream)
>> > if (stream->lsep->state == AVDTP_STATE_ABORTING)
>> > return -EINVAL;
>>
>> I suspect there i something else going on then just the lsep being
>> NULL since we do check it on the line above it would have crashed
>> anyway, is this perhaps the result of lsep being unregistered before
>> the avdtp_abort is called?
>>
>> > + sep = stream->lsep;
>> > avdtp_sep_set_state(session, sep, AVDTP_STATE_ABORTING);
Just use stream->lsep here since at this point we already verified
that stream != NULL and even attempted to read its state.
>> >
>> > if (session->req && stream == session->req->stream)
>> > --
>> > 2.25.0.265.gbab2e86ba0-goog
>> >
>>
>>
>> --
>> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
