Thread (6 messages) 6 messages, 2 authors, 25d ago

Re: [PATCH v2 1/3] HID: asus: fix missing hid_is_usb() check

From: sashiko-bot@kernel.org
Date: 2026-07-03 15:58:08

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

Pre-existing issues:
- [High] Dangling pointer in `drvdata->kbd_backlight` leading to Use-After-Free when WMI listener registration fails.
--

commit 0bcf68d0cd670d6cc4fac38b7be14d0855140731
Author: Jann Horn [off-list ref]

HID: asus: fix missing hid_is_usb() check

This commit fixes a kernel splat caused by uhid devices that identify as
being on BUS_USB without actually having a USB parent. It adds a missing
hid_is_usb() check before calling to_usb_interface().

Cc: stable@vger.kernel.org
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 isn't a bug introduced by this patch, but I noticed a potential
use-after-free later in this same function.

When asus_hid_register_listener() fails, the code frees the backlight 
structure but leaves the pointer dangling:

drivers/hid/hid-asus.c: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);
    }
    return ret;
}

Since asus_probe() logs a warning but continues successfully when this
fails, could this cause a use-after-free during device removal or
suspend?

It looks like asus_remove() checks if the pointer is non-null before
dereferencing it:

drivers/hid/hid-asus.c:asus_remove() {
    ...
    if (drvdata->kbd_backlight) {
        asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
        spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
    ...
}

Should drvdata->kbd_backlight be set to NULL after being freed to prevent
this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-hid-usbcheck-v2-0-c5ed7bc94772@google.com?part=1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help