Input: uinput - fix race that can block nonblocking read
Consider two threads calling read() on the same uinput-fd, both
non-blocking. Assume there is data-available so both will simultaneously
pass:
udev->head == udev->tail
Then the first thread goes to sleep and the second one pops the message
from the queue. Now assume udev->head == udev->tail. If the first thread
wakes up it will call wait_event_*() and sleep in the waitq. This
effectively turns the non-blocking FD into a blocking one.
We fix this by attempting to fetch events from the queue first and only
if we fail to retrieve any events we either return -EAGAIN (in case of
non-blocing read) or wait until there are more events.
This also fixes incorrect return code (we were returning 0 instead of
-EAGAIN for non-blocking reads) when an event is "stolen" by another
thread. Blocking reads will now continue to wait instead of returning 0
in this scenario.
Count of 0 continues to be a special case, as per spec: we will check for
device existence and whether there are events in the queue, but no events
will be actually retrieved.
Reported-by: David Herrmann <dh.herrmann@googlemail.com>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index e74ed9c..1719554 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -439,6 +439,9 @@
struct uinput_device *udev = file->private_data;
int retval;
+ if (count == 0)
+ return 0;
+
retval = mutex_lock_interruptible(&udev->mutex);
if (retval)
return retval;
@@ -470,48 +473,59 @@
return have_event;
}
-static ssize_t uinput_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos)
+static ssize_t uinput_events_to_user(struct uinput_device *udev,
+ char __user *buffer, size_t count)
+{
+ struct input_event event;
+ size_t read = 0;
+ int error = 0;
+
+ while (read + input_event_size() <= count &&
+ uinput_fetch_next_event(udev, &event)) {
+
+ if (input_event_to_user(buffer + read, &event)) {
+ error = -EFAULT;
+ break;
+ }
+
+ read += input_event_size();
+ }
+
+ return read ?: error;
+}
+
+static ssize_t uinput_read(struct file *file, char __user *buffer,
+ size_t count, loff_t *ppos)
{
struct uinput_device *udev = file->private_data;
- struct input_event event;
- int retval = 0;
+ ssize_t retval;
if (count != 0 && count < input_event_size())
return -EINVAL;
- if (udev->state != UIST_CREATED)
- return -ENODEV;
+ do {
+ retval = mutex_lock_interruptible(&udev->mutex);
+ if (retval)
+ return retval;
- if (udev->head == udev->tail && (file->f_flags & O_NONBLOCK))
- return -EAGAIN;
+ if (udev->state != UIST_CREATED)
+ retval = -ENODEV;
+ else if (udev->head == udev->tail &&
+ (file->f_flags & O_NONBLOCK))
+ retval = -EAGAIN;
+ else
+ retval = uinput_events_to_user(udev, buffer, count);
- retval = wait_event_interruptible(udev->waitq,
- udev->head != udev->tail || udev->state != UIST_CREATED);
- if (retval)
- return retval;
+ mutex_unlock(&udev->mutex);
- retval = mutex_lock_interruptible(&udev->mutex);
- if (retval)
- return retval;
+ if (retval || count == 0)
+ break;
- if (udev->state != UIST_CREATED) {
- retval = -ENODEV;
- goto out;
- }
-
- while (retval + input_event_size() <= count &&
- uinput_fetch_next_event(udev, &event)) {
-
- if (input_event_to_user(buffer + retval, &event)) {
- retval = -EFAULT;
- goto out;
- }
-
- retval += input_event_size();
- }
-
- out:
- mutex_unlock(&udev->mutex);
+ if (!(file->f_flags & O_NONBLOCK))
+ retval = wait_event_interruptible(udev->waitq,
+ udev->head != udev->tail ||
+ udev->state != UIST_CREATED);
+ } while (retval == 0);
return retval;
}