2020-06-01 21:39:55

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ] a2dp: Fix crash on transport_cb

From: Luiz Augusto von Dentz <[email protected]>

There have been reports of crashes on transport_cb where the setup
would most likely already have been freed but transport_cb would still
be called, so instead of assuming the setup pointer would be valid try
to lookup the list of active setups and log a warning when it happens.
---
profiles/audio/a2dp.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 7f14c880a..d88d1fa69 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -2217,6 +2217,14 @@ static void transport_cb(GIOChannel *io, GError *err, gpointer user_data)
{
struct a2dp_setup *setup = user_data;
uint16_t omtu, imtu;
+ GSList *l;
+
+ l = g_slist_find(setups, setup);
+ if (!l) {
+ warn("bt_io_accept: setup %p no longer valid", setup);
+ g_io_channel_shutdown(io, TRUE, NULL);
+ return;
+ }

if (err) {
error("%s", err->message);
--
2.25.3


2020-06-02 17:02:30

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ] a2dp: Fix crash on transport_cb

Hi,

On Mon, Jun 1, 2020 at 2:52 PM Alain Michaud <[email protected]> wrote:
>
> Hi Luiz,
>
> On Mon., Jun. 1, 2020, 5:39 p.m. Luiz Augusto von Dentz, <[email protected]> wrote:
>>
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> There have been reports of crashes on transport_cb where the setup
>> would most likely already have been freed but transport_cb would still
>> be called, so instead of assuming the setup pointer would be valid try
>> to lookup the list of active setups and log a warning when it happens.
>
>
> Reviewed-by: Alain Michaud <[email protected]>
>>
>> ---
>> profiles/audio/a2dp.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
>> index 7f14c880a..d88d1fa69 100644
>> --- a/profiles/audio/a2dp.c
>> +++ b/profiles/audio/a2dp.c
>> @@ -2217,6 +2217,14 @@ static void transport_cb(GIOChannel *io, GError *err, gpointer user_data)
>> {
>> struct a2dp_setup *setup = user_data;
>> uint16_t omtu, imtu;
>> + GSList *l;
>> +
>> + l = g_slist_find(setups, setup);
>> + if (!l) {
>> + warn("bt_io_accept: setup %p no longer valid", setup);
>> + g_io_channel_shutdown(io, TRUE, NULL);
>> + return;
>> + }
>>
>> if (err) {
>> error("%s", err->message);
>> --
>> 2.25.3

Pushed.

--
Luiz Augusto von Dentz

2020-06-02 17:09:46

by Alain Michaud

[permalink] [raw]
Subject: Re: [PATCH BlueZ] a2dp: Fix crash on transport_cb

Thanks Luiz!

On Tue, Jun 2, 2020 at 1:01 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi,
>
> On Mon, Jun 1, 2020 at 2:52 PM Alain Michaud <[email protected]> wrote:
> >
> > Hi Luiz,
> >
> > On Mon., Jun. 1, 2020, 5:39 p.m. Luiz Augusto von Dentz, <[email protected]> wrote:
> >>
> >> From: Luiz Augusto von Dentz <[email protected]>
> >>
> >> There have been reports of crashes on transport_cb where the setup
> >> would most likely already have been freed but transport_cb would still
> >> be called, so instead of assuming the setup pointer would be valid try
> >> to lookup the list of active setups and log a warning when it happens.
> >
> >
> > Reviewed-by: Alain Michaud <[email protected]>
> >>
> >> ---
> >> profiles/audio/a2dp.c | 8 ++++++++
> >> 1 file changed, 8 insertions(+)
> >>
> >> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> >> index 7f14c880a..d88d1fa69 100644
> >> --- a/profiles/audio/a2dp.c
> >> +++ b/profiles/audio/a2dp.c
> >> @@ -2217,6 +2217,14 @@ static void transport_cb(GIOChannel *io, GError *err, gpointer user_data)
> >> {
> >> struct a2dp_setup *setup = user_data;
> >> uint16_t omtu, imtu;
> >> + GSList *l;
> >> +
> >> + l = g_slist_find(setups, setup);
> >> + if (!l) {
> >> + warn("bt_io_accept: setup %p no longer valid", setup);
> >> + g_io_channel_shutdown(io, TRUE, NULL);
> >> + return;
> >> + }
> >>
> >> if (err) {
> >> error("%s", err->message);
> >> --
> >> 2.25.3
>
> Pushed.
>
> --
> Luiz Augusto von Dentz