[PATCH 0/2] input: evdev: Fix how SYN_DROPPED is generated

[PATCH 0/2] input: evdev: Fix how SYN_DROPPED is generated

[Kornel Dulęba]

This series fixes two issues discovered while working with a gpio-keys
backed device on Android. In short:

1. When a gpio-keys backed device is opened, its initial state is
   immediately reported.
2. Android issues EVIOCSCLOCKID right after, before reading any data.
3. evdev flushes the queue, enqueues SYN_DROPPED, and caches the current
   time as the device timestamp. Notably, it does not enqueue a
   SYN_REPORT.
4. When the next real event is reported by hardware, it is enqueued with
   the stale cached timestamp and a prepended SYN_DROPPED.
5. The whole packet, up to the first legitimate SYN_REPORT, is then
   ignored by userspace.

SYN_DROPPED needs to be issued after EVIOCSCLOCKID to ensure that any
partially read packet is dropped. At the same time, evdev only defines
the batched events() callback, not the single-event event() one. All
events in a packet up to SYN_REPORT are written to a client's queue at
once while holding client->buffer_lock. The EVIOCSCLOCKID handler takes
the same lock when flushing the queue. Therefore, EVIOCSCLOCKID cannot
inject SYN_DROPPED in the middle of a packet while it is being written
to the queue.

Based on the above, I believe that the best fix here is to emit
SYN_REPORT right after SYN_DROPPED.

Patch 1: Always follow SYN_DROPPED with a SYN_REPORT.
Patch 2: Stamp the SYN_DROPPED+SYN_REPORT events with a timestamp read
         directly from the clock, bypassing the cache.

Signed-off-by: Kornel Dulęba <korneld@google.com>
---
Kornel Dulęba (2):
      input: evdev: Always terminate SYN_DROPPED with SYN_REPORT
      input: evdev: Read clock directly in __evdev_queue_syn_dropped()

 drivers/input/evdev.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)
---
base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
change-id: 20261022-evdev_syndropped_fix-76352587616b

Best regards,
-- 
Kornel Dulęba <korneld@google.com>

[PATCH 1/2] input: evdev: Always terminate SYN_DROPPED with SYN_REPORT

[Kornel Dulęba]

Per Documentation/input/event-codes.rst, on SYN_DROPPED:
"Client should ignore all events up to and including next SYN_REPORT
event and query the device (using EVIOCG* ioctls) to obtain its
current state."

Currently, __evdev_queue_syn_dropped() queues a standalone SYN_DROPPED
event without a terminating SYN_REPORT. This causes two issues:

1. The queued SYN_DROPPED event remains unreadable until the next
   hardware event frame arrives and finishes with SYN_REPORT.
2. When the next hardware event frame arrives, SYN_DROPPED is prepended
   to it in the same packet. Compliant userspace clients then discard
   the entire hardware event frame up to its SYN_REPORT.

This was discovered on an Android system with a gpio-keys backed device:
1. gpio_keys_open() reports the initial state on open().
2. Android follows open() with EVIOCSCLOCKID.
3. This creates an unterminated SYN_DROPPED, causing the subsequent
   event to be discarded by userspace.

Fix this by appending SYN_REPORT after SYN_DROPPED in
__evdev_queue_syn_dropped() and updating client->packet_head
accordingly.

Signed-off-by: Kornel Dulęba <korneld@google.com>
---
 drivers/input/evdev.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 8bfaaa45e0b9..c6da2601480f 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -147,6 +147,7 @@ static void __evdev_queue_syn_dropped(struct evdev_client *client)
 {
 	ktime_t *ev_time = input_get_timestamp(client->evdev->handle.dev);
 	struct timespec64 ts = ktime_to_timespec64(ev_time[client->clk_type]);
+	unsigned int buf_mask = client->bufsize - 1;
 	struct input_event ev;
 
 	memset(&ev, 0, sizeof(ev));
@@ -156,13 +157,24 @@ static void __evdev_queue_syn_dropped(struct evdev_client *client)
 	ev.code = SYN_DROPPED;
 
 	client->buffer[client->head++] = ev;
-	client->head &= client->bufsize - 1;
+	client->head &= buf_mask;
 
 	if (unlikely(client->head == client->tail)) {
 		/* drop queue but keep our SYN_DROPPED event */
-		client->tail = (client->head - 1) & (client->bufsize - 1);
-		client->packet_head = client->tail;
+		client->tail = (client->head - 1) & buf_mask;
 	}
+
+	ev.code = SYN_REPORT;
+
+	client->buffer[client->head++] = ev;
+	client->head &= buf_mask;
+
+	if (unlikely(client->head == client->tail)) {
+		/* drop queue but keep our SYN_DROPPED and SYN_REPORT events */
+		client->tail = (client->head - 2) & buf_mask;
+	}
+
+	client->packet_head = client->head;
 }
 
 static void evdev_queue_syn_dropped(struct evdev_client *client)

