[PATCH] usb: use mutex_lock in iowarrior_read()

Jeongjun Park
2024-09-16T04:06:44Z

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@gmail.com>
---
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;
}

/*
--