[PATCH v3 0/8] HID: asus: security fixes and more hardware support

[PATCH v3 0/8] HID: asus: security fixes and more hardware support

[Denis Benato]

Hi all,

I have added support for controlling the (way too bright) XG mobile
LEDs in hid-asus and added the i2c version of already supported
hardware that was probed only when it's a USB: these are two separate
features changes and are the only two that are not fixes for
pre-existing issue (see below).

Auto-review bot has spotted a bunch of pre-existing problems alongside
problems in my own code therefore at this point I am going to fix the
more problems I can and including those fixes and improvements in this
patchset.

Link: https://lore.kernel.org/all/20260612124830.984592-1-denis.benato@linux.dev/
Link: https://lore.kernel.org/all/30258f28-4036-4e99-9494-031212544dc2@linux.dev/

Best regards,
Denis Benato

Link v2: https://lore.kernel.org/all/20260612142326.1704858-1-denis.benato@linux.dev/

Changelog:
  - v1
    - Initial submission (as separate patches)
  - v2
    - HID: asus: add i2c entry for FA808UM and other TUFs
      - Joined to the patchset
    - HID: asus: add support for xgm led
      - Use the correct callback set_brightness_blocking
      - Stop hardcoding the cled name
    - HID: asus: prevent wrong pointer cast
      - Added
    - HID: asus: mitigate possible use-after-free
      - Added
  - v3
    - HID: asus: add support for xgm led
      - check devm_kasprintf result
    - HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free
      - Added
    - HID: asus: avoid sleeping calls in atomic context
      - Added
    - HID: asus: cleanup keyboard listener on failure: avoid use-after-free
      - Added

Denis Benato (8):
  HID: asus: mitigate possible use-after-free
  HID: asus: prevent wrong pointer cast
  HID: asus: add support for xgm led
  HID: asus: cleanup keyboard listener on failure: avoid use-after-free
  HID: asus: avoid sleeping calls in atomic context
  HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free
  HID: asus: add i2c entry for FA808UM and other TUFs
  HID: asus: remove unnecessary OOM message

 drivers/hid/hid-asus.c | 183 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 165 insertions(+), 18 deletions(-)

-- 
2.47.3

[PATCH v3 1/8] HID: asus: mitigate possible use-after-free

[Denis Benato]

The kbd_backlight pointer is not being set to NULL after being freed in
asus_kbd_register_leds and other portions of the driver checks that field,
therefore set the pointer to NULL after being freed.

Link: https://lore.kernel.org/all/20260612130125.AAAE71F000E9@smtp.kernel.org/

Suggested-by: sashiko-bot@kernel.org
Reviewed-by: Antheas Kapenekakis <lkml@antheas.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d34d74df3dc0..4d7110b892bd 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;
-- 
2.47.3

Re: [PATCH v3 1/8] HID: asus: mitigate possible use-after-free

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing initialization of `fn_lock_sync_work` leading to `WARN_ON` in `cancel_work_sync()`.
--

commit 317502709e07b71f7cfc9f30cf392aa744d4f88d
Author: Denis Benato <denis.benato@linux.dev>

HID: asus: mitigate possible use-after-free

The kbd_backlight pointer is not being set to NULL after being freed in
asus_kbd_register_leds and other portions of the driver checks that field,
therefore set the pointer to NULL after being freed.

Link: https://lore.kernel.org/all/20260612130125.AAAE71F000E9@smtp.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 since the patch touches driver teardown
safety, does the current teardown logic in asus_remove() safely handle
uninitialized work structs?

In drivers/hid/hid-asus.c:asus_input_configured(), the work struct is
conditionally initialized:

    if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
        drvdata->fn_lock = true;
        INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
        asus_kbd_set_fn_lock(hdev, true);
    }

However, for some devices (like those with QUIRK_ROG_NKEY_KEYBOARD and
QUIRK_HID_FN_LOCK quirks), the HID core might skip input checks if the
device does not have inputs. If asus_input_configured() is never called,
fn_lock_sync_work remains zero-initialized.

When the device is unbound, drivers/hid/hid-asus.c:asus_remove() calls:

    if (drvdata->quirks & QUIRK_HID_FN_LOCK)
        cancel_work_sync(&drvdata->fn_lock_sync_work);

Can this trigger a WARN_ON(!work->func) in __flush_work() if
cancel_work_sync() is called on the zero-initialized work struct,
potentially causing a panic on systems with panic_on_warn=1?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=1

Re: [PATCH v3 1/8] HID: asus: mitigate possible use-after-free

[Antheas Kapenekakis]

On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@linux.dev> wrote:
>
> The kbd_backlight pointer is not being set to NULL after being freed in
> asus_kbd_register_leds and other portions of the driver checks that field,
> therefore set the pointer to NULL after being freed.
>
> Link: https://lore.kernel.org/all/20260612130125.AAAE71F000E9@smtp.kernel.org/
>

If you do another revision, you should drop the space between the link and tags

> Suggested-by: sashiko-bot@kernel.org
> Reviewed-by: Antheas Kapenekakis <lkml@antheas.dev>
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> ---
>  drivers/hid/hid-asus.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index d34d74df3dc0..4d7110b892bd 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;
> --
> 2.47.3
>
>

[PATCH v3 2/8] HID: asus: prevent wrong pointer cast

[Denis Benato]

The firmware version check for ROG ally devices assumes the parent device
is a USB device, therefore prevent possible out-of-bounds access by using
hid_is_usb() to mitigate possible out-of-bounds access.

Link: https://lore.kernel.org/all/20260612130125.AAAE71F000E9@smtp.kernel.org/

