On Sunday 02 May 2010 21:24:49 Alan Stern wrote:
> On Sun, 2 May 2010, Michael Buesch wrote:
>
> > This fixes a NULL pointer dereference triggered by an off-by-one
> > error, if the USB_REQ_SET_CONFIGURATION request to the device in
> > usb_reset_configuration() fails.
> >
> > Signed-off-by: Michael Buesch <mb@xxxxxxxxx>
> > Cc: stable@xxxxxxxxxx
> >
> > ---
> >
> > Alan, this fixes the crash.
>
> Can you explain why? I don't see any off-by-one errors.
>
> > Index: linux-2.6.33/drivers/usb/core/message.c
> > ===================================================================
> > --- linux-2.6.33.orig/drivers/usb/core/message.c 2010-05-02 19:41:58.000000000 +0200
> > +++ linux-2.6.33/drivers/usb/core/message.c 2010-05-02 19:42:46.000000000 +0200
> > @@ -1489,8 +1489,10 @@ reset_old_alts:
> > USB_REQ_SET_CONFIGURATION, 0,
> > config->desc.bConfigurationValue, 0,
> > NULL, 0, USB_CTRL_SET_TIMEOUT);
> > - if (retval < 0)
> > + if (retval < 0) {
> > + i--;
> > goto reset_old_alts;
> > + }
>
> This extra decrement should not be needed, because the "for" loop at
> reset_old_alts starts out by decrementing i.
No it does not. The initialization area of the for loop is empty.
Which is correct for the case where the loop is entered through the
first if (retval < 0) without a goto. Because in that case the previous
for-loop was terminated with a break. However for the second case, the first
for loop will have looped through all of its elements. Leaving i at bNumInterfaces.
So just before goto reset_old_alts i is bNumInterfaces. Which is an invalid index
as it is one beyond the array.
--
Greetings, Michael.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
