Re: [PATCH 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] Missing pointer invalidation after `devm_kfree` causes a deterministic Use-After-Free on the failure path.
--

commit d3bce769a13470fb7e8717746a98294d5eb006c9
Author: Jann Horn <jannh@google.com>

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.

> 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