Fixes: 00e005c952f7 ("hid-asus: check ROG Ally MCU version and warn")
Suggested-by: sashiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 4d7110b892bd..323dc0b7f3ff 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -753,7 +753,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 			return ret;
 	}
 
-	if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+	if (hid_is_usb(hdev) && (drvdata->quirks & QUIRK_ROG_ALLY_XPAD)) {
 		intf = to_usb_interface(hdev->dev.parent);
 		udev = interface_to_usbdev(intf);
 		validate_mcu_fw_version(hdev,
-- 
2.47.3

[PATCH v3 3/8] HID: asus: add support for xgm led

[Denis Benato]

XG mobile stations have very bright leds behind the fan that can be
turned either ON or OFF: add a cled interface to allow controlling the
brightness of those red leds.

Reviewed by: Antheas Kapenekakis <lkml@antheas.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 91 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 323dc0b7f3ff..0a6c97155549 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -51,6 +51,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define FEATURE_KBD_LED_REPORT_ID1 0x5d
 #define FEATURE_KBD_LED_REPORT_ID2 0x5e
 
+#define ROG_XGM_REPORT_SIZE 300
+
 #define ROG_ALLY_REPORT_SIZE 64
 #define ROG_ALLY_X_MIN_MCU 313
 #define ROG_ALLY_MIN_MCU 319
@@ -118,6 +120,11 @@ struct asus_kbd_leds {
 	bool removed;
 };
 
+struct asus_xgm_led {
+	struct led_classdev cdev;
+	struct hid_device *hdev;
+};
+
 struct asus_touchpad_info {
 	int max_x;
 	int max_y;
@@ -143,6 +150,7 @@ struct asus_drvdata {
 	unsigned long battery_next_query;
 	struct work_struct fn_lock_sync_work;
 	bool fn_lock;
+	struct asus_xgm_led *xgm_led;
 };
 
 static int asus_report_battery(struct asus_drvdata *, u8 *, int);
@@ -941,6 +949,23 @@ static int asus_battery_probe(struct hid_device *hdev)
 	return ret;
 }
 
+static int asus_xgm_led_set(struct led_classdev *led_cdev, enum led_brightness value)
+{
+	const u8 buf[ROG_XGM_REPORT_SIZE] = {
+		FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00
+	};
+	struct asus_xgm_led *xgm = container_of(led_cdev, struct asus_xgm_led, cdev);
+	int ret;
+
+	ret = asus_kbd_set_report(xgm->hdev, buf, ROG_XGM_REPORT_SIZE);
+	if (ret != ROG_XGM_REPORT_SIZE) {
+		hid_err(xgm->hdev, "Unable to set XG mobile led state: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
 {
 	struct input_dev *input = hi->input;
@@ -1169,6 +1194,51 @@ static int asus_start_multitouch(struct hid_device *hdev)
 	return 0;
 }
 
+static int asus_xgm_init(struct hid_device *hdev, struct asus_drvdata *drvdata)
+{
+	const char *name;
+	int ret;
+
+	drvdata->xgm_led = devm_kzalloc(&hdev->dev, sizeof(*drvdata->xgm_led), GFP_KERNEL);
+	if (drvdata->xgm_led == NULL)
+		return -ENOMEM;
+
+	name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "asus:xgm-%s:led",
+			      strlen(hdev->uniq) ? hdev->uniq : dev_name(&hdev->dev));
+
+	if (name == NULL) {
+		ret = -ENOMEM;
+		goto asus_xgm_init_err;
+	}
+
+	drvdata->xgm_led->hdev = hdev;
+	drvdata->xgm_led->cdev.name = name;
+	drvdata->xgm_led->cdev.brightness = 1;
+	drvdata->xgm_led->cdev.max_brightness = 1;
+	drvdata->xgm_led->cdev.brightness_set_blocking = asus_xgm_led_set;
+
+	/* LED state is arbitrary on boot, set a default */
+	ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
+	if (ret) {
+		hid_err(hdev, "Asus failed to set xgm led: %d\n", ret);
+		goto asus_xgm_init_led_err;
+	}
+
+	ret = led_classdev_register(&hdev->dev, &drvdata->xgm_led->cdev);
+	if (ret) {
+		hid_err(hdev, "Asus failed to register xgm led: %d\n", ret);
+		goto asus_xgm_init_led_err;
+	}
+
+	return 0;
+asus_xgm_init_led_err:
+	devm_kfree(&hdev->dev, name);
+asus_xgm_init_err:
+	devm_kfree(&hdev->dev, drvdata->xgm_led);
+	drvdata->xgm_led = NULL;
+	return ret;
+}
+
 static int __maybe_unused asus_resume(struct hid_device *hdev)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
@@ -1184,6 +1254,14 @@ static int __maybe_unused asus_resume(struct hid_device *hdev)
 		}
 	}
 
+	if (drvdata->xgm_led) {
+		ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
+		if (ret) {
+			hid_err(hdev, "Asus failed to restore xgm brightness: %d\n", ret);
+			goto asus_resume_err;
+		}
+	}
+
 asus_resume_err:
 	return ret;
 }
@@ -1310,6 +1388,16 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		}
 	}
 
+	if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
+	    ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
+	     (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
+		ret = asus_xgm_init(hdev, drvdata);
+		if (ret) {
+			hid_err(hdev, "Failed to initialize xg mobile: %d\n", ret);
+			goto err_stop_hw;
+		}
+	}
+
 	/* Laptops keyboard backlight is always at 0x5a */
 	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
 	    (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID)) &&
@@ -1366,6 +1454,9 @@ static void asus_remove(struct hid_device *hdev)
 	if (drvdata->quirks & QUIRK_HID_FN_LOCK)
 		cancel_work_sync(&drvdata->fn_lock_sync_work);
 
