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