From: Oliver Neukum Date: Wed, 26 Sep 2007 13:19:01 +0000 (-0300) Subject: V4L/DVB (6237): Oops in pwc v4l driver X-Git-Tag: v2.6.24-rc1~1463^2~62 X-Git-Url: http://www.pilppa.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=0b67f5c568c545cb36f88e9f418af2df1cc58589;p=linux-2.6-omap-h63xx.git V4L/DVB (6237): Oops in pwc v4l driver The pwc driver is defficient in locking, which can trigger an oops when disconnecting. Signed-off-by: Oliver Neukum CC: Luc Saillard Signed-off-by: Mauro Carvalho Chehab --- diff --git a/drivers/media/video/pwc/pwc-if.c b/drivers/media/video/pwc/pwc-if.c index 1088ebf5744..0ff5718bf9b 100644 --- a/drivers/media/video/pwc/pwc-if.c +++ b/drivers/media/video/pwc/pwc-if.c @@ -907,31 +907,49 @@ int pwc_isoc_init(struct pwc_device *pdev) return 0; } -void pwc_isoc_cleanup(struct pwc_device *pdev) +static void pwc_iso_stop(struct pwc_device *pdev) { int i; - PWC_DEBUG_OPEN(">> pwc_isoc_cleanup()\n"); - if (pdev == NULL) - return; - if (pdev->iso_init == 0) - 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) { - PWC_DEBUG_MEMORY("Unlinking URB %p\n", urb); - usb_kill_urb(urb); - } + PWC_DEBUG_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) { PWC_DEBUG_MEMORY("Freeing URB\n"); usb_free_urb(urb); pdev->sbuf[i].urb = NULL; } } +} + +void pwc_isoc_cleanup(struct pwc_device *pdev) +{ + PWC_DEBUG_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) @@ -1211,6 +1229,7 @@ static int pwc_video_close(struct inode *inode, struct file *file) PWC_DEBUG_OPEN(">> video_close called(vdev = 0x%p).\n", vdev); + lock_kernel(); pdev = (struct pwc_device *)vdev->priv; if (pdev->vopen == 0) PWC_DEBUG_MODULE("video_close() called on closed device?\n"); @@ -1230,7 +1249,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 */ @@ -1276,7 +1294,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; void *image_buffer_addr; PWC_DEBUG_READ("pwc_video_read(vdev=0x%p, buf=%p, count=%zd) called.\n", @@ -1286,8 +1304,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. */ + + mutex_lock(&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) { @@ -1298,17 +1320,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); @@ -1317,8 +1342,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; + } } PWC_DEBUG_READ("Copying data to user space.\n"); @@ -1333,14 +1360,20 @@ static ssize_t pwc_video_read(struct file *file, char __user *buf, image_buffer_addr = pdev->image_data; image_buffer_addr += pdev->images[pdev->fill_image].offset; image_buffer_addr += pdev->image_read_pos; - if (copy_to_user(buf, image_buffer_addr, count)) - return -EFAULT; + if (copy_to_user(buf, image_buffer_addr, 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); } + mutex_unlock(&pdev->modlock); return count; +err_out: + mutex_unlock(&pdev->modlock); + return rv; } static unsigned int pwc_video_poll(struct file *file, poll_table *wait) @@ -1366,7 +1399,20 @@ static unsigned int pwc_video_poll(struct file *file, poll_table *wait) 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; + + mutex_lock(&pdev->modlock); + if (!pdev->unplugged) + r = video_usercopy(inode, file, cmd, arg, pwc_video_do_ioctl); + mutex_unlock(&pdev->modlock); +out: + return r; } static int pwc_video_mmap(struct file *file, struct vm_area_struct *vma) @@ -1809,7 +1855,10 @@ static void usb_pwc_disconnect(struct usb_interface *intf) wake_up_interruptible(&pdev->frameq); /* Wait until device is closed */ if(pdev->vopen) { + mutex_lock(&pdev->modlock); pdev->unplugged = 1; + mutex_unlock(&pdev->modlock); + pwc_iso_stop(pdev); } else { /* Device is closed, so we can safely unregister it */ PWC_DEBUG_PROBE("Unregistering video device in disconnect().\n"); @@ -1827,7 +1876,6 @@ disconnect_out: unlock_kernel(); } - /* *grunt* We have to do atoi ourselves :-( */ static int pwc_atoi(const char *s) {