From 929d1af5478dec82903e05aa9662a4ec12ad655b Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Sun, 29 Jul 2012 22:48:31 -0700 Subject: Input: uinput - take event lock when fetching events from buffer When fetching events form device's buffer in uinput_read() we need to take input device's event_lock to avoid racing with new event delivery. Signed-off-by: Dmitry Torokhov --- drivers/input/misc/uinput.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'drivers/input/misc/uinput.c') diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index 736056897e50..1b4ee4a5c49c 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -452,9 +452,28 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer, size_t return retval; } +static bool uinput_fetch_next_event(struct uinput_device *udev, + struct input_event *event) +{ + bool have_event; + + spin_lock_irq(&udev->dev->event_lock); + + have_event = udev->head != udev->tail; + if (have_event) { + *event = udev->buff[udev->tail]; + udev->tail = (udev->tail + 1) % UINPUT_BUFFER_SIZE; + } + + spin_unlock_irq(&udev->dev->event_lock); + + return have_event; +} + 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; if (udev->state != UIST_CREATED) @@ -477,12 +496,14 @@ 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 (retval + input_event_size() <= count && + uinput_fetch_next_event(udev, &event)) { + + if (input_event_to_user(buffer + retval, &event)) { retval = -EFAULT; goto out; } - udev->tail = (udev->tail + 1) % UINPUT_BUFFER_SIZE; + retval += input_event_size(); } -- cgit v1.2.3 From f40033acc2d14acecd1b27a79dc8a0ad437e619a Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Sun, 29 Jul 2012 22:48:31 -0700 Subject: Input: uinput - return -EINVAL when read buffer size is too small 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. Note that we treat 0 as a special case, according to the standard: "Before any action described below is taken, and if nbyte is zero, the read() function may detect and return errors as described below. In the absence of errors, or if error detection is not performed, the read() function shall return zero and have no other results." Signed-off-by: David Herrmann Signed-off-by: Dmitry Torokhov --- drivers/input/misc/uinput.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/input/misc/uinput.c') diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index 1b4ee4a5c49c..e74ed9cc6371 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -476,6 +476,9 @@ static ssize_t uinput_read(struct file *file, char __user *buffer, size_t count, struct input_event event; int retval = 0; + if (count != 0 && count < input_event_size()) + return -EINVAL; + if (udev->state != UIST_CREATED) return -ENODEV; -- cgit v1.2.3 From 22ae19c6e3c22b390952e90f452f26adad9b8687 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Sun, 29 Jul 2012 22:48:31 -0700 Subject: 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 Signed-off-by: Dmitry Torokhov --- drivers/input/misc/uinput.c | 74 +++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 30 deletions(-) (limited to 'drivers/input/misc/uinput.c') diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index e74ed9cc6371..1719554fe194 100644 --- 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; } -- cgit v1.2.3 From 00ce756ce53acdb82d408346e6a7b734ca9e5bad Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Sun, 29 Jul 2012 22:48:32 -0700 Subject: Input: uinput - mark failed submission requests as free 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 --- drivers/input/misc/uinput.c | 78 +++++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 38 deletions(-) (limited to 'drivers/input/misc/uinput.c') diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index 1719554fe194..6099365102db 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) @@ -478,20 +483,17 @@ static ssize_t uinput_events_to_user(struct uinput_device *udev, { 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; - } + if (input_event_to_user(buffer + read, &event)) + return -EFAULT; read += input_event_size(); } - return read ?: error; + return read; } static ssize_t uinput_read(struct file *file, char __user *buffer, -- cgit v1.2.3 From c5b3533a82ef4b6ceae81b7675f8d6dadcc6f3ab Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Sun, 29 Jul 2012 22:48:32 -0700 Subject: Input: uinput - specify exact bit sizes on userspace APIs 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 --- drivers/input/misc/uinput.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'drivers/input/misc/uinput.c') diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index 6099365102db..b247e1c8e8f6 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; }; -- cgit v1.2.3 From 54ce165ebd9d9494b64149e0d1ab4ddbf5ea908b Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Sun, 29 Jul 2012 22:48:32 -0700 Subject: Input: uinput - fix formatting Reformat the code to keep it within 80 columns. Signed-off-by: Dmitry Torokhov --- drivers/input/misc/uinput.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) (limited to 'drivers/input/misc/uinput.c') diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index b247e1c8e8f6..86328e9c9848 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -40,7 +40,8 @@ #include #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; @@ -744,7 +751,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; } @@ -827,7 +835,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)); } @@ -872,4 +881,3 @@ MODULE_VERSION("0.3"); module_init(uinput_init); module_exit(uinput_exit); - -- cgit v1.2.3 From b4adbbefc2099476a4f1020041c99f52cf3cd67d Mon Sep 17 00:00:00 2001 From: Henrik Rydberg Date: Sat, 11 Aug 2012 22:07:55 +0200 Subject: Input: MT - Add flags to input_mt_init_slots() Preparing to move more repeated code into the mt core, add a flags argument to the input_mt_slots_init() function. Reviewed-and-tested-by: Benjamin Tissoires Tested-by: Ping Cheng Acked-by: Dmitry Torokhov Signed-off-by: Henrik Rydberg --- drivers/input/misc/uinput.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/input/misc/uinput.c') diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index 736056897e50..6b1797503e34 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -405,7 +405,7 @@ static int uinput_setup_device(struct uinput_device *udev, const char __user *bu goto exit; if (test_bit(ABS_MT_SLOT, dev->absbit)) { int nslot = input_abs_get_max(dev, ABS_MT_SLOT) + 1; - input_mt_init_slots(dev, nslot); + input_mt_init_slots(dev, nslot, 0); } else if (test_bit(ABS_MT_POSITION_X, dev->absbit)) { input_set_events_per_packet(dev, 60); } -- cgit v1.2.3