-- 
2.56.0.307.g5fe7bd680c-goog

Re: [PATCH 1/2] input: evdev: Always terminate SYN_DROPPED with SYN_REPORT

[Dmitry Torokhov]

Hi Kornel,

On Thu, Oct 22, 2026 at 01:39:12PM +0000, Kornel Dulęba wrote:
> Per Documentation/input/event-codes.rst, on SYN_DROPPED:
> "Client should ignore all events up to and including next SYN_REPORT
> event and query the device (using EVIOCG* ioctls) to obtain its
> current state."
> 
> Currently, __evdev_queue_syn_dropped() queues a standalone SYN_DROPPED
> event without a terminating SYN_REPORT. This causes two issues:
> 
> 1. The queued SYN_DROPPED event remains unreadable until the next
>    hardware event frame arrives and finishes with SYN_REPORT.
> 2. When the next hardware event frame arrives, SYN_DROPPED is prepended
>    to it in the same packet. Compliant userspace clients then discard
>    the entire hardware event frame up to its SYN_REPORT.
> 
> This was discovered on an Android system with a gpio-keys backed device:
> 1. gpio_keys_open() reports the initial state on open().
> 2. Android follows open() with EVIOCSCLOCKID.
> 3. This creates an unterminated SYN_DROPPED, causing the subsequent
>    event to be discarded by userspace.
> 
> Fix this by appending SYN_REPORT after SYN_DROPPED in
> __evdev_queue_syn_dropped() and updating client->packet_head
> accordingly.

I do not think that injecting synthetic SYN_REPORT is a good idea: we do
not know at what point the clock change will arrive, it may arrive in a
middle of transmitting a packet, and if we discard only half of such
packet it will result in incomplete state being presented as complete
one.

Compliant userspace should not only discard events until the next
SYN_REPORT, but also query state via EVIOCG* to reconstruct full
picture. Is it not happening?

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] input: evdev: Always terminate SYN_DROPPED with SYN_REPORT

[Kornel Dulęba]

Hey,

On Tue, Nov 3, 2026 at 4:33 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Kornel,
>
> On Thu, Oct 22, 2026 at 01:39:12PM +0000, Kornel Dulęba wrote:
> > Per Documentation/input/event-codes.rst, on SYN_DROPPED:
> > "Client should ignore all events up to and including next SYN_REPORT
> > event and query the device (using EVIOCG* ioctls) to obtain its
> > current state."
> >
> > Currently, __evdev_queue_syn_dropped() queues a standalone SYN_DROPPED
> > event without a terminating SYN_REPORT. This causes two issues:
> >
> > 1. The queued SYN_DROPPED event remains unreadable until the next
> >    hardware event frame arrives and finishes with SYN_REPORT.
> > 2. When the next hardware event frame arrives, SYN_DROPPED is prepended
> >    to it in the same packet. Compliant userspace clients then discard
> >    the entire hardware event frame up to its SYN_REPORT.
> >
> > This was discovered on an Android system with a gpio-keys backed device:
> > 1. gpio_keys_open() reports the initial state on open().
> > 2. Android follows open() with EVIOCSCLOCKID.
> > 3. This creates an unterminated SYN_DROPPED, causing the subsequent
> >    event to be discarded by userspace.
> >
> > Fix this by appending SYN_REPORT after SYN_DROPPED in
> > __evdev_queue_syn_dropped() and updating client->packet_head
> > accordingly.
>
> I do not think that injecting synthetic SYN_REPORT is a good idea: we do
> not know at what point the clock change will arrive, it may arrive in a
> middle of transmitting a packet, and if we discard only half of such
> packet it will result in incomplete state being presented as complete
> one.

I don't think it's possible to flush the queue in the middle of
receiving a packet from the input framework. The only callback evdev
implements for parsing incoming packets is "events," which I believe
is called only for full, SYN_REPORT-terminated frames.

The only possible scenario for flushing a partial packet is when
userspace has read a part of a packet from the queue. In such a
situation, userspace will only see a single SYN_REPORT, though:

1. A full packet is written; userspace might have already read a part
of it. Let's say that the remaining part is: [foo, bar, SYN_REPORT].
2. An ioctl to change the clock is received. The queue is flushed and
replaced with: [SYN_DROPPED, SYN_REPORT].
3. Userspace now sees the part of the packet it managed to read, plus
the [SYN_DROPPED, SYN_REPORT].

