[PATCH 1/6] Input: uinput - return -EINVAL when read buffer size is too small

[PATCH 1/6] Input: uinput - return -EINVAL when read buffer size is too small

[Dmitry Torokhov]

From: David Herrmann <dh.herrmann@googlemail.com>

Let's check whether the user-supplied buffer is actually big enough and
return -EINVAL if it is not. This differs from current behavior, which
caused 0 to be returned and actually does not make any sense, as
broken application will simply repeat the read getting into endless
loop.

Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/misc/uinput.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 7360568..eb9723a 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -457,6 +457,9 @@ static ssize_t uinput_read(struct file *file, char __user *buffer, size_t count,
 	struct uinput_device *udev = file->private_data;
 	int retval = 0;
 
+	if (count < input_event_size())
+		return -EINVAL;
+
 	if (udev->state != UIST_CREATED)
 		return -ENODEV;
 
-- 
1.7.7.6

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

[Dmitry Torokhov]

From: David Herrmann <dh.herrmann@googlemail.com>

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 <dh.herrmann@googlemail.com>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 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;
+	}
 
 	retval = mutex_lock_interruptible(&udev->mutex);
 	if (retval)
@@ -480,8 +477,10 @@ static ssize_t uinput_read(struct file *file, char __user *buffer, size_t count,
 		goto out;
 	}
 
