Re: [PATCH 1/3] HID: asus: fix missing hid_is_usb() check
From: sashiko-bot@kernel.org
Date: 2026-07-03 15:32:18
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Missing pointer invalidation after `devm_kfree` causes a deterministic Use-After-Free on the failure path. -- commit d3bce769a13470fb7e8717746a98294d5eb006c9 Author: Jann Horn [off-list ref] HID: asus: fix missing hid_is_usb() check to_usb_interface() can only be used on a hid_device whose parent is really USB; uhid can create devices that identify as being on BUS_USB, but don't actually have a USB parent. Fix the use of to_usb_interface() without a hid_is_usb() check.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index 3f5e96900b67a..befa990b3210e 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 ((drvdata->quirks & QUIRK_ROG_ALLY_XPAD) && hid_is_usb(hdev)) { intf = to_usb_interface(hdev->dev.parent); udev = interface_to_usbdev(intf); validate_mcu_fw_version(hdev,
[Severity: High]
This is a pre-existing issue, but does the failure path in this function leave
a dangling pointer that can cause a use-after-free?
If asus_hid_register_listener() fails later in asus_kbd_register_leds():
ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
if (ret < 0) {
/* No need to have this still around */
devm_kfree(&hdev->dev, drvdata->kbd_backlight);
}
The memory allocated for drvdata->kbd_backlight is freed, but the pointer is
not set to NULL.
Then asus_probe() catches the failure, emits a warning, but improperly allows
probe to succeed:
if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
(asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID)) &&
(asus_kbd_register_leds(hdev)))
hid_warn(hdev, "Failed to initialize backlight.\n");
If this occurs, could a subsequent call to asus_remove() or asus_resume()
dereference the freed memory?
if (drvdata->kbd_backlight) {
asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
drvdata->kbd_backlight->removed = true;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-hid-usbcheck-v1-0-e80259ff625d@google.com?part=1