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 e74ed9cc6371d2454ef87baaf06838e043364a90..1719554fe194b6012eb7b1ea54946caac2c698f6 100644 (file)
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -439,6 +439,9 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer, size_t
        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 @@ static bool uinput_fetch_next_event(struct uinput_device *udev,
        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 uinput_device *udev = file->private_data;
        struct input_event event;
-       int retval = 0;
+       size_t read = 0;
+       int error = 0;
 
-       if (count != 0 && count < input_event_size())
-               return -EINVAL;
+       while (read + input_event_size() <= count &&
+              uinput_fetch_next_event(udev, &event)) {
 
-       if (udev->state != UIST_CREATED)
-               return -ENODEV;
+               if (input_event_to_user(buffer + read, &event)) {
+                       error = -EFAULT;
+                       break;
+               }
 
-       if (udev->head == udev->tail && (file->f_flags & O_NONBLOCK))
-               return -EAGAIN;
+               read += input_event_size();
+       }
 
-       retval = wait_event_interruptible(udev->waitq,
-                       udev->head != udev->tail || udev->state != UIST_CREATED);
-       if (retval)
-               return retval;
+       return read ?: error;
+}
 
-       retval = mutex_lock_interruptible(&udev->mutex);
-       if (retval)
-               return retval;
+static ssize_t uinput_read(struct file *file, char __user *buffer,
+                          size_t count, loff_t *ppos)
+{
+       struct uinput_device *udev = file->private_data;
+       ssize_t retval;
 
-       if (udev->state != UIST_CREATED) {
-               retval = -ENODEV;
-               goto out;
-       }
+       if (count != 0 && count < input_event_size())
+               return -EINVAL;
 
-       while (retval + input_event_size() <= count &&
-              uinput_fetch_next_event(udev, &event)) {
+       do {
+               retval = mutex_lock_interruptible(&udev->mutex);
+               if (retval)
+                       return retval;
 
-               if (input_event_to_user(buffer + retval, &event)) {
-                       retval = -EFAULT;
-                       goto out;
-               }
+               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 += input_event_size();
-       }
+               mutex_unlock(&udev->mutex);
 
- out:
-       mutex_unlock(&udev->mutex);
+               if (retval || count == 0)
+                       break;
+
+               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;
 }