On Mon, Feb 03, 2014 at 11:36:52AM +0200, Andrei Emeltchenko wrote:
> On Mon, Feb 03, 2014 at 01:20:11AM -0800, Luiz Augusto von Dentz wrote:
> > 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.
>
> No, we break if handler->pdu_id == pdu->pdu_id one line above ;)
Sorry this was wrong. So my idea is to check that we are not at the end of
the table. In this case every element of a struct is zero, so it does not
matter which one to check.
Best regards
Andrei Emeltchenko
--
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
