[PATCH v2] Input: atkbd - fix UAF in atkbd_set_repeat_rate() on disconnect

[PATCH v2] Input: atkbd - fix UAF in atkbd_set_repeat_rate() on disconnect

[Jeffin Philip]

Commit 0ef7a26af127 ("Input: atkbd - fix canceling event_work in disconnect")
moved cancel_delayed_work_sync() after input_unregister_device() on
the premise that events may arrive until input_unregister_device returns.

However, this created a UAF as work that may have passed the
atkbd->enabled check in atkbd_event_work() may attempt to dereference dev
which is freed in input_unregister_device(). Reverting the commit also
does not solve the issue as events may still come through and pass the
enabled check as atkbd_disable() and event_work() guard with different
locks. Fix this by closing the hardware first using serio_close()
and then unregistering to prevent work from executing after
input_unregister_device(). serio_close() closes the device preventing
work from arriving. Additionally add a check for atkbd->enabled in
atkbd_event() to prevent userspace from queuing new events after
atkbd_disable() sets atkbd->enabled to false.

Reported-by: syzbot+1e2ef9bcb29af666b2e6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1e2ef9bcb29af666b2e6
Fixes: 0ef7a26af127 ("Input: atkbd - fix canceling event_work in disconnect")
Signed-off-by: Jeffin Philip <jeffinphilip14@gmail.com>
---
Changelog:
 - Changes in v2:
     Added a check for atkbd->enabled so userspace cannot queue events after
   atkbd_disable.
Link to v1: https://lore.kernel.org/all/20260828080839.116065-1-jeffinphilip14@gmail.com/T/#u
---
 drivers/input/keyboard/atkbd.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 5736f4bc5a50..79e4cd9ee1db 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -672,6 +672,9 @@ static int atkbd_event(struct input_dev *dev,
 	if (!atkbd->write)
 		return -1;
 
+	if (!atkbd->enabled)
+		return -1;
+
 	switch (type) {
 	case EV_LED:
 		atkbd_schedule_event_work(atkbd, ATKBD_LED_EVENT_BIT);
@@ -963,17 +966,18 @@ static void atkbd_disconnect(struct serio *serio)
 
 	atkbd_disable(atkbd);
 
-	input_unregister_device(atkbd->dev);
-
 	/*
-	 * Make sure we don't have a command in flight.
-	 * Note that since atkbd->enabled is false event work will keep
-	 * rescheduling itself until it gets canceled and will not try
-	 * accessing freed input device or serio port.
+	 * close serio first so device will not get any data, which prevents
+	 * atkbd_event_work from being rescheduled after cancel_delayed_work_sync
+	 * returns. This ensures no work can dereference atkbd->dev after it has
+	 * been freed.
 	 */
-	cancel_delayed_work_sync(&atkbd->event_work);
 
 	serio_close(serio);
+	cancel_delayed_work_sync(&atkbd->event_work);
+
+	input_unregister_device(atkbd->dev);
+
 	serio_set_drvdata(serio, NULL);
 	kfree(atkbd);
 }
-- 
2.55.0

Re: [PATCH v2] Input: atkbd - fix UAF in atkbd_set_repeat_rate() on disconnect

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition in `atkbd_event()` allows delayed work to be queued after `cancel_delayed_work_sync()`, leading to a Use-After-Free of the `atkbd` structure.
--

commit 0f8148a1f313524a8dd641967f56cd788e4c8ac4
Author: Jeffin Philip <jeffinphilip14@gmail.com>

Input: atkbd - fix UAF in atkbd_set_repeat_rate() on disconnect

This commit attempts to fix a use-after-free issue by closing the serio
port before canceling delayed work and adding an enabled check to
atkbd_event(). It aims to prevent work from being scheduled after
disconnection.

Link: https://lore.kernel.org/all/20260828080839.116065-1-jeffinphilip14@gmail.com/T/#u

> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
> --- a/drivers/input/keyboard/atkbd.c
> +++ b/drivers/input/keyboard/atkbd.c
> @@ -672,6 +672,9 @@ static int atkbd_event(struct input_dev *dev,
>  	if (!atkbd->write)
>  		return -1;
>  
> +	if (!atkbd->enabled)
> +		return -1;
> +

[Severity: High]
Can this lockless check introduce a Time-of-Check to Time-of-Use (TOCTOU) race
condition?

If userspace calls evdev_write(), which invokes atkbd_event(), a thread could
read atkbd->enabled as true here, but then stall or be preempted before
actually scheduling the work.

[ ... ]

> @@ -963,17 +966,18 @@ static void atkbd_disconnect(struct serio *serio)
>  
>  	atkbd_disable(atkbd);
>  
> -	input_unregister_device(atkbd->dev);
> -
>  	/*
> -	 * Make sure we don't have a command in flight.
> -	 * Note that since atkbd->enabled is false event work will keep
> -	 * rescheduling itself until it gets canceled and will not try
> -	 * accessing freed input device or serio port.
> +	 * close serio first so device will not get any data, which prevents
> +	 * atkbd_event_work from being rescheduled after cancel_delayed_work_sync
> +	 * returns. This ensures no work can dereference atkbd->dev after it has
> +	 * been freed.
>  	 */
> -	cancel_delayed_work_sync(&atkbd->event_work);
>  
>  	serio_close(serio);
> +	cancel_delayed_work_sync(&atkbd->event_work);
> +
> +	input_unregister_device(atkbd->dev);
> +

[Severity: High]
If the thread running atkbd_event() is delayed after checking atkbd->enabled,
could it resume here and queue work after cancel_delayed_work_sync() has
already returned?

Since serio_close() only stops hardware interrupts and does not block userspace
from invoking atkbd_event(), atkbd_disconnect() could proceed to unregister
the device and free the atkbd structure.

When the delayed thread finally resumes and queues the work, won't that queued
work execute later and dereference the freed atkbd structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828100547.120553-1-jeffinphilip14@gmail.com?part=1

Re: [PATCH v2] Input: atkbd - fix UAF in atkbd_set_repeat_rate() on disconnect

[Dmitry Torokhov]

Hi Jeffin,

On Fri, Aug 28, 2026 at 03:35:47PM +0530, Jeffin Philip wrote:
> Commit 0ef7a26af127 ("Input: atkbd - fix canceling event_work in disconnect")
> moved cancel_delayed_work_sync() after input_unregister_device() on
> the premise that events may arrive until input_unregister_device returns.
> 
> However, this created a UAF as work that may have passed the
> atkbd->enabled check in atkbd_event_work() may attempt to dereference dev
> which is freed in input_unregister_device(). Reverting the commit also
> does not solve the issue as events may still come through and pass the
> enabled check as atkbd_disable() and event_work() guard with different
> locks. Fix this by closing the hardware first using serio_close()
> and then unregistering to prevent work from executing after
> input_unregister_device(). serio_close() closes the device preventing
> work from arriving. Additionally add a check for atkbd->enabled in
> atkbd_event() to prevent userspace from queuing new events after
> atkbd_disable() sets atkbd->enabled to false.

I believe the proper fix is to replace cancel_delayed_work_sync() with
disable_delayed_work_sync() to prevent rescheduling due to atkbd being
marked as disabled.

Thanks.

-- 
Dmitry