+	if (drvdata->xgm_led)
+		led_classdev_unregister(&drvdata->xgm_led->cdev);
+
 	hid_hw_stop(hdev);
 }
 
-- 
2.47.3

Re: [PATCH v3 3/8] HID: asus: add support for xgm led

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing led_classdev_unregister() in the probe error path leads to a use-after-free.
- [Medium] An error during keyboard backlight resume causes the XGM LED resume to be skipped.
- [Medium] Returning a positive short transfer length from brightness_set_blocking instead of a negative error code.
--

commit 201b953e7851d7aae6a4427d79499eefd3b7b433
Author: Denis Benato <denis.benato@linux.dev>

HID: asus: add support for xgm led

XG mobile stations have very bright leds behind the fan that can be
turned either ON or OFF: add a cled interface to allow controlling the
brightness of those red leds.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 52d755659d951..95e7dc6a447d8 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -941,6 +949,23 @@ static int asus_battery_probe(struct hid_device *hdev)
>  	return ret;
>  }
>  
> +static int asus_xgm_led_set(struct led_classdev *led_cdev, enum led_brightness value)
> +{
> +	const u8 buf[ROG_XGM_REPORT_SIZE] = {
> +		FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00
> +	};
> +	struct asus_xgm_led *xgm = container_of(led_cdev, struct asus_xgm_led, cdev);
> +	int ret;
> +
> +	ret = asus_kbd_set_report(xgm->hdev, buf, ROG_XGM_REPORT_SIZE);
> +	if (ret != ROG_XGM_REPORT_SIZE) {
> +		hid_err(xgm->hdev, "Unable to set XG mobile led state: %d\n", ret);
> +		return ret;

[Severity: Medium]
If a short transfer occurs, asus_kbd_set_report() will return a positive
number less than ROG_XGM_REPORT_SIZE. 

Will returning this positive value cause the LED subsystem API to incorrectly
interpret the short transfer as a success instead of a failure?

> +	}
> +
> +	return 0;
> +}
> +
[ ... ]
> @@ -1178,6 +1248,14 @@ static int __maybe_unused asus_resume(struct hid_device *hdev)
>  		}
>  	}
>  

[Severity: Medium]
If restoring the keyboard backlight fails earlier in asus_resume(), the
early return via goto asus_resume_err causes this XGM LED resume logic
to be skipped.

Should the device resume function try to resume as many independent
components as possible instead of unconditionally aborting here?

> +	if (drvdata->xgm_led) {
> +		ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
> +		if (ret) {
> +			hid_err(hdev, "Asus failed to restore xgm brightness: %d\n", ret);
> +			goto asus_resume_err;
> +		}
> +	}
> +
>  asus_resume_err:
>  	return ret;
>  }
[ ... ]
> @@ -1304,6 +1382,16 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		}
>  	}
>  
> +	if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
> +	    ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
> +	     (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
> +		ret = asus_xgm_init(hdev, drvdata);

[Severity: High]
If asus_probe() encounters an error later on (such as asus_start_multitouch()
failing), the error handling path does not unregister the LED class device.

Because drvdata->xgm_led was allocated with devm_kzalloc(), devres will
free it. Does this lead to a use-after-free if userspace interacts with the
sysfs interface of the leaked cdev structure?

Would using devm_led_classdev_register() in asus_xgm_init() instead of
led_classdev_register() cleanly avoid this issue?

> +		if (ret) {
> +			hid_err(hdev, "Failed to initialize xg mobile: %d\n", ret);
> +			goto err_stop_hw;
> +		}
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=3

[PATCH v3 4/8] HID: asus: cleanup keyboard listener on failure: avoid use-after-free

[Denis Benato]

asus_kbd_register_leds(), I noticed it registers a listener to a global list
and uses devm_kzalloc(). If a subsequent initialization step in asus_probe()
fails the driver returns without unregistering the listener, and the devres
subsystem will automatically free the memory, leaving a dangling pointer
in the global list.

Fixes: b34b5945a769 ("HID: asus: listen to the asus-wmi brightness device instead of creating one")
Reported-by: sahiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 0a6c97155549..f38b18ad65c6 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1426,11 +1426,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		if (drvdata->tp) {
 			ret = asus_start_multitouch(hdev);
 			if (ret)
-				goto err_stop_hw;
+				goto err_unregister_backlight;
 		}
 	}
 
 	return 0;
+err_unregister_backlight:
+	if (drvdata->kbd_backlight) {
+		asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
+		devm_kfree(&hdev->dev, drvdata->kbd_backlight);
+		drvdata->kbd_backlight = NULL;
+	}
 err_stop_hw:
 	hid_hw_stop(hdev);
 	return ret;
-- 
2.47.3

Re: [PATCH v3 4/8] HID: asus: cleanup keyboard listener on failure: avoid use-after-free

[sashiko-bot]

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

New issues:
- [High] The newly added `err_unregister_backlight` error path frees `drvdata->kbd_backlight` without canceling its associated workqueue, leading to a Use-After-Free.
- [High] The `asus_probe` error path fails to unregister the dynamically registered `xgm_led` class device, leaving a dangling pointer in the LED subsystem.

Pre-existing issues:
- [High] The `asus_probe` error paths fail to cancel `fn_lock_sync_work` and `wmi_work`, creating a potential Use-After-Free if the works are scheduled during probe.
--

commit 6e04c2967d627dea02497ed7471439a5d35c56bc
Author: Denis Benato <denis.benato@linux.dev>
Subject: HID: asus: cleanup keyboard listener on failure: avoid use-after-free

