[PATCH] Input: evdev: drain queued events before reporting device removal

[PATCH] Input: evdev: drain queued events before reporting device removal

[zhangliuyang.zly]

When an input device is unregistered, the input core calls
input_dev_release_keys() and sends a SYN_REPORT before closing the input
handles. This is intended to deliver synthetic key-up events for keys that
are still pressed when the device disappears.

However, evdev_read() currently checks evdev->exist before draining the
per-client event queue. If evdev_disconnect() marks the evdev node dead
before userspace reads the queued synthetic release packet, read() returns
-ENODEV immediately and the queued key-up events are lost from userspace's
point of view.

This can happen on Android with USB OTG 2.4G keyboard receivers. The input
core generates the release event during disconnect, but Android EventHub 
may observe the evdev hangup/removal first and then fail to read the 
pending EV_KEY value=0 event. The framework then falls back to device 
reset/cancel semantics instead of dispatching a normal ACTION_UP.

Allow evdev_read() to drain already queued events even after evdev->exist 
is cleared. Return -ENODEV only when the client queue is empty, or when 
the client has been revoked. Keep zero-length reads compatible with the 
previous error-checking behavior.

Signed-off-by: zhangliuyang.zly <zhangliuyang.zly@bytedance.com>
---
 drivers/input/evdev.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index c7325226cb86..89bdd7a2c763 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -565,20 +565,38 @@ static ssize_t evdev_read(struct file *file, char __user *buffer,
 	if (count != 0 && count < input_event_size())
 		return -EINVAL;
 
-	for (;;) {
+	/*
+	 * count == 0 is special - no IO is done but we still check for
+	 * error conditions, preserving the historical behavior.
+	 */
+	if (count == 0) {
 		if (!evdev->exist || client->revoked)
 			return -ENODEV;
 
-		if (client->packet_head == client->tail &&
-		    (file->f_flags & O_NONBLOCK))
-			return -EAGAIN;
+		return 0;
+	}
 
+	for (;;) {
 		/*
-		 * count == 0 is special - no IO is done but we check
-		 * for error conditions (see above).
+		 * A revoked client must not consume any more events.
 		 */
-		if (count == 0)
-			break;
+		if (client->revoked)
+			return -ENODEV;
+
+		/*
+		 * The input core may queue synthetic release events during
+		 * device unregister before evdev_disconnect() marks the evdev
+		 * node dead. Do not drop those already queued events by
+		 * returning -ENODEV too early. Drain the client queue first and
+		 * report -ENODEV only when there is nothing left to read.
+		 */
+		if (client->packet_head == client->tail) {
+			if (!evdev->exist)
+				return -ENODEV;
+
+			if (file->f_flags & O_NONBLOCK)
+				return -EAGAIN;
+		}
 
 		while (read + input_event_size() <= count &&
 		       evdev_fetch_next_event(client, &event)) {
-- 
2.54.0

Re: [PATCH] Input: evdev: drain queued events before reporting device removal

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The patch breaks zero-length O_NONBLOCK reads by returning 0 instead of -EAGAIN when the queue is empty, altering userspace ABI.
- [High] A TOCTOU race between lockless queue checks and device death drops synthetic key-up events, defeating the patch's purpose.

Pre-existing issues:
- [High] Uninitialized kernel stack memory is leaked to user-space due to missing padding initialization in `struct input_event` on architectures like SPARC64.
--

--- Patch [1]: [PATCH] Input: evdev: drain queued events before reporting device removal ---
commit 0c2e6f64ca8c023851919612ec0b8e230776b440
Author: zhangliuyang.zly <zhangliuyang.zly@bytedance.com>
Subject: Input: evdev: drain queued events before reporting device removal

This commit modifies the evdev read path to drain the per-client event queue
before reporting -ENODEV when a device is removed. It aims to prevent dropping
synthetic key-up events generated during device unregistration.

> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c

[ ... ]

> @@ -565,20 +565,38 @@ static ssize_t evdev_read(struct file *file, char __user *buffer,
>  	if (count != 0 && count < input_event_size())
>  		return -EINVAL;
>  
> -	for (;;) {
> +	/*
> +	 * count == 0 is special - no IO is done but we still check for
> +	 * error conditions, preserving the historical behavior.
> +	 */
> +	if (count == 0) {
> +		if (!evdev->exist || client->revoked)
> +			return -ENODEV;
> +
> +		return 0;
> +	}

[Severity: High]
Does this change alter the historical behavior for non-blocking reads?

Previously, if count == 0 and the file descriptor was set to O_NONBLOCK with
an empty queue, the code would return -EAGAIN. By moving the count == 0 check
above the loop and unconditionally returning 0, applications checking for queue
status using a zero-length read might now incorrectly assume events are
available.

>  
> +	for (;;) {
> +		/*
> +		 * A revoked client must not consume any more events.
> +		 */
> +		if (client->revoked)
> +			return -ENODEV;
> +
> +		/*
> +		 * The input core may queue synthetic release events during
> +		 * device unregister before evdev_disconnect() marks the evdev
> +		 * node dead. Do not drop those already queued events by
> +		 * returning -ENODEV too early. Drain the client queue first and
> +		 * report -ENODEV only when there is nothing left to read.
> +		 */
> +		if (client->packet_head == client->tail) {
> +			if (!evdev->exist)
> +				return -ENODEV;

[Severity: High]
Might this lockless conditional block still drop synthetic key-up events due
to a race condition?

Because these checks are performed locklessly without memory barriers, a thread
could evaluate packet_head == tail as true. If a context switch occurs and the
input core queues synthetic events and subsequently marks evdev->exist as false,
when this thread resumes it will evaluate !evdev->exist as true.

This would result in returning -ENODEV and missing the newly queued synthetic
events entirely, defeating the original purpose of the patch.

> +
> +			if (file->f_flags & O_NONBLOCK)
> +				return -EAGAIN;
> +		}
>  
>  		while (read + input_event_size() <= count &&
>  		       evdev_fetch_next_event(client, &event)) {
>  
> -			/*
> -			 * count == 0 is special - no IO is done but we check
> -			 * for error conditions (see above).
> -			 */
> -			if (count == 0)
> -				break;
> +			if (input_event_to_user(buffer + read, &event))
> +				return -EFAULT;

[Severity: High]
This is a pre-existing issue, but could this leak uninitialized kernel stack
memory to userspace?

In evdev_pass_values(), the input_event structure is allocated on the stack
but its padding fields (such as the 32-bit padding on SPARC64) are not fully
initialized before being copied into the client buffer.

When evdev_read() fetches these events via evdev_fetch_next_event() and
subsequently copies them to userspace using input_event_to_user():

evdev_pass_values() {
    struct input_event event;
    ...
    event.type = v->type;
    ...
    __pass_event(client, &event);
}

The uninitialized padding bytes are copied out here, potentially leaking
sensitive kernel data.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729092753.1596400-1-zhangliuyang.zly@bytedance.com?part=1