Thread (8 messages) 8 messages, 2 authors, 2012-04-03

Re: [PATCH 2/6] Input: uinput - fix race that can block nonblocking read

From: Aristeu Rozanski <hidden>
Date: 2012-04-03 16:37:37

On Fri, Mar 30, 2012 at 11:06:19PM -0700, Dmitry Torokhov wrote:
quoted hunk ↗ jump to hunk
From: David Herrmann <redacted>

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 never calling wait_event_*() for non-blocking FDs hence we
will never sleep in the waitq here.

Also, if we fail to retrieve an event because it was "stolen" by another
thread, we will return -EAGAIN instead of 0 in case of nonblocking read.

Signed-off-by: David Herrmann <redacted>
Signed-off-by: Dmitry Torokhov <redacted>
---
 drivers/input/misc/uinput.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index eb9723a..5339c1d 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -460,16 +460,13 @@ static ssize_t uinput_read(struct file *file, char __user *buffer, size_t count,
 	if (count < input_event_size())
 		return -EINVAL;
 
-	if (udev->state != UIST_CREATED)
-		return -ENODEV;
-
-	if (udev->head == udev->tail && (file->f_flags & O_NONBLOCK))
-		return -EAGAIN;
-
-	retval = wait_event_interruptible(udev->waitq,
-			udev->head != udev->tail || udev->state != UIST_CREATED);
-	if (retval)
-		return retval;
+	if (!(file->f_flags & O_NONBLOCK)) {
+		retval = wait_event_interruptible(udev->waitq,
+						  udev->head != udev->tail ||
+						  udev->state != UIST_CREATED);
+		if (retval)
+			return retval;
+	}
no. if the state is not UIST_CREATED, it should return ENODEV, not EAGAIN.
and you're not checking it if O_NONBLOCK is present.

-- 
Aristeu
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help