This commit fixes a potential use-after-free by unregistering the keyboard
listener on probe failure. It explicitly adds an error path to unregister
the listener and free the memory when a subsequent initialization step fails.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 95e7dc6a447d8..e3a03f7cf77d0 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1420,11 +1420,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		if (drvdata->tp) {
>  			ret = asus_start_multitouch(hdev);
>  			if (ret)
> -				goto err_stop_hw;
> +				goto err_unregister_backlight;
>  		}
>  	}
>  
>  	return 0;
> +err_unregister_backlight:
> +	if (drvdata->kbd_backlight) {
> +		asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> +		devm_kfree(&hdev->dev, drvdata->kbd_backlight);

[Severity: High]
Does this code leave pending work items referencing freed memory?

If concurrent events have queued `drvdata->kbd_backlight->work` (e.g.,
via `asus_kbd_backlight_set()` or `asus_hid_event()`), freeing
`kbd_backlight` without synchronizing via `cancel_work_sync()` might
cause a use-after-free when the workqueue executes.

> +		drvdata->kbd_backlight = NULL;
> +	}
>  err_stop_hw:
>  	hid_hw_stop(hdev);
>  	return ret;

[Severity: High]
Could failing to unregister `xgm_led` leave a dangling pointer in the LED
subsystem?

If `asus_xgm_init()` succeeds, it registers `drvdata->xgm_led->cdev` using
`led_classdev_register()`, which is not devres-managed. If `asus_probe()`
subsequently fails and jumps here, the code exits without calling
`led_classdev_unregister()`. Since `drvdata->xgm_led` is devres-managed, it
will be freed, but the LED subsystem will retain a pointer to it,
potentially causing a use-after-free.

[Severity: High]
This is a pre-existing issue, but does this error path also need to cancel
`fn_lock_sync_work` and `wmi_work`?

Once the hardware is started via `hid_hw_start()`, incoming input events
can schedule `drvdata->fn_lock_sync_work` or `drvdata->wmi_work`. If
`asus_probe()` fails and exits here, devres will automatically free
`drvdata`. Since `cancel_work_sync()` is omitted from this error path, any
queued work could execute asynchronously after `drvdata` is freed, leading
to a use-after-free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=4

Re: [PATCH v3 4/8] HID: asus: cleanup keyboard listener on failure: avoid use-after-free

[Antheas Kapenekakis]

On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@linux.dev> wrote:
>
> asus_kbd_register_leds(), I noticed it registers a listener to a global list
> and uses devm_kzalloc(). If a subsequent initialization step in asus_probe()
> fails the driver returns without unregistering the listener, and the devres
> subsystem will automatically free the memory, leaving a dangling pointer
> in the global list.
>
> Fixes: b34b5945a769 ("HID: asus: listen to the asus-wmi brightness device instead of creating one")
> Reported-by: sahiko-bot@kernel.org
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> ---
>  drivers/hid/hid-asus.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 0a6c97155549..f38b18ad65c6 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1426,11 +1426,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>                 if (drvdata->tp) {
>                         ret = asus_start_multitouch(hdev);
>                         if (ret)
> -                               goto err_stop_hw;
> +                               goto err_unregister_backlight;
>                 }

This block currently does two things, rename and init the touchpad.
Instead,  you may pull the touchpad init to be below the hid_hw_start
block and keep the same bail.

>         }
>
>         return 0;
> +err_unregister_backlight:
> +       if (drvdata->kbd_backlight) {
> +               asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> +               devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> +               drvdata->kbd_backlight = NULL;
> +       }
>  err_stop_hw:

OR add asus_hid_unregister_listener(&drvdata->kbd_backlight->listener) here.

Here, you'd also drop err_unregister_backlight, devm_kfree, and
nulling. The driver is closing. It is not necessary.

err_stop_hw is only used once, you do not need a secondary tag.

>         hid_hw_stop(hdev);
>         return ret;
> --
> 2.47.3
>
>

[PATCH v3 5/8] HID: asus: avoid sleeping calls in atomic context

[Denis Benato]

Avoid possibly calling asus_wmi_send_event(): a method that can sleep in
asus_raw_event() that is called in atomic context.

This commit changes when methods are called, not method parameters:
the driver behaves as before.

Fixes: 1489a34e97ef ("HID: asus: Implement Fn+F5 fan control key handler")
Reported-by: sahiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 67 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index f38b18ad65c6..a6467172c455 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -150,6 +150,13 @@ struct asus_drvdata {
 	unsigned long battery_next_query;
 	struct work_struct fn_lock_sync_work;
 	bool fn_lock;
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+	struct work_struct wmi_work;
+	bool wmi_work_disabled;
+	u8 wmi_data[FEATURE_KBD_REPORT_SIZE];
+	int wmi_size;
+	spinlock_t wmi_lock;
+#endif
 	struct asus_xgm_led *xgm_led;
 };
 
@@ -338,6 +345,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
 	return 0;
 }
 
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
 /*
  * Send events to asus-wmi driver for handling special keys
  */
@@ -361,6 +369,32 @@ static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
 	return 0;
 }
 
