media: gspca: Kill all URBs before releasing any of them

Some subdrivers access the gspca_dev->urb array in the completion handler.
To prevent use-after-free (actually, NULL dereferences) we need to
synchronously kill all the URBs before we release them.

In particular, this is currently the case for drivers such
as sn9c20x and sonixj, which access the gspca_dev->urb[0]
in the context of completion handler for *any* of the URBs.

This commit changes the destroy_urb implementation, so it kills
all URBs first, and then proceed to set the URBs to NULL in the
array and release them.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index a727996..57aa521 100644 (file)
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -472,13 +472,20 @@ static void destroy_urbs(struct gspca_dev *gspca_dev)
        unsigned int i;
 
        gspca_dbg(gspca_dev, D_STREAM, "kill transfer\n");
+
+       /* Killing all URBs guarantee that no URB completion
+        * handler is running. Therefore, there shouldn't
+        * be anyone trying to access gspca_dev->urb[i]
+        */
+       for (i = 0; i < MAX_NURBS; i++)
+               usb_kill_urb(gspca_dev->urb[i]);
+
+       gspca_dbg(gspca_dev, D_STREAM, "releasing urbs\n");
        for (i = 0; i < MAX_NURBS; i++) {
                urb = gspca_dev->urb[i];
-               if (urb == NULL)
-                       break;
-
+               if (!urb)
+                       continue;
                gspca_dev->urb[i] = NULL;
-               usb_kill_urb(urb);
                usb_free_coherent(gspca_dev->dev,
                                  urb->transfer_buffer_length,
                                  urb->transfer_buffer,