From d33e2f267f57e415d8d3fdca375652eb5767e908 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Thu, 1 Nov 2007 04:30:09 +0100 Subject: [PATCH] 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 Signed-off-by: Adrian Bunk --- drivers/usb/media/pwc/pwc-if.c | 94 ++++++++++++++++++++++++++-------- 1 file changed, 72 insertions(+), 22 deletions(-) diff --git a/drivers/usb/media/pwc/pwc-if.c b/drivers/usb/media/pwc/pwc-if.c index 8e55391fe2ec..583001e68702 100644 --- 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) { -- 2.39.5