+static void asus_wmi_work(struct work_struct *work)
+{
+	struct asus_drvdata *drvdata = container_of(work, struct asus_drvdata, wmi_work);
+	u8 report_data[FEATURE_KBD_REPORT_SIZE];
+	int report_size;
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&drvdata->wmi_lock, flags);
+	memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
+	report_size = drvdata->wmi_size;
+	spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
+
+	ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
+	if (ret) {
+		if (ret != -ENODEV)
+			hid_warn(drvdata->hdev, "Failed to notify asus-wmi: %d\n", ret);
+
+		/* Fallback: pass the raw event to the HID core */
+		hid_report_raw_event(drvdata->hdev, HID_INPUT_REPORT,
+				     report_data, report_size,
+				     report_size, 1);
+	}
+}
+#endif
+
 static int asus_event(struct hid_device *hdev, struct hid_field *field,
 		      struct hid_usage *usage, __s32 value)
 {
@@ -397,6 +431,9 @@ static int asus_raw_event(struct hid_device *hdev,
 		struct hid_report *report, u8 *data, int size)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+	unsigned long flags;
+#endif
 
 	if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
 		return asus_report_battery(drvdata, data, size);
@@ -422,19 +459,18 @@ static int asus_raw_event(struct hid_device *hdev,
 			 * pass to userspace so it can implement its own fan control.
 			 */
 			if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
-				int ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
-
-				if (ret == 0) {
-					/* Successfully handled by asus-wmi, block event */
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+				spin_lock_irqsave(&drvdata->wmi_lock, flags);
+				memcpy(drvdata->wmi_data, data, min_t(int, size, sizeof(drvdata->wmi_data)));
+				drvdata->wmi_size = size;
+				spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
+
+				if (!drvdata->wmi_work_disabled) {
+					schedule_work(&drvdata->wmi_work);
+					/* Successfully handled asynchronously, block event */
 					return -1;
 				}
-
-				/*
-				 * Warn if asus-wmi failed (but not if it's unavailable).
-				 * Let the event reach userspace in all failure cases.
-				 */
-				if (ret != -ENODEV)
-					hid_warn(hdev, "Failed to notify asus-wmi: %d\n", ret);
+#endif
 			}
 
 			/*
@@ -1350,6 +1386,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
 
 	drvdata->hdev = hdev;
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+	INIT_WORK(&drvdata->wmi_work, asus_wmi_work);
+	spin_lock_init(&drvdata->wmi_lock);
+#endif
 
 	if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
 		ret = asus_battery_probe(hdev);
@@ -1460,6 +1500,11 @@ static void asus_remove(struct hid_device *hdev)
 	if (drvdata->quirks & QUIRK_HID_FN_LOCK)
 		cancel_work_sync(&drvdata->fn_lock_sync_work);
 
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+	drvdata->wmi_work_disabled = true;
+	cancel_work_sync(&drvdata->wmi_work);
+#endif
+
 	if (drvdata->xgm_led)
 		led_classdev_unregister(&drvdata->xgm_led->cdev);
 
-- 
2.47.3

Re: [PATCH v3 5/8] HID: asus: avoid sleeping calls in atomic context

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Unbounded wmi_size assignment causes a stack buffer overflow and out-of-bounds read in asus_wmi_work().
- [High] Asynchronous invocation of hid_report_raw_event without acquiring driver_input_lock causes a data race.
- [High] Missing cancel_work_sync in asus_probe error paths leaves wmi_work pending, causing a Use-After-Free.
- [Medium] Passing report_size as bufsize to hid_report_raw_event silently drops valid short reports.
--

commit 59c560d9da130f0e811e0abb99b0edf58eef0925
Author: Denis Benato <denis.benato@linux.dev>

HID: asus: avoid sleeping calls in atomic context

Avoid possibly calling asus_wmi_send_event(): a method that can sleep in
asus_raw_event() that is called in atomic context.

This commit changes when methods are called, not method parameters:
the driver behaves as before.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index e3a03f7cf77d0..a3437789b2341 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c

[ ... ]

> @@ -361,6 +369,32 @@ static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
>  	return 0;
>  }
>  
> +static void asus_wmi_work(struct work_struct *work)
> +{
> +	struct asus_drvdata *drvdata = container_of(work, struct asus_drvdata, wmi_work);
> +	u8 report_data[FEATURE_KBD_REPORT_SIZE];
> +	int report_size;
> +	unsigned long flags;
> +	int ret;
> +
> +	spin_lock_irqsave(&drvdata->wmi_lock, flags);
> +	memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);

[Severity: High]
Can this memcpy cause a stack buffer overflow?

In asus_raw_event(), drvdata->wmi_size is assigned the untrusted raw event
size without being clipped to the maximum buffer size. When the asynchronous
worker processes this, it uses that unclipped size as the length for memcpy
into the fixed 64-byte report_data stack array.

A malicious or malfunctioning USB device sending a report larger than
64 bytes could overflow the stack here.

> +	report_size = drvdata->wmi_size;
> +	spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> +	ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> +	if (ret) {
> +		if (ret != -ENODEV)
> +			hid_warn(drvdata->hdev, "Failed to notify asus-wmi: %d\n", ret);
> +
> +		/* Fallback: pass the raw event to the HID core */
> +		hid_report_raw_event(drvdata->hdev, HID_INPUT_REPORT,
> +				     report_data, report_size,
> +				     report_size, 1);

[Severity: High]
Is it safe to call hid_report_raw_event() asynchronously without acquiring
hdev->driver_input_lock?

The HID core expects callers of hid_report_raw_event() to hold this lock
to ensure exclusive access when modifying shared state within the HID report
structure. If a concurrent hardware interrupt triggers __hid_input_report(),
it could successfully acquire the lock and mutate the HID report state at the
same time, potentially leading to data corruption or crashes.

[Severity: Medium]
Does passing report_size as the bufsize parameter prevent the HID core from
safely handling short reports?

The HID core requires bufsize to be at least the expected size of the report
so it can safely zero-pad short reports. Because bufsize is constrained here
to the actual payload size, the safety check inside hid_report_raw_event()
might fail, causing valid short reports to be permanently dropped with an
EINVAL error.

> +	}
> +}
> +#endif
> +

[ ... ]