-	while (udev->head != udev->tail && retval + input_event_size() <= count) {
-		if (input_event_to_user(buffer + retval, &udev->buff[udev->tail])) {
+	while (udev->head != udev->tail &&
+               retval + input_event_size() <= count) {
+		if (input_event_to_user(buffer + retval,
+					&udev->buff[udev->tail])) {
 			retval = -EFAULT;
 			goto out;
 		}
@@ -489,6 +488,9 @@ static ssize_t uinput_read(struct file *file, char __user *buffer, size_t count,
 		retval += input_event_size();
 	}
 
+	if (retval == 0 && (file->f_flags & O_NONBLOCK))
+		retval = -EAGAIN;
+
  out:
 	mutex_unlock(&udev->mutex);
 
-- 
1.7.7.6

[PATCH 3/6] Input: uinput - return number of bytes copied on partial reads

[Dmitry Torokhov]

If uinput_read() fails after copying several events to userspace
we need to return number of bytes successfully copied, not -EFAULT.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/misc/uinput.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 5339c1d..a33a7b3 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -455,7 +455,8 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer, size_t
 static ssize_t uinput_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos)
 {
 	struct uinput_device *udev = file->private_data;
-	int retval = 0;
+	size_t read = 0;
+	int retval;
 
 	if (count < input_event_size())
 		return -EINVAL;
@@ -478,23 +479,23 @@ static ssize_t uinput_read(struct file *file, char __user *buffer, size_t count,
 	}
 
 	while (udev->head != udev->tail &&
-               retval + input_event_size() <= count) {
-		if (input_event_to_user(buffer + retval,
+               read + input_event_size() <= count) {
+		if (input_event_to_user(buffer + read,
 					&udev->buff[udev->tail])) {
 			retval = -EFAULT;
 			goto out;
 		}
 		udev->tail = (udev->tail + 1) % UINPUT_BUFFER_SIZE;
-		retval += input_event_size();
+		read += input_event_size();
 	}
 
-	if (retval == 0 && (file->f_flags & O_NONBLOCK))
+	if (read == 0 && (file->f_flags & O_NONBLOCK))
 		retval = -EAGAIN;
 
  out:
 	mutex_unlock(&udev->mutex);
 
-	return retval;
+	return read ?: retval;
 }
 
 static unsigned int uinput_poll(struct file *file, poll_table *wait)
-- 
1.7.7.6

[PATCH 4/6] Input: uinput - mark failed submission requests as free

[Dmitry Torokhov]

If uinput_request_submit() fails after new request ID was allocated
we need to mark that request ID as free, otherwise it will always
stay occupied and we may run out of available IDs.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/misc/uinput.c |   69 +++++++++++++++++++++++--------------------
 1 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index a33a7b3..65e444e 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -56,10 +56,11 @@ static int uinput_dev_event(struct input_dev *dev, unsigned int type, unsigned i
 }
 
 /* Atomically allocate an ID for the given request. Returns 0 on success. */
-static int uinput_request_alloc_id(struct uinput_device *udev, struct uinput_request *request)
+static bool uinput_request_alloc_id(struct uinput_device *udev,
+				    struct uinput_request *request)
 {
 	int id;
-	int err = -1;
+	bool reserved = false;
 
 	spin_lock(&udev->requests_lock);
 
@@ -67,13 +68,13 @@ static int uinput_request_alloc_id(struct uinput_device *udev, struct uinput_req
 		if (!udev->requests[id]) {
 			request->id = id;
 			udev->requests[id] = request;
-			err = 0;
+			reserved = true;
 			break;
 		}
 	}
 
 	spin_unlock(&udev->requests_lock);
-	return err;
+	return reserved;
 }
 
 static struct uinput_request *uinput_request_find(struct uinput_device *udev, int id)
@@ -85,11 +86,12 @@ static struct uinput_request *uinput_request_find(struct uinput_device *udev, in
 	return udev->requests[id];
 }
 
-static inline int uinput_request_reserve_slot(struct uinput_device *udev, struct uinput_request *request)
+static int uinput_request_reserve_slot(struct uinput_device *udev,
+				       struct uinput_request *request)
 {
 	/* Allocate slot. If none are available right away, wait. */
 	return wait_event_interruptible(udev->requests_waitq,
-					!uinput_request_alloc_id(udev, request));
+					uinput_request_alloc_id(udev, request));
 }
 
 static void uinput_request_done(struct uinput_device *udev, struct uinput_request *request)
@@ -101,14 +103,11 @@ static void uinput_request_done(struct uinput_device *udev, struct uinput_reques
 	complete(&request->done);
 }
 
-static int uinput_request_submit(struct uinput_device *udev, struct uinput_request *request)
+static int uinput_request_send(struct uinput_device *udev,
+			       struct uinput_request *request)
 {
 	int retval;
 
-	retval = uinput_request_reserve_slot(udev, request);
-	if (retval)
-		return retval;
-
 	retval = mutex_lock_interruptible(&udev->mutex);
 	if (retval)
 		return retval;
@@ -118,7 +117,12 @@ static int uinput_request_submit(struct uinput_device *udev, struct uinput_reque
 		goto out;
 	}
 
-	/* Tell our userspace app about this new request by queueing an input event */
+	init_completion(&request->done);
+
+	/*
+	 * Tell our userspace application about this new request
+	 * by queueing an input event.
+	 */
 	uinput_dev_event(udev->dev, EV_UINPUT, request->code, request->id);
 
  out:
@@ -126,6 +130,25 @@ static int uinput_request_submit(struct uinput_device *udev, struct uinput_reque
 	return retval;
 }
 
+static int uinput_request_submit(struct uinput_device *udev,
+				 struct uinput_request *request)
+{
+	int error;
+
+	error = uinput_request_reserve_slot(udev, request);
+	if (error)
+		return error;
+
+	error = uinput_request_send(udev, request);
+	if (error) {
+		uinput_request_done(udev, request);
+		return error;
+	}
+
+	wait_for_completion(&request->done);
+	return request->retval;
+}
+
 /*
  * Fail all ouitstanding requests so handlers don't wait for the userspace
  * to finish processing them.
@@ -167,7 +190,6 @@ static int uinput_dev_upload_effect(struct input_dev *dev, struct ff_effect *eff
 {
 	struct uinput_device *udev = input_get_drvdata(dev);
 	struct uinput_request request;
-	int retval;
 
 	/*
 	 * uinput driver does not currently support periodic effects with
@@ -180,42 +202,25 @@ static int uinput_dev_upload_effect(struct input_dev *dev, struct ff_effect *eff
 			effect->u.periodic.waveform == FF_CUSTOM)
 		return -EINVAL;
 
-	request.id = -1;
-	init_completion(&request.done);
 	request.code = UI_FF_UPLOAD;
 	request.u.upload.effect = effect;
 	request.u.upload.old = old;
 
-	retval = uinput_request_submit(udev, &request);
-	if (!retval) {
-		wait_for_completion(&request.done);
-		retval = request.retval;
-	}
-
-	return retval;
+	return uinput_request_submit(udev, &request);
 }
 
 static int uinput_dev_erase_effect(struct input_dev *dev, int effect_id)
 {
 	struct uinput_device *udev = input_get_drvdata(dev);
 	struct uinput_request request;
-	int retval;
 
 	if (!test_bit(EV_FF, dev->evbit))
 		return -ENOSYS;
 
-	request.id = -1;
-	init_completion(&request.done);
 	request.code = UI_FF_ERASE;
 	request.u.effect_id = effect_id;
 
-	retval = uinput_request_submit(udev, &request);
-	if (!retval) {
-		wait_for_completion(&request.done);
-		retval = request.retval;
-	}
-
-	return retval;
+	return uinput_request_submit(udev, &request);
 }
 
 static void uinput_destroy_device(struct uinput_device *udev)
-- 
1.7.7.6

[PATCH 5/6] Input: uinput - specify exact bit sizes on userspace APIs

[Dmitry Torokhov]

Switch to using __u32/__s32 instead of ordinary 'int' in structures
forming userspace API.

Also internally make request_id unsigned int.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/misc/uinput.c |   11 ++++++-----
 include/linux/uinput.h      |   26 +++++++++++++-------------
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 65e444e..bfdee05 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -59,7 +59,7 @@ static int uinput_dev_event(struct input_dev *dev, unsigned int type, unsigned i
 static bool uinput_request_alloc_id(struct uinput_device *udev,
 				    struct uinput_request *request)
 {
-	int id;
+	unsigned int id;
 	bool reserved = false;
 
 	spin_lock(&udev->requests_lock);
@@ -77,10 +77,11 @@ static bool uinput_request_alloc_id(struct uinput_device *udev,
 	return reserved;
 }
 
-static struct uinput_request *uinput_request_find(struct uinput_device *udev, int id)
+static struct uinput_request *uinput_request_find(struct uinput_device *udev,
+						  unsigned int id)
 {
 	/* Find an input request, by ID. Returns NULL if the ID isn't valid. */
-	if (id >= UINPUT_NUM_REQUESTS || id < 0)
+	if (id >= UINPUT_NUM_REQUESTS)
 		return NULL;
 
 	return udev->requests[id];
@@ -527,8 +528,8 @@ static int uinput_release(struct inode *inode, struct file *file)
 
 #ifdef CONFIG_COMPAT
 struct uinput_ff_upload_compat {
-	int			request_id;
-	int			retval;
+	__u32			request_id;
+	__s32			retval;
 	struct ff_effect_compat	effect;
 	struct ff_effect_compat	old;
 };
diff --git a/include/linux/uinput.h b/include/linux/uinput.h
index 2aa2881..2457598 100644
--- a/include/linux/uinput.h
+++ b/include/linux/uinput.h
@@ -44,14 +44,14 @@
 enum uinput_state { UIST_NEW_DEVICE, UIST_SETUP_COMPLETE, UIST_CREATED };
 
 struct uinput_request {
-	int			id;
-	int			code;	/* UI_FF_UPLOAD, UI_FF_ERASE */
+	unsigned int		id;
+	unsigned int		code;	/* UI_FF_UPLOAD, UI_FF_ERASE */
 
 	int			retval;
 	struct completion	done;
 
 	union {
-		int		effect_id;
+		unsigned int	effect_id;
 		struct {
 			struct ff_effect *effect;
 			struct ff_effect *old;
@@ -77,16 +77,16 @@ struct uinput_device {
 #endif	/* __KERNEL__ */
 
 struct uinput_ff_upload {
-	int			request_id;
-	int			retval;
+	__u32			request_id;
+	__s32			retval;
 	struct ff_effect	effect;
 	struct ff_effect	old;
 };
 
 struct uinput_ff_erase {
-	int			request_id;
-	int			retval;
-	int			effect_id;
+	__u32			request_id;
+	__s32			retval;
+	__u32			effect_id;
 };
 
 /* ioctl */
@@ -166,11 +166,11 @@ struct uinput_ff_erase {
 struct uinput_user_dev {
 	char name[UINPUT_MAX_NAME_SIZE];
 	struct input_id id;
-	int ff_effects_max;
-	int absmax[ABS_CNT];
-	int absmin[ABS_CNT];
-	int absfuzz[ABS_CNT];
-	int absflat[ABS_CNT];
+	__u32 ff_effects_max;
+	__s32 absmax[ABS_CNT];
+	__s32 absmin[ABS_CNT];
+	__s32 absfuzz[ABS_CNT];
+	__s32 absflat[ABS_CNT];
 };
 #endif	/* __UINPUT_H_ */
 
-- 
1.7.7.6

[PATCH 6/6] Input: uinput - fix formatting

[Dmitry Torokhov]

Reformat the code to keep it within 80 columns.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/misc/uinput.c |   31 ++++++++++++++++++++-----------
 1 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index bfdee05..d9feed6 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -40,7 +40,8 @@
 #include <linux/input/mt.h>
 #include "../input-compat.h"
 
-static int uinput_dev_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
+static int uinput_dev_event(struct input_dev *dev,
+			    unsigned int type, unsigned int code, int value)
 {
 	struct uinput_device	*udev = input_get_drvdata(dev);
 
@@ -95,7 +96,8 @@ static int uinput_request_reserve_slot(struct uinput_device *udev,
 					uinput_request_alloc_id(udev, request));
 }
 
-static void uinput_request_done(struct uinput_device *udev, struct uinput_request *request)
+static void uinput_request_done(struct uinput_device *udev,
+				struct uinput_request *request)
 {
 	/* Mark slot as available */
 	udev->requests[request->id] = NULL;
@@ -151,7 +153,7 @@ static int uinput_request_submit(struct uinput_device *udev,
 }
 
 /*
- * Fail all ouitstanding requests so handlers don't wait for the userspace
+ * Fail all outstanding requests so handlers don't wait for the userspace
  * to finish processing them.
  */
 static void uinput_flush_requests(struct uinput_device *udev)
@@ -187,7 +189,9 @@ static int uinput_dev_playback(struct input_dev *dev, int effect_id, int value)
 	return uinput_dev_event(dev, EV_FF, effect_id, value);
 }
 
-static int uinput_dev_upload_effect(struct input_dev *dev, struct ff_effect *effect, struct ff_effect *old)
+static int uinput_dev_upload_effect(struct input_dev *dev,
+				    struct ff_effect *effect,
+				    struct ff_effect *old)
 {
 	struct uinput_device *udev = input_get_drvdata(dev);
 	struct uinput_request request;
@@ -353,7 +357,8 @@ static int uinput_allocate_device(struct uinput_device *udev)
 	return 0;
 }
 
-static int uinput_setup_device(struct uinput_device *udev, const char __user *buffer, size_t count)
+static int uinput_setup_device(struct uinput_device *udev,
+			       const char __user *buffer, size_t count)
 {
 	struct uinput_user_dev	*user_dev;
 	struct input_dev	*dev;
@@ -425,7 +430,8 @@ static int uinput_setup_device(struct uinput_device *udev, const char __user *bu
 	return retval;
 }
 
-static inline ssize_t uinput_inject_event(struct uinput_device *udev, const char __user *buffer, size_t count)
+static ssize_t uinput_inject_event(struct uinput_device *udev,
+				   const char __user *buffer, size_t count)
 {
 	struct input_event ev;
 
@@ -440,7 +446,8 @@ static inline ssize_t uinput_inject_event(struct uinput_device *udev, const char
 	return input_event_size();
 }
 
-static ssize_t uinput_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+static ssize_t uinput_write(struct file *file, const char __user *buffer,
+			    size_t count, loff_t *ppos)
 {
 	struct uinput_device *udev = file->private_data;
 	int retval;
@@ -458,7 +465,8 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer, size_t
 	return retval;
 }
 
-static ssize_t uinput_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos)
+static ssize_t uinput_read(struct file *file, char __user *buffer,
+			   size_t count, loff_t *ppos)
 {
 	struct uinput_device *udev = file->private_data;
 	size_t read = 0;
@@ -715,7 +723,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 				break;
 
 			req = uinput_request_find(udev, ff_up.request_id);
-			if (!req || req->code != UI_FF_UPLOAD || !req->u.upload.effect) {
+			if (!req || req->code != UI_FF_UPLOAD ||
+			    !req->u.upload.effect) {
 				retval = -EINVAL;
 				break;
 			}
@@ -798,7 +807,8 @@ static long uinput_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 }
 
 #ifdef CONFIG_COMPAT
-static long uinput_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+static long uinput_compat_ioctl(struct file *file,
+				unsigned int cmd, unsigned long arg)
 {
 	return uinput_ioctl_handler(file, cmd, arg, compat_ptr(arg));
 }
@@ -843,4 +853,3 @@ MODULE_VERSION("0.3");
 
 module_init(uinput_init);
 module_exit(uinput_exit);
-
-- 
1.7.7.6

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

[Aristeu Rozanski]

On Fri, Mar 30, 2012 at 11:06:19PM -0700, Dmitry Torokhov wrote:
> From: David Herrmann <dh.herrmann@googlemail.com>
> 
> 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 <dh.herrmann@googlemail.com>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>  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

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

[Dmitry Torokhov]

On Tue, Apr 03, 2012 at 12:28:40PM -0400, Aristeu Rozanski wrote:
> On Fri, Mar 30, 2012 at 11:06:19PM -0700, Dmitry Torokhov wrote:
> > From: David Herrmann <dh.herrmann@googlemail.com>
> > 
> > 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 <dh.herrmann@googlemail.com>
> > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > ---
> >  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.

Actually this condition is checked again one we take the mutex. Anyway,
please take a look at the updated version I posted; it looks differently
now.

Thanks.

-- 
Dmitry

[PATCH 5/6] Input: uinput - specify exact bit sizes on userspace APIs

[Dmitry Torokhov]

Switch to using __u32/__s32 instead of ordinary 'int' in structures
forming userspace API.

Also internally make request_id unsigned int.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/misc/uinput.c |   11 ++++++-----
 include/linux/uinput.h      |   27 ++++++++++++++-------------
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 6099365..b247e1c 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -59,7 +59,7 @@ static int uinput_dev_event(struct input_dev *dev, unsigned int type, unsigned i
 static bool uinput_request_alloc_id(struct uinput_device *udev,
 				    struct uinput_request *request)
 {
-	int id;
+	unsigned int id;
 	bool reserved = false;
 
 	spin_lock(&udev->requests_lock);
@@ -77,10 +77,11 @@ static bool uinput_request_alloc_id(struct uinput_device *udev,
 	return reserved;
 }
 
-static struct uinput_request *uinput_request_find(struct uinput_device *udev, int id)
+static struct uinput_request *uinput_request_find(struct uinput_device *udev,
+						  unsigned int id)
 {
 	/* Find an input request, by ID. Returns NULL if the ID isn't valid. */
-	if (id >= UINPUT_NUM_REQUESTS || id < 0)
+	if (id >= UINPUT_NUM_REQUESTS)
 		return NULL;
 
 	return udev->requests[id];
@@ -556,8 +557,8 @@ static int uinput_release(struct inode *inode, struct file *file)
 
 #ifdef CONFIG_COMPAT
 struct uinput_ff_upload_compat {
-	int			request_id;
-	int			retval;
+	__u32			request_id;
+	__s32			retval;
 	struct ff_effect_compat	effect;
 	struct ff_effect_compat	old;
 };
diff --git a/include/linux/uinput.h b/include/linux/uinput.h
index 2aa2881..c454bbe 100644
--- a/include/linux/uinput.h
+++ b/include/linux/uinput.h
@@ -32,6 +32,7 @@
  *		- first public version
  */
 
+#include <linux/types.h>
 #include <linux/input.h>
 
 #define UINPUT_VERSION		3
@@ -44,14 +45,14 @@
 enum uinput_state { UIST_NEW_DEVICE, UIST_SETUP_COMPLETE, UIST_CREATED };
 
 struct uinput_request {
-	int			id;
-	int			code;	/* UI_FF_UPLOAD, UI_FF_ERASE */
+	unsigned int		id;
+	unsigned int		code;	/* UI_FF_UPLOAD, UI_FF_ERASE */
 
 	int			retval;
 	struct completion	done;
 
 	union {
-		int		effect_id;
+		unsigned int	effect_id;
 		struct {
 			struct ff_effect *effect;
 			struct ff_effect *old;
@@ -77,16 +78,16 @@ struct uinput_device {
 #endif	/* __KERNEL__ */
 
 struct uinput_ff_upload {
-	int			request_id;
-	int			retval;
+	__u32			request_id;
+	__s32			retval;
 	struct ff_effect	effect;
 	struct ff_effect	old;
 };
 
 struct uinput_ff_erase {
-	int			request_id;
-	int			retval;
-	int			effect_id;
+	__u32			request_id;
+	__s32			retval;
+	__u32			effect_id;
 };
 
 /* ioctl */
@@ -166,11 +167,11 @@ struct uinput_ff_erase {
 struct uinput_user_dev {
 	char name[UINPUT_MAX_NAME_SIZE];
 	struct input_id id;
-	int ff_effects_max;
-	int absmax[ABS_CNT];
-	int absmin[ABS_CNT];
-	int absfuzz[ABS_CNT];
-	int absflat[ABS_CNT];
+	__u32 ff_effects_max;
+	__s32 absmax[ABS_CNT];
+	__s32 absmin[ABS_CNT];
+	__s32 absfuzz[ABS_CNT];
+	__s32 absflat[ABS_CNT];
 };
 #endif	/* __UINPUT_H_ */
 
-- 
1.7.7.6