USB: don't clear urb->dev in scatter-gather library
Alan Stern [Thu, 22 Mar 2012 15:00:21 +0000 (11:00 -0400)]
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>

drivers/usb/core/message.c

index b3bdfed..aed3e07 100644 (file)
@@ -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;
 
        /* 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 *io)
                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 *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);
@@ -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);
                }