Re: [PATCH] HID: asus: fix UAF of ->kbd_backlight on !CONFIG_ASUS_WMI

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Race condition in asus_remove() due to incorrect teardown order for fn_lock_sync_work, leading to Use-After-Free.
--

commit 10088228c9fb50f752e52778b8014fc75f17200d
Author: Jann Horn <jannh@google.com>

HID: asus: fix UAF of ->kbd_backlight on !CONFIG_ASUS_WMI

This commit fixes a Use-After-Free bug where drvdata->kbd_backlight was not
set to NULL after being freed in asus_kbd_register_leds(). By explicitly
clearing the pointer when asus_hid_register_listener() fails, it prevents
asus_remove() from following a freed pointer during teardown.

Cc: stable@vger.kernel.org

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 3f5e96900b67a..c2a5edebbd7a7 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -777,6 +777,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>  	if (ret < 0) {
>  		/* No need to have this still around */
>  		devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> +		drvdata->kbd_backlight = NULL;
>  	}
>  
>  	return ret;

[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition in
asus_remove() that might lead to a Use-After-Free.

When a device is removed, asus_remove() calls cancel_work_sync() for
fn_lock_sync_work before stopping the hardware:

drivers/hid/hid-asus.c:asus_remove() {
    ...
	if (drvdata->quirks & QUIRK_HID_FN_LOCK)
		cancel_work_sync(&drvdata->fn_lock_sync_work);

	hid_hw_stop(hdev);
}

If a KEY_FN_ESC event arrives in the window between these two calls, won't
the HID core invoke asus_event() and blindly requeue the work?

drivers/hid/hid-asus.c:asus_event() {
    ...
		case KEY_FN_ESC:
			if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
				drvdata->fn_lock = !drvdata->fn_lock;
				schedule_work(&drvdata->fn_lock_sync_work);
			}
			break;
    ...
}

Since the driver data is freed after device removal, could this executed
work access freed memory if the teardown order isn't reversed so that
hid_hw_stop() runs before cancel_work_sync()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-hid-asus-uaf-v1-1-1a2af2534eb0@google.com?part=1