Re: [PATCH 1/6] android/a2dp: Fix possible NULL dereference

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

 



Hi Andrei,

On Wed, Dec 4, 2013 at 10:36 AM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@xxxxxxxxx> wrote:
> Hi Luiz,
>
> On Tue, Dec 03, 2013 at 09:53:43PM +0200, Luiz Augusto von Dentz wrote:
>> Hi Andrei,
>>
>> On Tue, Dec 3, 2013 at 5:53 PM, Andrei Emeltchenko
>> <Andrei.Emeltchenko.news@xxxxxxxxx> wrote:
>> > From: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx>
>> >
>> > Since a2dp_record may return NULL, check return value. This
>> > silences static analysers tools.
>> > ---
>> >  android/a2dp.c | 5 +++--
>> >  1 file changed, 3 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/android/a2dp.c b/android/a2dp.c
>> > index cee4bfa..36a0714 100644
>> > --- a/android/a2dp.c
>> > +++ b/android/a2dp.c
>> > @@ -366,9 +366,10 @@ bool bt_a2dp_register(const bdaddr_t *addr)
>> >         }
>> >
>> >         rec = a2dp_record();
>> > -       if (bt_adapter_add_record(rec, SVC_HINT_CAPTURING) < 0) {
>> > +       if (!rec || bt_adapter_add_record(rec, SVC_HINT_CAPTURING) < 0) {
>>
>> Usually we check the return individually, that means you do if (rec)
>> and perhaps handle the error path with goto, but first make sure that
>> a2dp_record can actually fail otherwise this is pointless.
>
> It might return NULL if malloc fails, do you think that we need to change
> malloc to g_malloc in sdp code. Otherwise every tools warns about NULL
> dereference.
>
> Best regards
> Andrei Emeltchenko
>
>>
>> >                 error("Failed to register on A2DP record");
>> > -               sdp_record_free(rec);
>> > +               if (rec)
>> > +                       sdp_record_free(rec);
>> >                 g_io_channel_shutdown(server, TRUE, NULL);
>> >                 g_io_channel_unref(server);
>> >                 server = NULL;
>> > --
>> > 1.8.3.2
>> >
>> > --
>> > 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

I fixed this myself and applied 1-4, patch 5 is actually wrong since
sdp_next_handle may return values bellow 0x10000 if we run out of
handles and patch 6 is not necessary since what is in android/avdtp.c
is what we will be using in the future.


-- 
Luiz Augusto von Dentz
--
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