Skip to content

Commit

Permalink
Input: uinput - fix race that can block nonblocking read
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
dtor committed Aug 22, 2012
1 parent f40033a commit 22ae19c
Showing 1 changed file with 44 additions and 30 deletions.
74 changes: 44 additions & 30 deletions drivers/input/misc/uinput.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 22ae19c

Please sign in to comment.