From 69acbaac303e8cb948801a9ddd0ac24e86cc4a1b Mon Sep 17 00:00:00 2001 From: Ian Abbott Date: Mon, 8 Jul 2013 13:36:19 +0100 Subject: [PATCH 1/9] staging: comedi: COMEDI_CANCEL ioctl should wake up read/write Comedi devices can do blocking read() or write() (or poll()) if an asynchronous command has been set up, blocking for data (for read()) or buffer space (for write()). Various events associated with the asynchronous command will wake up the blocked reader or writer (or poller). It is also possible to force the asynchronous command to terminate by issuing a `COMEDI_CANCEL` ioctl. That shuts down the asynchronous command, but does not currently wake up the blocked reader or writer (or poller). If the blocked task could be woken up, it would see that the command is no longer active and return. The caller of the `COMEDI_CANCEL` ioctl could attempt to wake up the blocked task by sending a signal, but that's a nasty workaround. Change `do_cancel_ioctl()` to wake up the wait queue after it returns from `do_cancel()`. `do_cancel()` can propagate an error return value from the low-level comedi driver's cancel routine, but it always shuts the command down regardless, so `do_cancel_ioctl()` can wake up he wait queue regardless of the return value from `do_cancel()`. Signed-off-by: Ian Abbott Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/staging/comedi/comedi_fops.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index 8647518259f6..6cdef9d5f8d0 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -1705,6 +1705,7 @@ static int do_cancel_ioctl(struct comedi_device *dev, unsigned int arg, void *file) { struct comedi_subdevice *s; + int ret; if (arg >= dev->n_subdevices) return -EINVAL; @@ -1721,7 +1722,11 @@ static int do_cancel_ioctl(struct comedi_device *dev, unsigned int arg, if (s->busy != file) return -EBUSY; - return do_cancel(dev, s); + ret = do_cancel(dev, s); + if (comedi_get_subdevice_runflags(s) & SRF_USER) + wake_up_interruptible(&s->async->wait_head); + + return ret; } /* From 4b18f08be01a7b3c7b6df497137b6e3cb28adaa3 Mon Sep 17 00:00:00 2001 From: Ian Abbott Date: Fri, 5 Jul 2013 16:49:34 +0100 Subject: [PATCH 2/9] staging: comedi: fix a race between do_cmd_ioctl() and read/write `do_cmd_ioctl()` is called with the comedi device's mutex locked to process the `COMEDI_CMD` ioctl to set up comedi's asynchronous command handling on a comedi subdevice. `comedi_read()` and `comedi_write()` are the `read` and `write` handlers for the comedi device, but do not lock the mutex (for performance reasons, as some things can hold the mutex for quite a long time). There is a race condition if `comedi_read()` or `comedi_write()` is running at the same time and for the same file object and comedi subdevice as `do_cmd_ioctl()`. `do_cmd_ioctl()` sets the subdevice's `busy` pointer to the file object way before it sets the `SRF_RUNNING` flag in the subdevice's `runflags` member. `comedi_read() and `comedi_write()` check the subdevice's `busy` pointer is pointing to the current file object, then if the `SRF_RUNNING` flag is not set, will call `do_become_nonbusy()` to shut down the asyncronous command. Bad things can happen if the asynchronous command is being shutdown and set up at the same time. To prevent the race, don't set the `busy` pointer until after the `SRF_RUNNING` flag has been set. Also, make sure the mutex is held in `comedi_read()` and `comedi_write()` while calling `do_become_nonbusy()` in order to avoid moving the race condition to a point within that function. Change some error handling `goto cleanup` statements in `do_cmd_ioctl()` to simple `return -ERRFOO` statements as a result of changing when the `busy` pointer is set. Signed-off-by: Ian Abbott Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/staging/comedi/comedi_fops.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index 6cdef9d5f8d0..f4a197b2d1fd 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -1413,22 +1413,19 @@ static int do_cmd_ioctl(struct comedi_device *dev, DPRINTK("subdevice busy\n"); return -EBUSY; } - s->busy = file; /* make sure channel/gain list isn't too long */ if (cmd.chanlist_len > s->len_chanlist) { DPRINTK("channel/gain list too long %u > %d\n", cmd.chanlist_len, s->len_chanlist); - ret = -EINVAL; - goto cleanup; + return -EINVAL; } /* make sure channel/gain list isn't too short */ if (cmd.chanlist_len < 1) { DPRINTK("channel/gain list too short %u < 1\n", cmd.chanlist_len); - ret = -EINVAL; - goto cleanup; + return -EINVAL; } async->cmd = cmd; @@ -1438,8 +1435,7 @@ static int do_cmd_ioctl(struct comedi_device *dev, kmalloc(async->cmd.chanlist_len * sizeof(int), GFP_KERNEL); if (!async->cmd.chanlist) { DPRINTK("allocation failed\n"); - ret = -ENOMEM; - goto cleanup; + return -ENOMEM; } if (copy_from_user(async->cmd.chanlist, user_chanlist, @@ -1491,6 +1487,9 @@ static int do_cmd_ioctl(struct comedi_device *dev, comedi_set_subdevice_runflags(s, ~0, SRF_USER | SRF_RUNNING); + /* set s->busy _after_ setting SRF_RUNNING flag to avoid race with + * comedi_read() or comedi_write() */ + s->busy = file; ret = s->do_cmd(dev, s); if (ret == 0) return 0; @@ -2058,11 +2057,13 @@ static ssize_t comedi_write(struct file *file, const char __user *buf, if (!comedi_is_subdevice_running(s)) { if (count == 0) { + mutex_lock(&dev->mutex); if (comedi_is_subdevice_in_error(s)) retval = -EPIPE; else retval = 0; do_become_nonbusy(dev, s); + mutex_unlock(&dev->mutex); } break; } @@ -2161,11 +2162,13 @@ static ssize_t comedi_read(struct file *file, char __user *buf, size_t nbytes, if (n == 0) { if (!comedi_is_subdevice_running(s)) { + mutex_lock(&dev->mutex); do_become_nonbusy(dev, s); if (comedi_is_subdevice_in_error(s)) retval = -EPIPE; else retval = 0; + mutex_unlock(&dev->mutex); break; } if (file->f_flags & O_NONBLOCK) { @@ -2203,9 +2206,11 @@ static ssize_t comedi_read(struct file *file, char __user *buf, size_t nbytes, buf += n; break; /* makes device work like a pipe */ } - if (comedi_is_subdevice_idle(s) && - async->buf_read_count - async->buf_write_count == 0) { - do_become_nonbusy(dev, s); + if (comedi_is_subdevice_idle(s)) { + mutex_lock(&dev->mutex); + if (async->buf_read_count - async->buf_write_count == 0) + do_become_nonbusy(dev, s); + mutex_unlock(&dev->mutex); } set_current_state(TASK_RUNNING); remove_wait_queue(&async->wait_head, &wait); From c8145c56103099565be2e18aeac2a32f79361f3f Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 22 Jul 2013 09:57:49 +0300 Subject: [PATCH 3/9] staging: frontier: use after free in disconnect() usb_alphatrack_delete() frees "dev" so we can't use it on that path. Signed-off-by: Dan Carpenter Signed-off-by: Greg Kroah-Hartman --- drivers/staging/frontier/alphatrack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/frontier/alphatrack.c b/drivers/staging/frontier/alphatrack.c index 5590ebf1da15..817f837b240d 100644 --- a/drivers/staging/frontier/alphatrack.c +++ b/drivers/staging/frontier/alphatrack.c @@ -827,11 +827,11 @@ static void usb_alphatrack_disconnect(struct usb_interface *intf) mutex_unlock(&dev->mtx); usb_alphatrack_delete(dev); } else { + atomic_set(&dev->writes_pending, 0); dev->intf = NULL; mutex_unlock(&dev->mtx); } - atomic_set(&dev->writes_pending, 0); mutex_unlock(&disconnect_mutex); dev_info(&intf->dev, "Alphatrack Surface #%d now disconnected\n", From 3a349cee593a464b25f329a1b76635900187fdb0 Mon Sep 17 00:00:00 2001 From: Paul Bolle Date: Sun, 14 Jul 2013 13:29:48 +0200 Subject: [PATCH 4/9] staging: drm/imx: drop "select OF_VIDEOMODE" Commit ac4c1a9b33 ("staging: drm/imx: Add LDB support") added the DRM_IMX_LDB Kconfig entry. That entry selects OF_VIDEOMODE. But there is no Kconfig symbol named OF_VIDEOMODE. The select statement for that symbol is a nop. Drop it. Signed-off-by: Paul Bolle Acked-by: Sascha Hauer Signed-off-by: Greg Kroah-Hartman --- drivers/staging/imx-drm/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/imx-drm/Kconfig b/drivers/staging/imx-drm/Kconfig index 22339059837f..bd0f2fd01db4 100644 --- a/drivers/staging/imx-drm/Kconfig +++ b/drivers/staging/imx-drm/Kconfig @@ -33,7 +33,6 @@ config DRM_IMX_TVE config DRM_IMX_LDB tristate "Support for LVDS displays" depends on DRM_IMX - select OF_VIDEOMODE help Choose this to enable the internal LVDS Display Bridge (LDB) found on i.MX53 and i.MX6 processors. From f2a6fed1ceaecf6054627f0ddbd4becf43c997fc Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 22 Jul 2013 11:00:53 +0300 Subject: [PATCH 5/9] staging: gdm72xx: potential use after free in send_qos_list() Sometimes free_qos_entry() sometimes frees its argument. I have moved the dereference of "entry" ahead on line to avoid a use after free. Signed-off-by: Dan Carpenter Signed-off-by: Greg Kroah-Hartman --- drivers/staging/gdm72xx/gdm_qos.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/gdm72xx/gdm_qos.c b/drivers/staging/gdm72xx/gdm_qos.c index b795353e8348..cc3692439a5c 100644 --- a/drivers/staging/gdm72xx/gdm_qos.c +++ b/drivers/staging/gdm72xx/gdm_qos.c @@ -250,8 +250,8 @@ static void send_qos_list(struct nic *nic, struct list_head *head) list_for_each_entry_safe(entry, n, head, list) { list_del(&entry->list); - free_qos_entry(entry); gdm_wimax_send_tx(entry->skb, entry->dev); + free_qos_entry(entry); } } From 644d478793c6594277f8ae76954da4ace7ac6f96 Mon Sep 17 00:00:00 2001 From: Sergey Senozhatsky Date: Wed, 26 Jun 2013 15:28:39 +0300 Subject: [PATCH 6/9] staging: zram: protect zram_reset_device() call Commit 9b3bb7abcdf2df0f1b2657e6cbc9d06bc2b3b36f (remove zram_sysfs file (v2)) accidentally made zram_reset_device() racy. Protect zram_reset_device() call with zram->lock. Signed-off-by: Sergey Senozhatsky Acked-by: Jerome Marchand Signed-off-by: Greg Kroah-Hartman --- drivers/staging/zram/zram_drv.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c index 82c7202fd5cc..e77fb6ea40c9 100644 --- a/drivers/staging/zram/zram_drv.c +++ b/drivers/staging/zram/zram_drv.c @@ -527,8 +527,11 @@ static void zram_reset_device(struct zram *zram) size_t index; struct zram_meta *meta; - if (!zram->init_done) + down_write(&zram->init_lock); + if (!zram->init_done) { + up_write(&zram->init_lock); return; + } meta = zram->meta; zram->init_done = 0; @@ -549,6 +552,7 @@ static void zram_reset_device(struct zram *zram) zram->disksize = 0; set_capacity(zram->disk, 0); + up_write(&zram->init_lock); } static void zram_init_device(struct zram *zram, struct zram_meta *meta) From 72bb99cfe9c57d2044445fb34bbc95b4c0bae6f2 Mon Sep 17 00:00:00 2001 From: Karlis Ogsts Date: Mon, 22 Jul 2013 13:51:42 -0700 Subject: [PATCH 7/9] staging: android: logger: Correct write offset reset on error In the situation that a writer fails to copy data from userspace it will reset the write offset to the value it had before it went to sleep. This discarding any messages written while aquiring the mutex. Therefore the reset offset needs to be retrieved after acquiring the mutex. Cc: Android Kernel Team Signed-off-by: Bjorn Andersson Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/staging/android/logger.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/android/logger.c b/drivers/staging/android/logger.c index 080abf2faf97..a8c344422a77 100644 --- a/drivers/staging/android/logger.c +++ b/drivers/staging/android/logger.c @@ -469,7 +469,7 @@ static ssize_t logger_aio_write(struct kiocb *iocb, const struct iovec *iov, unsigned long nr_segs, loff_t ppos) { struct logger_log *log = file_get_log(iocb->ki_filp); - size_t orig = log->w_off; + size_t orig; struct logger_entry header; struct timespec now; ssize_t ret = 0; @@ -490,6 +490,8 @@ static ssize_t logger_aio_write(struct kiocb *iocb, const struct iovec *iov, mutex_lock(&log->mutex); + orig = log->w_off; + /* * Fix up any readers, pulling them forward to the first readable * entry after (what will be) the new write offset. We do this now From 932ef3685f287799fa844862d607a6b596ba5b9e Mon Sep 17 00:00:00 2001 From: Jingoo Han Date: Wed, 24 Jul 2013 14:34:08 +0900 Subject: [PATCH 8/9] staging: tidspbridge: replace strict_strtol() with kstrtos32() The usage of strict_strtol() is not preferred, because strict_strtol() is obsolete. Thus, kstrtos32() should be used in order to convert a string to s32. Also, error handling is added to get rid of a __must_check warning. This fixes a memory corruption bug as well. Signed-off-by: Jingoo Han Reviewed-by: Dan Carpenter Signed-off-by: Greg Kroah-Hartman --- drivers/staging/tidspbridge/pmgr/dbll.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/staging/tidspbridge/pmgr/dbll.c b/drivers/staging/tidspbridge/pmgr/dbll.c index c191ae203565..41e88abe47af 100644 --- a/drivers/staging/tidspbridge/pmgr/dbll.c +++ b/drivers/staging/tidspbridge/pmgr/dbll.c @@ -1120,8 +1120,11 @@ static int dbll_rmm_alloc(struct dynamic_loader_allocate *this, or DYN_EXTERNAL, then mem granularity information is present within the section name - only process if there are at least three tokens within the section name (just a minor optimization) */ - if (count >= 3) - strict_strtol(sz_last_token, 10, (long *)&req); + if (count >= 3) { + status = kstrtos32(sz_last_token, 10, &req); + if (status) + goto func_cont; + } if ((req == 0) || (req == 1)) { if (strcmp(sz_sec_last_token, "DYN_DARAM") == 0) { From 81b884c9dfe6753a78df0cbf4e96f095d718bd95 Mon Sep 17 00:00:00 2001 From: Lidza Louina Date: Wed, 24 Jul 2013 11:29:33 -0400 Subject: [PATCH 9/9] MAINTAINERS: Update the list of maintainers for staging/comedi driver. This patch updates the list of maintainers for the staging/comedi driver. Signed-off-by: Lidza Louina Acked-by: H Hartley Sweeten Acked-by: Ian Abbott Signed-off-by: Greg Kroah-Hartman --- MAINTAINERS | 2 +- drivers/staging/comedi/TODO | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index bf61e04291ab..6ddb33d5c653 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7812,7 +7812,7 @@ F: drivers/staging/asus_oled/ STAGING - COMEDI M: Ian Abbott -M: Mori Hess +M: H Hartley Sweeten S: Odd Fixes F: drivers/staging/comedi/ diff --git a/drivers/staging/comedi/TODO b/drivers/staging/comedi/TODO index b10f739b7e3e..fa8da9aada30 100644 --- a/drivers/staging/comedi/TODO +++ b/drivers/staging/comedi/TODO @@ -9,4 +9,4 @@ TODO: Please send patches to Greg Kroah-Hartman and copy: Ian Abbott - Frank Mori Hess + H Hartley Sweeten