On Thu, Mar 22, 2012 at 3:30 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > This patch (as1517) fixes an error in the USB scatter-gather library. > The library code uses urb->dev to determine whether or nor an URB is > currently active; the completion handler sets urb->dev to NULL. > However the core unlinking routines need to use urb->dev. Since > unlinking always racing with completion, the completion handler must > not clear urb->dev -- it can lead to invalid memory accesses when a > transfer has to be cancelled. > > This patch fixes the problem by getting rid of the lines that clear > urb->dev after urb has been submitted. As a result we may end up > trying to unlink an URB that failed in submission or that has already > completed, so an extra check is added after each unlink to avoid > printing an error message when this happens. > > Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > Reported-and-tested-by: Illia Zaitsev <I.Zaitsev@xxxxxxxxxxxxx> > CC: <stable@xxxxxxxxxxxxxxx> > > --- > > drivers/usb/core/message.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > Index: usb-3.3/drivers/usb/core/message.c > =================================================================== > --- usb-3.3.orig/drivers/usb/core/message.c > +++ usb-3.3/drivers/usb/core/message.c > @@ -308,7 +308,8 @@ static void sg_complete(struct urb *urb) > retval = usb_unlink_urb(io->urbs [i]); > if (retval != -EINPROGRESS && > retval != -ENODEV && > - retval != -EBUSY) > + retval != -EBUSY && > + retval != -EIDRM) > dev_err(&io->dev->dev, > "%s, unlink --> %d\n", > __func__, retval); > @@ -317,7 +318,6 @@ static void sg_complete(struct urb *urb) > } > spin_lock(&io->lock); > } > - urb->dev = NULL; Given the above is removed, the check on urb->dev in sg_complete and sg_cancel may be removed too. > > /* on the last completion, signal usb_sg_wait() */ > io->bytes += urb->actual_length; > @@ -524,7 +524,6 @@ void usb_sg_wait(struct usb_sg_request * > case -ENXIO: /* hc didn't queue this one */ > case -EAGAIN: > case -ENOMEM: > - io->urbs[i]->dev = NULL; > retval = 0; > yield(); > break; > @@ -542,7 +541,6 @@ void usb_sg_wait(struct usb_sg_request * > > /* fail any uncompleted urbs */ > default: > - io->urbs[i]->dev = NULL; Maybe the check of (retval != -EIDRM) should be added in sg_cancel() too, otherwise warning will be printed even though the URB has been completed. Thanks, -- Ming Lei -- 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
