Skip to content

Commit

Permalink
USB: don't clear urb->dev in scatter-gather library
Browse files Browse the repository at this point in the history
This patch (as1517b) 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.  The checks are updated
in both sg_complete() and sg_cancel(), and the second is updated to
match the first (currently it prints out unnecessary warning messages
if a device is unplugged while a transfer is in progress).

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: Illia Zaitsev <I.Zaitsev@adbglobal.com>
CC: Ming Lei <tom.leiming@gmail.com>
CC: <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Alan Stern authored and Greg Kroah-Hartman committed Apr 6, 2012
1 parent c825bab commit bcf3985
Showing 1 changed file with 6 additions and 5 deletions.
11 changes: 6 additions & 5 deletions drivers/usb/core/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -317,7 +318,6 @@ static void sg_complete(struct urb *urb)
}
spin_lock(&io->lock);
}
urb->dev = NULL;

/* on the last completion, signal usb_sg_wait() */
io->bytes += urb->actual_length;
Expand Down Expand Up @@ -524,7 +524,6 @@ void usb_sg_wait(struct usb_sg_request *io)
case -ENXIO: /* hc didn't queue this one */
case -EAGAIN:
case -ENOMEM:
io->urbs[i]->dev = NULL;
retval = 0;
yield();
break;
Expand All @@ -542,7 +541,6 @@ void usb_sg_wait(struct usb_sg_request *io)

/* fail any uncompleted urbs */
default:
io->urbs[i]->dev = NULL;
io->urbs[i]->status = retval;
dev_dbg(&io->dev->dev, "%s, submit --> %d\n",
__func__, retval);
Expand Down Expand Up @@ -593,7 +591,10 @@ void usb_sg_cancel(struct usb_sg_request *io)
if (!io->urbs [i]->dev)
continue;
retval = usb_unlink_urb(io->urbs [i]);
if (retval != -EINPROGRESS && retval != -EBUSY)
if (retval != -EINPROGRESS
&& retval != -ENODEV
&& retval != -EBUSY
&& retval != -EIDRM)
dev_warn(&io->dev->dev, "%s, unlink --> %d\n",
__func__, retval);
}
Expand Down

0 comments on commit bcf3985

Please sign in to comment.