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

[sashiko-bot@kernel.org]

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 <jannh@google.com>

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

> 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