Re: [PATCH] avrcp: Fix wrong pointer check

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

 



Hi Andrei,

On Sun, Feb 2, 2014 at 11:51 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@xxxxxxxxx> wrote:
> Hi Luiz,
>
> On Sun, Feb 02, 2014 at 08:03:34AM -0800, Luiz Augusto von Dentz wrote:
>> Hi Andrei,
>>
>> On Fri, Jan 31, 2014 at 12:33 AM, Andrei Emeltchenko
>> <Andrei.Emeltchenko.news@xxxxxxxxx> wrote:
>> > From: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx>
>> >
>> > There is wrong assumption that handler might be NULL while it is a
>> > pointer to a struct table so check instead for struct members. This
>> > fixes accessing wrong memory.
>> > ---
>> >  profiles/audio/avrcp.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
>> > index df88138..5030ce1 100644
>> > --- a/profiles/audio/avrcp.c
>> > +++ b/profiles/audio/avrcp.c
>> > @@ -1673,7 +1673,7 @@ static size_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
>> >                         break;
>> >         }
>> >
>> > -       if (!handler || handler->code != *code) {
>> > +       if (!handler->code || handler->code != *code) {
>>
>> The code checks if  session->control_handlers is initialized and Im
>> not sure what is the invalid memory access you are talking about since
>> handle->code is no a pointer, I do agree that we should probably drop
>> the second check for the handler in the lines bellow.
>
> handler is a pointer, and it points to
>
> static const struct control_pdu_handler control_handlers[]
> table with the last element:
>
> ...
>                 { },
> };
> ...
>
> So checking for !handler is pointless.

Right, because checking for pointer is pointless, yes Im being
sarcastic here... Now lets be clear, you are changing a check of a
pointer to a value and claiming it fixes invalid accesses which does
not make any sense, what could make sense is to check if
handler->pdu_id == pdu->pdu_id since that what we check when we lookup
for a handle.



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