Re: [PATCH v5 4/5] HID: asus: add support for xgm led
From: sashiko-bot@kernel.org
Date: 2026-06-19 00:23:04
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] A failure to initialize the cosmetic XG Mobile LED aborts the entire HID device probe.
- [High] Failing to restore the XGM LED brightness causes the entire HID device resume process to fail.
Pre-existing issues:
- [High] The devm-managed battery is not explicitly unregistered before stopping the hardware, leading to use-after-stop during teardown.
--
commit 7aa41eaa0bd3e715bef4c850703c7d47445cad14
Author: Denis Benato [off-list ref]
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.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index 0fb8cd6437b77..06e4888f090ca 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1361,7 +1432,17 @@ static int __maybe_unused asus_resume(struct hid_device *hdev) if (drvdata->listener.brightness_set) asus_kbd_backlight_set(&drvdata->listener, drvdata->kbd_backlight_brightness); + 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; + } + } + return 0; +asus_resume_err: + return ret;
[Severity: High] In asus_resume(), if asus_xgm_led_set() fails to restore the LED state, returning an error causes the entire HID device resume process to fail. Will this leave the device in an inconsistent state and break input functionality? As documented in the comments for the backlight, non-fatal LED state restorations shouldn't fail the system resume. Could this just ignore the error here as well? [ ... ]
quoted hunk ↗ jump to hunk
@@ -1491,6 +1572,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; + } + } +
[Severity: High] If asus_xgm_init() fails due to a transient USB error during the initial asus_xgm_led_set() call, will this completely abort the HID device probe and disable the keyboard? Since similar features like the keyboard backlight only emit a warning on failure, should a non-fatal cosmetic LED initialization failure also just warn and allow the driver to continue normal operation? [ ... ]
quoted hunk ↗ jump to hunk
@@ -1540,6 +1631,9 @@ static void asus_remove(struct hid_device *hdev) if (drvdata->listener.brightness_set) asus_hid_unregister_listener(&drvdata->listener); + if (drvdata->xgm_led) + devm_led_classdev_unregister(&hdev->dev, &drvdata->xgm_led->cdev); + asus_worker_stop(drvdata->worker); hid_hw_stop(hdev); }
[Severity: High] This is a pre-existing issue, but the devm-managed battery is not explicitly unregistered before stopping the hardware. Because the battery is registered via devm_power_supply_register() before hid_hw_start(), and hid_hw_stop() is called here before the devres cleanup, could this lead to a use-after-stop? If a concurrent read of the battery sysfs properties happens during teardown, it triggers asus_battery_get_property() which calls hid_hw_raw_request() on a stopped HID device. Can this cause bus hangs or undefined behavior? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260619001103.1189200-1-denis.benato@linux.dev?part=4