> Compliant userspace should not only discard events until the next
> SYN_REPORT, but also query state via EVIOCG* to reconstruct full
> picture. Is it not happening?

Heh, good point. In my case, userspace called EVIOCGKEY(0) and
EVIOCGSW(0), but it didn't parse the results correctly. I actually
fixed that too, but for some reason, I didn't realize that it meant
the kernel patches are no longer needed.
I'm not sure if this patch is relevant anymore; sorry for the noise.
BTW, the second patch in this series is only relevant if this one is
applied. Because of that, I didn't respond to your comments there.

[PATCH 2/2] input: evdev: Read clock directly in __evdev_queue_syn_dropped()

[Kornel Dulęba]

Currently, __evdev_queue_syn_dropped() calls input_get_timestamp() on
the underlying input_dev to obtain the event timestamp. If no timestamp
is set, input_get_timestamp() caches the current time and returns it;
otherwise, it returns the cached value.

There are two problems with this approach:

1. Locking. The timestamp is obtained in the same way in
   evdev_events(), which is called with dev->event_lock held. Calls to
   __evdev_queue_syn_dropped() are only serialized with
   client->buffer_lock. This makes both accesses likely racy.
2. If no timestamp is cached yet, the subsequent hardware event
   generated after __evdev_queue_syn_dropped() is called will receive
   the stale cached timestamp.

Fix this by reading the current time directly based on the value of
client->clk_type. Open-coding this is not the prettiest solution, but I
couldn't figure out anything better.

I initially thought about fixing this by resetting the input timestamp
in this function instead, similarly to how INPUT_FLUSH handles it.
However, I decided against it, as that would require non-trivial
locking changes.

Signed-off-by: Kornel Dulęba <korneld@google.com>
---
 drivers/input/evdev.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index c6da2601480f..da7396c19188 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -145,11 +145,18 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
 
 static void __evdev_queue_syn_dropped(struct evdev_client *client)
 {
-	ktime_t *ev_time = input_get_timestamp(client->evdev->handle.dev);
-	struct timespec64 ts = ktime_to_timespec64(ev_time[client->clk_type]);
 	unsigned int buf_mask = client->bufsize - 1;
 	struct input_event ev;
+	ktime_t time;
+	struct timespec64 ts;
+
+	time = client->clk_type == INPUT_CLK_REAL ?
+			ktime_get_real() :
+			client->clk_type == INPUT_CLK_MONO ?
+				ktime_get() :
+				ktime_get_boottime();
 
+	ts = ktime_to_timespec64(time);
 	memset(&ev, 0, sizeof(ev));
 	ev.input_event_sec = ts.tv_sec;
 	ev.input_event_usec = ts.tv_nsec / NSEC_PER_USEC;

-- 
2.56.0.307.g5fe7bd680c-goog

Re: [PATCH 2/2] input: evdev: Read clock directly in __evdev_queue_syn_dropped()

[Dmitry Torokhov]

Hi Kornel,

On Thu, Oct 22, 2026 at 01:39:13PM +0000, Kornel Dulęba wrote:
> Currently, __evdev_queue_syn_dropped() calls input_get_timestamp() on
> the underlying input_dev to obtain the event timestamp. If no timestamp
> is set, input_get_timestamp() caches the current time and returns it;
> otherwise, it returns the cached value.
> 
> There are two problems with this approach:
> 
> 1. Locking. The timestamp is obtained in the same way in
>    evdev_events(), which is called with dev->event_lock held. Calls to
>    __evdev_queue_syn_dropped() are only serialized with
>    client->buffer_lock. This makes both accesses likely racy.

handler->events() is called under dev->event_lock. I'd love to stop
doing this, this is how it is now.

> 2. If no timestamp is cached yet, the subsequent hardware event
>    generated after __evdev_queue_syn_dropped() is called will receive
>    the stale cached timestamp.
> 
> Fix this by reading the current time directly based on the value of
> client->clk_type. Open-coding this is not the prettiest solution, but I
> couldn't figure out anything better.
> 
> I initially thought about fixing this by resetting the input timestamp
> in this function instead, similarly to how INPUT_FLUSH handles it.
> However, I decided against it, as that would require non-trivial
> locking changes.

I wonder if we care about timing on the SYN_DROPPED events? Can we just
grab the dev->timestamp and if it is 0 that's fine. Or not even set the
timestamp at all on SYN_DROPPED and have it always at 0.

What do you think?

Thanks.

-- 
Dmitry