> @@ -422,19 +459,18 @@ static int asus_raw_event(struct hid_device *hdev,
>  			 * pass to userspace so it can implement its own fan control.
>  			 */
>  			if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
> -				int ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> -
> -				if (ret == 0) {
> -					/* Successfully handled by asus-wmi, block event */
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +				spin_lock_irqsave(&drvdata->wmi_lock, flags);
> +				memcpy(drvdata->wmi_data, data, min_t(int, size, sizeof(drvdata->wmi_data)));
> +				drvdata->wmi_size = size;

[Severity: High]
Should drvdata->wmi_size be capped to the size of the buffer?

While the memcpy directly above safely clips the copy using min_t(),
drvdata->wmi_size is assigned the unclipped size, leading to the stack buffer
overflow when it is consumed in asus_wmi_work().

> +				spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> +				if (!drvdata->wmi_work_disabled) {
> +					schedule_work(&drvdata->wmi_work);
> +					/* Successfully handled asynchronously, block event */
>  					return -1;
>  				}

[ ... ]

> @@ -1344,6 +1380,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
>  
>  	drvdata->hdev = hdev;
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +	INIT_WORK(&drvdata->wmi_work, asus_wmi_work);
> +	spin_lock_init(&drvdata->wmi_lock);
> +#endif

[Severity: High]
Could wmi_work trigger a use-after-free if probe fails during later stages?

In asus_probe(), after wmi_work is initialized, hid_hw_start() is called which
enables hardware interrupts. At this point, incoming events can queue wmi_work.

If a subsequent initialization step fails (for example, asus_xgm_init()),
the function jumps to the err_stop_hw label:

err_stop_hw:
    hid_hw_stop(hdev);
    return ret;

This returns an error without calling cancel_work_sync(&drvdata->wmi_work).
The workqueue subsystem guidelines state that once a work struct is queued,
shutdown methods must prevent leaking it. The drvdata memory will be freed
by devres, and any pending wmi_work will execute on freed memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=5

Re: [PATCH v3 5/8] HID: asus: avoid sleeping calls in atomic context

[Antheas Kapenekakis]

On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@linux.dev> wrote:
>
> Avoid possibly calling asus_wmi_send_event(): a method that can sleep in
> asus_raw_event() that is called in atomic context.
>
> This commit changes when methods are called, not method parameters:
> the driver behaves as before.

Observe the new AI guidelines and add assisted by tags if necessary.
Or write the patches manually for mainline. This sentence is an AI
euphemism

Who let 1489a34e97ef through? I implemented the plumbing for WMI
callbacks with asus_hid_event last year because WMI cannot be called
through HID. Brightness events had the exact same problem.
kbd_led_work/kbd_led_update_all could be renamed and used for this. I
would rather avoid introducing a new work queue. The one I added is
enough if not too much

Antheas

> Fixes: 1489a34e97ef ("HID: asus: Implement Fn+F5 fan control key handler")
> Reported-by: sahiko-bot@kernel.org
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> ---
>  drivers/hid/hid-asus.c | 67 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 56 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index f38b18ad65c6..a6467172c455 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -150,6 +150,13 @@ struct asus_drvdata {
>         unsigned long battery_next_query;
>         struct work_struct fn_lock_sync_work;
>         bool fn_lock;
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +       struct work_struct wmi_work;
> +       bool wmi_work_disabled;
> +       u8 wmi_data[FEATURE_KBD_REPORT_SIZE];
> +       int wmi_size;
> +       spinlock_t wmi_lock;
> +#endif
>         struct asus_xgm_led *xgm_led;
>  };
>
> @@ -338,6 +345,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
>         return 0;
>  }
>
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
>  /*
>   * Send events to asus-wmi driver for handling special keys
>   */
> @@ -361,6 +369,32 @@ static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
>         return 0;
>  }
>
> +static void asus_wmi_work(struct work_struct *work)
> +{
> +       struct asus_drvdata *drvdata = container_of(work, struct asus_drvdata, wmi_work);
> +       u8 report_data[FEATURE_KBD_REPORT_SIZE];
> +       int report_size;
> +       unsigned long flags;
> +       int ret;
> +
> +       spin_lock_irqsave(&drvdata->wmi_lock, flags);
> +       memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
> +       report_size = drvdata->wmi_size;
> +       spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> +       ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> +       if (ret) {
> +               if (ret != -ENODEV)
> +                       hid_warn(drvdata->hdev, "Failed to notify asus-wmi: %d\n", ret);
> +
> +               /* Fallback: pass the raw event to the HID core */
> +               hid_report_raw_event(drvdata->hdev, HID_INPUT_REPORT,
> +                                    report_data, report_size,
> +                                    report_size, 1);
> +       }
> +}
> +#endif
> +
>  static int asus_event(struct hid_device *hdev, struct hid_field *field,
>                       struct hid_usage *usage, __s32 value)
>  {
> @@ -397,6 +431,9 @@ static int asus_raw_event(struct hid_device *hdev,
>                 struct hid_report *report, u8 *data, int size)
>  {
>         struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +       unsigned long flags;
> +#endif
>
>         if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
>                 return asus_report_battery(drvdata, data, size);
> @@ -422,19 +459,18 @@ static int asus_raw_event(struct hid_device *hdev,
>                          * pass to userspace so it can implement its own fan control.
>                          */
>                         if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
> -                               int ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> -
> -                               if (ret == 0) {
> -                                       /* Successfully handled by asus-wmi, block event */
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +                               spin_lock_irqsave(&drvdata->wmi_lock, flags);
> +                               memcpy(drvdata->wmi_data, data, min_t(int, size, sizeof(drvdata->wmi_data)));
> +                               drvdata->wmi_size = size;
> +                               spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> +                               if (!drvdata->wmi_work_disabled) {
> +                                       schedule_work(&drvdata->wmi_work);
> +                                       /* Successfully handled asynchronously, block event */
>                                         return -1;
>                                 }
> -
> -                               /*
> -                                * Warn if asus-wmi failed (but not if it's unavailable).
> -                                * Let the event reach userspace in all failure cases.
> -                                */
> -                               if (ret != -ENODEV)
> -                                       hid_warn(hdev, "Failed to notify asus-wmi: %d\n", ret);
> +#endif
>                         }
>
>                         /*
> @@ -1350,6 +1386,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>                 hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
>
>         drvdata->hdev = hdev;
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +       INIT_WORK(&drvdata->wmi_work, asus_wmi_work);
> +       spin_lock_init(&drvdata->wmi_lock);
> +#endif
>
>         if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
>                 ret = asus_battery_probe(hdev);
> @@ -1460,6 +1500,11 @@ static void asus_remove(struct hid_device *hdev)
>         if (drvdata->quirks & QUIRK_HID_FN_LOCK)
>                 cancel_work_sync(&drvdata->fn_lock_sync_work);
>
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +       drvdata->wmi_work_disabled = true;
> +       cancel_work_sync(&drvdata->wmi_work);
> +#endif
> +
>         if (drvdata->xgm_led)
>                 led_classdev_unregister(&drvdata->xgm_led->cdev);
>
> --
> 2.47.3
>
>

[PATCH v3 6/8] HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free

[Denis Benato]

A late KEY_FN_ESC event can trigger asus_event() and unconditionally call
schedule_work() after the work was supposedly canceled.

Once the driver is removed and drvdata is freed by devres, the workqueue
could execute asus_sync_fn_lock() and dereference the freed drvdata.

Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
Reported-by: sahiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a6467172c455..a65a28137f0d 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -148,6 +148,7 @@ struct asus_drvdata {
 	int battery_stat;
 	bool battery_in_query;
 	unsigned long battery_next_query;
+	bool fn_lock_sync_work_disabled;
 	struct work_struct fn_lock_sync_work;
 	bool fn_lock;
 #if IS_REACHABLE(CONFIG_ASUS_WMI)
@@ -416,7 +417,7 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
 		case KEY_KBDILLUMTOGGLE:
 			return !asus_hid_event(ASUS_EV_BRTTOGGLE);
 		case KEY_FN_ESC:
-			if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
+			if (!drvdata->fn_lock_sync_work_disabled && (drvdata->quirks & QUIRK_HID_FN_LOCK)) {
 				drvdata->fn_lock = !drvdata->fn_lock;
 				schedule_work(&drvdata->fn_lock_sync_work);
 			}
@@ -1497,8 +1498,10 @@ static void asus_remove(struct hid_device *hdev)
 		cancel_work_sync(&drvdata->kbd_backlight->work);
 	}
 
-	if (drvdata->quirks & QUIRK_HID_FN_LOCK)
+	if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
+		drvdata->fn_lock_sync_work_disabled = true;
 		cancel_work_sync(&drvdata->fn_lock_sync_work);
+	}
 
 #if IS_REACHABLE(CONFIG_ASUS_WMI)
 	drvdata->wmi_work_disabled = true;
-- 
2.47.3

Re: [PATCH v3 6/8] HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free

[sashiko-bot]

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

New issues:
- [Critical] Unvalidated `size` from an untrusted HID event directly overwrites `drvdata->wmi_size`, leading to a stack buffer overflow in `asus_wmi_work()`.
- [High] Missing `led_classdev_unregister` for `xgm_led` on `asus_probe` error paths leading to a Use-After-Free.

Pre-existing issues:
- [High] Missing `cancel_work_sync` on the `asus_probe` error path can lead to a use-after-free of the `kbd_backlight->work` workqueue structure.
--

commit 621a1df344c100c1c65a96b5fd1d410d256fafef
Author: Denis Benato <denis.benato@linux.dev>

HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free

A late KEY_FN_ESC event can trigger asus_event() and unconditionally call
schedule_work() after the work was supposedly canceled.

Once the driver is removed and drvdata is freed by devres, the workqueue
could execute asus_sync_fn_lock() and dereference the freed drvdata.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index a3437789b2341..c1a247348c3d9 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c

[ ... ]

[Severity: Critical]
Can this lead to a stack buffer overflow?

Unvalidated size from an untrusted HID event directly overwrites
drvdata->wmi_size in the commit 'HID: asus: avoid sleeping calls in atomic
context':

drivers/hid/hid-asus.c:asus_raw_event() {
    ...
    if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
        ...
        drvdata->wmi_size = size;
    ...
}

Because size comes from the transport layer, a malicious USB device can
supply an event size up to HID_MAX_BUFFER_SIZE (4096 bytes).

Later, the asynchronously scheduled asus_wmi_work() uses this oversized
wmi_size:

drivers/hid/hid-asus.c:asus_wmi_work() {
    ...
    u8 report_data[FEATURE_KBD_REPORT_SIZE];
    ...
    memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
    ...
}

Will this overflow the 64-byte stack buffer report_data?

[Severity: High]
This is a pre-existing issue, but does this error path need a call to
cancel_work_sync(&drvdata->kbd_backlight->work)?

drivers/hid/hid-asus.c:asus_probe() {
    ...
err_unregister_backlight:
    if (drvdata->kbd_backlight) {
        asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
        devm_kfree(&hdev->dev, drvdata->kbd_backlight);
        drvdata->kbd_backlight = NULL;
    }
    ...
}

If a subsequent initialization step fails, the error path unregisters the
listener and frees the backlight structure.

If the work was queued prior to the failure, wouldn't the workqueue core
execute the pending work on the newly freed memory?

[Severity: High]
Does this code leave a dangling pointer in the LED subsystem list?

drivers/hid/hid-asus.c:asus_probe() {
    ...
err_stop_hw:
    hid_hw_stop(hdev);
    return ret;
}

In the commit 'HID: asus: add support for xgm led', asus_xgm_init()
allocates drvdata->xgm_led using devm_kzalloc() and registers it via
led_classdev_register().

If a subsequent initialization step fails, the error handling paths branch
to err_unregister_backlight and err_stop_hw without calling
led_classdev_unregister() for the XGM LED.

When asus_probe() returns, devres will automatically free drvdata->xgm_led,
but won't unregister the device, which could leave a dangling pointer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=6

Re: [PATCH v3 6/8] HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free

[Antheas Kapenekakis]

On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@linux.dev> wrote:
>
> A late KEY_FN_ESC event can trigger asus_event() and unconditionally call
> schedule_work() after the work was supposedly canceled.
>
> Once the driver is removed and drvdata is freed by devres, the workqueue
> could execute asus_sync_fn_lock() and dereference the freed drvdata.
>
> Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
> Reported-by: sahiko-bot@kernel.org
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> ---
>  drivers/hid/hid-asus.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index a6467172c455..a65a28137f0d 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -148,6 +148,7 @@ struct asus_drvdata {
>         int battery_stat;
>         bool battery_in_query;
>         unsigned long battery_next_query;
> +       bool fn_lock_sync_work_disabled;
>         struct work_struct fn_lock_sync_work;
>         bool fn_lock;
>  #if IS_REACHABLE(CONFIG_ASUS_WMI)
> @@ -416,7 +417,7 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
>                 case KEY_KBDILLUMTOGGLE:
>                         return !asus_hid_event(ASUS_EV_BRTTOGGLE);
>                 case KEY_FN_ESC:
> -                       if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
> +                       if (!drvdata->fn_lock_sync_work_disabled && (drvdata->quirks & QUIRK_HID_FN_LOCK)) {
>                                 drvdata->fn_lock = !drvdata->fn_lock;
>                                 schedule_work(&drvdata->fn_lock_sync_work);
>                         }
> @@ -1497,8 +1498,10 @@ static void asus_remove(struct hid_device *hdev)
>                 cancel_work_sync(&drvdata->kbd_backlight->work);
>         }
>
> -       if (drvdata->quirks & QUIRK_HID_FN_LOCK)
> +       if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
> +               drvdata->fn_lock_sync_work_disabled = true;
>                 cancel_work_sync(&drvdata->fn_lock_sync_work);
> +       }

I am not sure this is a real bug. can _event be called during _remove?
Doesn't the core prevent that?

>  #if IS_REACHABLE(CONFIG_ASUS_WMI)
>         drvdata->wmi_work_disabled = true;
> --
> 2.47.3
>
>

[PATCH v3 7/8] HID: asus: add i2c entry for FA808UM and other TUFs

[Denis Benato]

On newer TUF laptops the keyboard HID device uses the same PID/VID of a
USB device that was found in ROG laptops: add it to hid-asus as i2c too.

Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a65a28137f0d..efad6b623e65 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1646,6 +1646,9 @@ static const struct hid_device_id asus_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
+	{ HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
+	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
+	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
-- 
2.47.3

Re: [PATCH v3 7/8] HID: asus: add i2c entry for FA808UM and other TUFs

[sashiko-bot]

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

Pre-existing issues:
- [High] NULL pointer dereference or UAF in asus_e1239t_event when handling a Medion E1239T device with no input usages
--

commit f6d872852d0965599a5f0e0ef39dc3d4ec54c196
Author: Denis Benato <denis.benato@linux.dev>

HID: asus: add i2c entry for FA808UM and other TUFs

On newer TUF laptops the keyboard HID device uses the same PID/VID of a USB
device that was found in ROG laptops. This patch adds the device to hid-asus
as an I2C device to enable support.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index c1a247348c3d9..639d42708b5fc 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1640,6 +1640,9 @@ static const struct hid_device_id asus_devices[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>  	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
>  	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
> +	{ HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
> +	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
> +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>  	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
>  	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },

[ ... ]

[Severity: High]
This is a pre-existing issue, but could asus_e1239t_event() trigger a NULL
pointer dereference or use-after-free when handling a Medion E1239T device
with no input usages?

If a malicious Medion E1239T device provides a HID descriptor with no
recognized input usages, asus_input_configured() is not called, or the input
is freed by the core. This leaves drvdata->input NULL or dangling.

However, hid_hw_start() will still succeed by claiming the device as a hidraw
device. When a raw event (such as a mute key press) is received,
asus_raw_event() routes it to asus_e1239t_event().

This function unconditionally dereferences the input pointer:

drivers/hid/hid-asus.c:asus_e1239t_event() {
    ...
    input_report_key(drvdat->input, KEY_MUTE, 1);
    ...
}

Is it possible for this raw event routing to dereference drvdata->input
without validating if it is non-NULL or if the input was properly claimed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=7

[PATCH v3 8/8] HID: asus: remove unnecessary OOM message

[Denis Benato]

If devm_kzalloc fails an allocation error is already being reported:
there is no need for the driver to repeat it, moreover if this were
new code checkpatch.pl would throw a warning.

Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index efad6b623e65..f1fe3358a298 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1322,10 +1322,8 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	int ret;
 
 	drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
-	if (drvdata == NULL) {
-		hid_err(hdev, "Can't alloc Asus descriptor\n");
+	if (drvdata == NULL)
 		return -ENOMEM;
-	}
 
 	hid_set_drvdata(hdev, drvdata);
 
-- 
2.47.3