Fix oops in pwc v4l driver

The pwc driver is defficient in locking, which can trigger an oops
when disconnecting.

Adrian Bunk:
Backported to 2.6.16.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Adrian Bunk <bunk@kernel.org>

diff --git a/drivers/usb/media/pwc/pwc-if.c b/drivers/usb/media/pwc/pwc-if.c
index 8e55391fe2ec08fbf0fab1d92a98696e3eb62242..583001e68702f22d4aa17ee8ca2e57e1ba7419ca 100644 (file)
--- a/drivers/usb/media/pwc/pwc-if.c
+++ b/drivers/usb/media/pwc/pwc-if.c
@@ -914,29 +914,49 @@ static int pwc_isoc_init(struct pwc_device *pdev)
        return 0;
 }
 
-static void pwc_isoc_cleanup(struct pwc_device *pdev)
+static void pwc_iso_stop(struct pwc_device *pdev)
 {
        int i;
 
-       Trace(TRACE_OPEN, ">> pwc_isoc_cleanup()\n");
-       if (pdev == NULL)
-               return;
-
        /* Unlinking ISOC buffers one by one */
        for (i = 0; i < MAX_ISO_BUFS; i++) {
                struct urb *urb;
 
                urb = pdev->sbuf[i].urb;
                if (urb != 0) {
-                       if (pdev->iso_init) {
-                               Trace(TRACE_MEMORY, "Unlinking URB %p\n", urb);
-                               usb_kill_urb(urb);
-                       }
+                       Trace(TRACE_MEMORY, "Unlinking URB %p\n", urb);
+                       usb_kill_urb(urb);
+               }
+       }
+}
+
+static void pwc_iso_free(struct pwc_device *pdev)
+{
+       int i;
+
+       /* Freeing ISOC buffers one by one */
+       for (i = 0; i < MAX_ISO_BUFS; i++) {
+               struct urb *urb;
+
+               urb = pdev->sbuf[i].urb;
+               if (urb != 0) {
                        Trace(TRACE_MEMORY, "Freeing URB\n");
                        usb_free_urb(urb);
                        pdev->sbuf[i].urb = NULL;
                }
        }
+}
+
+void pwc_isoc_cleanup(struct pwc_device *pdev)
+{
+       Trace(TRACE_OPEN, ">> pwc_isoc_cleanup()\n");
+       if (pdev == NULL)
+               return;
+       if (pdev->iso_init == 0)
+               return;
+
+       pwc_iso_stop(pdev);
+       pwc_iso_free(pdev);
 
        /* Stop camera, but only if we are sure the camera is still there (unplug
           is signalled by EPIPE) 
@@ -1115,6 +1135,7 @@ static int pwc_video_close(struct inode *inode, struct file *file)
 
        Trace(TRACE_OPEN, ">> video_close called(vdev = 0x%p).\n", vdev);
 
+       lock_kernel();
        pdev = (struct pwc_device *)vdev->priv;
        if (pdev->vopen == 0)
                Info("video_close() called on closed device?\n");
@@ -1146,7 +1167,6 @@ static int pwc_video_close(struct inode *inode, struct file *file)
        pwc_isoc_cleanup(pdev);
        pwc_free_buffers(pdev);
 
-       lock_kernel();
        /* Turn off LEDS and power down camera, but only when not unplugged */
        if (!pdev->unplugged) {
                /* Turn LEDs off */
@@ -1192,7 +1212,7 @@ static ssize_t pwc_video_read(struct file *file, char __user * buf,
        struct pwc_device *pdev;
        int noblock = file->f_flags & O_NONBLOCK;
        DECLARE_WAITQUEUE(wait, current);
-        int bytes_to_read;
+       int bytes_to_read, rv = 0;
 
        Trace(TRACE_READ, "video_read(0x%p, %p, %zu) called.\n", vdev, buf, count);
        if (vdev == NULL)
@@ -1200,8 +1220,12 @@ static ssize_t pwc_video_read(struct file *file, char __user * buf,
        pdev = vdev->priv;
        if (pdev == NULL)
                return -EFAULT;
-       if (pdev->error_status)
-               return -pdev->error_status; /* Something happened, report what. */
+
+       down(&pdev->modlock);
+       if (pdev->error_status) {
+               rv = -pdev->error_status; /* Something happened, report what. */
+               goto err_out;
+       }
 
        /* In case we're doing partial reads, we don't have to wait for a frame */
        if (pdev->image_read_pos == 0) {
@@ -1212,17 +1236,20 @@ static ssize_t pwc_video_read(struct file *file, char __user * buf,
                        if (pdev->error_status) {
                                remove_wait_queue(&pdev->frameq, &wait);
                                set_current_state(TASK_RUNNING);
-                               return -pdev->error_status ;
+                               rv = -pdev->error_status ;
+                               goto err_out;
                        }
                        if (noblock) {
                                remove_wait_queue(&pdev->frameq, &wait);
                                set_current_state(TASK_RUNNING);
-                               return -EWOULDBLOCK;
+                               rv = -EWOULDBLOCK;
+                               goto err_out;
                        }
                        if (signal_pending(current)) {
                                remove_wait_queue(&pdev->frameq, &wait);
                                set_current_state(TASK_RUNNING);
-                               return -ERESTARTSYS;
+                               rv = -ERESTARTSYS;
+                               goto err_out;
                        }
                        schedule();
                        set_current_state(TASK_INTERRUPTIBLE);
@@ -1231,8 +1258,10 @@ static ssize_t pwc_video_read(struct file *file, char __user * buf,
                set_current_state(TASK_RUNNING);
                                                                                                                                                                                 
                /* Decompress and release frame */
-               if (pwc_handle_frame(pdev))
-                       return -EFAULT;
+               if (pwc_handle_frame(pdev)) {
+                       rv = -EFAULT;
+                       goto err_out;
+               }
        }
 
        Trace(TRACE_READ, "Copying data to user space.\n");
@@ -1244,14 +1273,20 @@ static ssize_t pwc_video_read(struct file *file, char __user * buf,
        /* copy bytes to user space; we allow for partial reads */
        if (count + pdev->image_read_pos > bytes_to_read)
                count = bytes_to_read - pdev->image_read_pos;
-       if (copy_to_user(buf, pdev->image_ptr[pdev->fill_image] + pdev->image_read_pos, count))
-               return -EFAULT;
+       if (copy_to_user(buf, pdev->image_ptr[pdev->fill_image] + pdev->image_read_pos, count)) {
+               rv = -EFAULT;
+               goto err_out;
+       }
        pdev->image_read_pos += count;
        if (pdev->image_read_pos >= bytes_to_read) { /* All data has been read */
                pdev->image_read_pos = 0;
                pwc_next_image(pdev);
        }
+       up(&pdev->modlock);
        return count;
+err_out:
+       up(&pdev->modlock);
+       return rv;
 }
 
 static unsigned int pwc_video_poll(struct file *file, poll_table *wait)
@@ -1615,7 +1650,20 @@ static int pwc_video_do_ioctl(struct inode *inode, struct file *file,
 static int pwc_video_ioctl(struct inode *inode, struct file *file,
                           unsigned int cmd, unsigned long arg)
 {
-       return video_usercopy(inode, file, cmd, arg, pwc_video_do_ioctl);
+       struct video_device *vdev = file->private_data;
+       struct pwc_device *pdev;
+       int r = -ENODEV;
+
+       if (!vdev)
+               goto out;
+       pdev = vdev->priv;
+
+       down(&pdev->modlock);
+       if (!pdev->unplugged)
+               r = video_usercopy(inode, file, cmd, arg, pwc_video_do_ioctl);
+       up(&pdev->modlock);
+out:
+       return r;
 }
 
 
@@ -2007,7 +2055,10 @@ static void usb_pwc_disconnect(struct usb_interface *intf)
        wake_up_interruptible(&pdev->frameq);
        /* Wait until device is closed */
        if(pdev->vopen) {
+               down(&pdev->modlock);
                pdev->unplugged = 1;
+               up(&pdev->modlock);
+               pwc_iso_stop(pdev);
        } else {
                /* Device is closed, so we can safely unregister it */
                Trace(TRACE_PROBE, "Unregistering video device in disconnect().\n");
@@ -2025,7 +2076,6 @@ disconnect_out:
        unlock_kernel();
 }
 
-
 /* *grunt* We have to do atoi ourselves :-( */
 static int pwc_atoi(const char *s)
 {