Currently, iowarrior_read() does not provide any protection for the iowarrior structure, so the iowarrior structure is vulnerable to data-race. Therefore, I think it is appropriate to protect the structure using mutex_lock in iowarrior_read(). Fixes: 946b960d13c1 ("USB: add driver for iowarrior devices.") Signed-off-by: Jeongjun Park <aha310510@xxxxxxxxx> --- drivers/usb/misc/iowarrior.c | 42 +++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c index 6d28467ce352..7f3d37b395c3 100644 --- a/drivers/usb/misc/iowarrior.c +++ b/drivers/usb/misc/iowarrior.c @@ -277,28 +277,41 @@ static ssize_t iowarrior_read(struct file *file, char __user *buffer, struct iowarrior *dev; int read_idx; int offset; + int retval = 0; dev = file->private_data; + if (!dev) { + retval = -ENODEV; + goto exit; + } + + mutex_lock(&dev->mutex); /* verify that the device wasn't unplugged */ - if (!dev || !dev->present) - return -ENODEV; + if (!dev->present) { + retval = -ENODEV; + goto unlock_exit; + } dev_dbg(&dev->interface->dev, "minor %d, count = %zd\n", dev->minor, count); /* read count must be packet size (+ time stamp) */ if ((count != dev->report_size) - && (count != (dev->report_size + 1))) - return -EINVAL; + && (count != (dev->report_size + 1))) { + retval = -EINVAL; + goto unlock_exit; + } /* repeat until no buffer overrun in callback handler occur */ do { atomic_set(&dev->overflow_flag, 0); if ((read_idx = read_index(dev)) == -1) { /* queue empty */ - if (file->f_flags & O_NONBLOCK) - return -EAGAIN; + if (file->f_flags & O_NONBLOCK) { + retval = -EAGAIN; + goto unlock_exit; + } else { //next line will return when there is either new data, or the device is unplugged int r = wait_event_interruptible(dev->read_wait, @@ -309,28 +322,37 @@ static ssize_t iowarrior_read(struct file *file, char __user *buffer, -1)); if (r) { //we were interrupted by a signal - return -ERESTART; + retval = -ERESTART; + goto unlock_exit; } if (!dev->present) { //The device was unplugged - return -ENODEV; + retval = -ENODEV; + goto unlock_exit; } if (read_idx == -1) { // Can this happen ??? - return 0; + goto unlock_exit; } } } offset = read_idx * (dev->report_size + 1); if (copy_to_user(buffer, dev->read_queue + offset, count)) { - return -EFAULT; + retval = -EFAULT; + goto unlock_exit; } } while (atomic_read(&dev->overflow_flag)); read_idx = ++read_idx == MAX_INTERRUPT_BUFFER ? 0 : read_idx; atomic_set(&dev->read_idx, read_idx); + mutex_unlock(&dev->mutex); return count; + +unlock_exit: + mutex_unlock(&dev->mutex); +exit: + return retval; } /* --