[PATCH v4] Input: atkbd - fix uaf in atkbd_set_repeat_rate

[PATCH v4] Input: atkbd - fix uaf in atkbd_set_repeat_rate

[Jeffin Philip]

atkbd_disable() marks atkbd as disabled to prevent any work
event to be executed, however this can race with atkbd_event_work().
If a pending work passes the atkbd->enabled check and atkbd_disable()
runs after in disconnect, it could dereference dev which is freed in
disconnect following atkbd being marked disabled. Fix this by adding
disable_delayed_work_sync() which drops all work events preventing
any rescheduling after atkbd is marked disabled.

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:
 - Changed in v4:
     Changed location of disable_pending_work_sync() after input_unregister_device()
   as per Dmitry's suggestion.
 
 - Changes in v3:
     Rewrote the patch to include disable_delayed_work_sync() as per
   Dmitry's suggestion.
 
 - Changes in v2:
     Added a check for atkbd->enabled so userspace cannot queue events after
   atkbd_disable.
---
 drivers/input/keyboard/atkbd.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 5736f4bc5a50..41a0d55dae0e 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -966,12 +966,9 @@ static void atkbd_disconnect(struct serio *serio)
 	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.
+	 * Prevent work from being rescheduled after atkbd is marked disabled
 	 */
-	cancel_delayed_work_sync(&atkbd->event_work);
+	disable_delayed_work_sync(&atkbd->event_work);
 
 	serio_close(serio);
 	serio_set_drvdata(serio, NULL);
-- 
2.55.0

Re: [PATCH v4] Input: atkbd - fix uaf in atkbd_set_repeat_rate

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fails to resolve the Use-After-Free (UAF) race condition it intends to fix because `disable_delayed_work_sync()` is incorrectly placed after `input_unregister_device()`.
--

commit 364595b57a843b20a975005b27712754f014825e
Author: Jeffin Philip <jeffinphilip14@gmail.com>

Input: atkbd - fix uaf in atkbd_set_repeat_rate
    
atkbd_disable() marks atkbd as disabled to prevent any work
event to be executed, however this can race with atkbd_event_work().
If a pending work passes the atkbd->enabled check and atkbd_disable()
runs after in disconnect, it could dereference dev which is freed in
disconnect following atkbd being marked disabled. Fix this by adding
disable_delayed_work_sync() which drops all work events preventing
any rescheduling after atkbd is marked disabled.
    
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>

> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
> index 3509b58e64920..0d75a7ba48d3c 100644
> --- a/drivers/input/keyboard/atkbd.c
> +++ b/drivers/input/keyboard/atkbd.c
> @@ -966,12 +966,9 @@ static void atkbd_disconnect(struct serio *serio)
>  	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.
> +	 * Prevent work from being rescheduled after atkbd is marked disabled
>  	 */
> -	cancel_delayed_work_sync(&atkbd->event_work);
> +	disable_delayed_work_sync(&atkbd->event_work);

[Severity: High]
This isn't a bug introduced by this patch, but does calling
disable_delayed_work_sync() after input_unregister_device() leave a
Use-After-Free race window open for the background work?

If atkbd_event_work() executes concurrently with atkbd_disconnect() and
passes the atkbd->enabled check, it can access atkbd->dev after it has
been freed.

For example, if atkbd_disconnect() calls input_unregister_device(atkbd->dev)
which drops the last reference and frees the device, the background work
could then access the freed dev in atkbd_set_leds():

drivers/input/keyboard/atkbd.c:atkbd_set_leds() {
    ...
	param[0] = (test_bit(LED_SCROLLL, dev->led) ? 1 : 0)
    ...
}

Should disable_delayed_work_sync() be moved above input_unregister_device()
to fully close this race window and ensure the work item cannot execute
after the device is freed?

>  
>  	serio_close(serio);
>  	serio_set_drvdata(serio, NULL);

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

Re: [PATCH v4] Input: atkbd - fix uaf in atkbd_set_repeat_rate

[Dmitry Torokhov]

On Sat, Aug 29, 2026 at 07:29:59PM +0530, Jeffin Philip wrote:
> atkbd_disable() marks atkbd as disabled to prevent any work
> event to be executed, however this can race with atkbd_event_work().
> If a pending work passes the atkbd->enabled check and atkbd_disable()
> runs after in disconnect, it could dereference dev which is freed in
> disconnect following atkbd being marked disabled. Fix this by adding
> disable_delayed_work_sync() which drops all work events preventing
> any rescheduling after atkbd is marked disabled.
> 
> 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:
>  - Changed in v4:
>      Changed location of disable_pending_work_sync() after input_unregister_device()
>    as per Dmitry's suggestion.

Actually I think I was wrong and Sashiko was right. I will probably pick
your v3 but I do not like that we do not clean up/reset LEDs properly
on disconnect. I will have to think about it some more...

Thanks.

-- 
Dmitry

Re: [PATCH v4] Input: atkbd - fix uaf in atkbd_set_repeat_rate

[Jeffin Philip]

On Sat, 29 Aug 2026 09:34:48 -0700, Dmitry Torokhov wrote:
>Actually I think I was wrong and Sashiko was right. I will probably pick
>your v3 but I do not like that we do not clean up/reset LEDs properly
>on disconnect.

Would cleaning up leds cause any weird issues on reconnect? We could reset
LEDs though. Compiled only, but this?

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 41a0d55dae0e..3d0e58be5ba2 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -963,13 +963,16 @@ static void atkbd_disconnect(struct serio *serio)
 
 	atkbd_disable(atkbd);
 
-	input_unregister_device(atkbd->dev);
-
 	/*
 	 * Prevent work from being rescheduled after atkbd is marked disabled
 	 */
 	disable_delayed_work_sync(&atkbd->event_work);
 
+	if (atkbd->write)
+		atkbd_set_leds(atkbd);
+
+	input_unregister_device(atkbd->dev);
+
 	serio_close(serio);
 	serio_set_drvdata(serio, NULL);
 	kfree(atkbd);

Thanks,
Jeffin.

Re: [PATCH v4] Input: atkbd - fix uaf in atkbd_set_repeat_rate

[Jeffin Philip]

On Sun, 30 Aug 2026 13:44:15 +0530, Jeffin Philip wrote:
>Would cleaning up leds cause any weird issues on reconnect?

Correction: Would _NOT_ cleaning up leds cause any weird issues
on reconnect?

Thanks,
Jeffin.