Re: a2dp.c: finalize_config(setup) can destroy setup

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

 



Hi Alex,

On Thu, May 16, 2013 at 7:06 AM, Alex Deymo <deymo@xxxxxxxxxxxx> wrote:
> Hello,
>
> I just found this invalid read where finalize_config(setup); is
> actually destroying the setup pointer passed there. The log attached
> below is running with valgrind on bluez 5.5 (with a tiny patch to
> src/log.{c|h} to have the debug file:line:function on error() as you
> may see on the logs)
>
> The code around the open_cfm (a2dp.c:730) is:
> finalize_config(setup);
>
> if (!setup->start || !err)
>         return;

> and the invalid must be the "setup->start". The debug log shows that
> setup_free was called just before and if I'm not missing something, it
> must come from finalize_setup()... but not sure why.

Yep, this is actually a regression introduced by:
commit 99c6f5221800a48e8ce0b1e070e97d1c26a0f90b
Author: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
Date:   Tue Feb 19 12:54:55 2013 +0200

    A2DP: Mark start flag if resume happen while in configured state

    If SEP is in configured state that means OPEN is about to happen so just
    mark start flag and send START once OPEN completes.

As we don't have a start flag, perhaps because the endpoint don't
try/delay the Acquire, the setup is free by finalize_config because
there is no other operation pending.

> The steps to run into this (although there may be some timing
> involved) are simply scan, pair, trust and connect the device with
> bluetoothctl. Device is a Bose SoundLink model 404600.

Looks like there are some new devices that should try to get a hold,
anyway this problem should be fixed ASAP so what about the following
patch:

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 215f4db..c6973ae 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -723,16 +723,12 @@ static void open_cfm(struct avdtp *session,
struct avdtp_local_sep *sep,
        if (err) {
                setup->stream = NULL;
                setup->err = err;
+               if (setup->start)
+                       finalize_resume(setup);
        }

        finalize_config(setup);

-       if (!setup->start || !err)
-               return;
-
-       setup->start = FALSE;
-       finalize_resume(setup);
-
        return;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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