[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

[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