[PATCH v2 0/3] hid: fix missing hid_is_usb() checks in three drivers

[PATCH v2 0/3] hid: fix missing hid_is_usb() checks in three drivers

[Jann Horn]

This fixes missing hid_is_usb() checks before to_usb_interface() in
three HID drivers.
I've split it into three patches so that they can have separate "Fixes"
tags, hopefully they are easier to stable-backport this way.

Signed-off-by: Jann Horn <jannh@google.com>
---
Changes in v2:
- patch 3/3: fix typo in "Fixes" line
- patch 3/3: add USB_HID dependency (alternative would be to implement a
  stub for hid_is_usb())
- Link to v1: https://patch.msgid.link/20260703-hid-usbcheck-v1-0-e80259ff625d@google.com

---
Jann Horn (3):
      HID: asus: fix missing hid_is_usb() check
      HID: huawei: fix missing hid_is_usb() check
      HID: rapoo: fix missing hid_is_usb() check

 drivers/hid/Kconfig      | 1 +
 drivers/hid/hid-asus.c   | 2 +-
 drivers/hid/hid-huawei.c | 5 +++--
 drivers/hid/hid-rapoo.c  | 2 +-
 4 files changed, 6 insertions(+), 4 deletions(-)
---
base-commit: 51512e22efe813d8223de27f6fd02a8a48ea2323
change-id: 20260703-hid-usbcheck-9163e6cf6015

Best regards,
--  
Jann Horn <jannh@google.com>

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

[Jann Horn]

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.

I have verified that it is currently possible to trigger a kernel splat due
to this bug in an ASAN build, and that this commit fixes the issue.

Fixes: 00e005c952f7 ("hid-asus: check ROG Ally MCU version and warn")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
 drivers/hid/hid-asus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 3f5e96900b67..befa990b3210 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,

-- 
2.55.0.rc0.799.gd6f94ed593-goog

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

[sashiko-bot]

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

[PATCH v2 2/3] HID: huawei: fix missing hid_is_usb() check

[Jann Horn]

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.

I have verified that it is currently possible to trigger a kernel splat due
to this bug in an ASAN build, and that this commit fixes the issue.

Fixes: e93faaca84b7 ("HID: huawei: fix CD30 keyboard report descriptor issue")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
 drivers/hid/hid-huawei.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-huawei.c b/drivers/hid/hid-huawei.c
index 6a616bf21b38..ee3fc6f68475 100644
--- a/drivers/hid/hid-huawei.c
+++ b/drivers/hid/hid-huawei.c
@@ -44,11 +44,12 @@ static const __u8 huawei_cd30_kbd_rdesc_fixed[] = {
 static const __u8 *huawei_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 				  unsigned int *rsize)
 {
-	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+	struct usb_interface *intf = hid_is_usb(hdev) ?
+			to_usb_interface(hdev->dev.parent) : NULL;
 
 	switch (hdev->product) {
 	case USB_DEVICE_ID_HUAWEI_CD30KBD:
-		if (intf->cur_altsetting->desc.bInterfaceNumber == 1) {
+		if (!intf || intf->cur_altsetting->desc.bInterfaceNumber == 1) {
 			if (*rsize != sizeof(huawei_cd30_kbd_rdesc_fixed) ||
 				memcmp(huawei_cd30_kbd_rdesc_fixed, rdesc,
 					sizeof(huawei_cd30_kbd_rdesc_fixed)) != 0) {

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 3/3] HID: rapoo: fix missing hid_is_usb() check

[Jann Horn]

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.

Add a dependency on USB_HID for hid_is_usb(), as other HID drivers do; the
alternative would be to provide a simple stub implementation on !USB_HID
builds.

I have verified that it is currently possible to trigger a kernel splat due
to this bug in an ASAN build, and that this commit fixes the issue.

Fixes: b3b1c68fb726 ("HID: rapoo: Add support for side buttons on RAPOO 0x2015 mouse")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
 drivers/hid/Kconfig     | 1 +
 drivers/hid/hid-rapoo.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index f9bcaeb66385..48934c4f3c45 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1048,6 +1048,7 @@ config HID_PXRC
 
 config HID_RAPOO
 	tristate "Rapoo non-fully HID-compliant devices"
+	depends on USB_HID
 	help
 	Support for Rapoo devices that are not fully compliant with the
 	HID standard.
diff --git a/drivers/hid/hid-rapoo.c b/drivers/hid/hid-rapoo.c
index 4c81f3086de4..5c9c396fabf7 100644
--- a/drivers/hid/hid-rapoo.c
+++ b/drivers/hid/hid-rapoo.c
@@ -36,7 +36,7 @@ static int rapoo_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return ret;
 	}
 
-	if (hdev->bus == BUS_USB) {
+	if (hid_is_usb(hdev)) {
 		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
 
 		if (intf->cur_altsetting->desc.bInterfaceNumber != 1)

-- 
2.55.0.rc0.799.gd6f94ed593-goog

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing hid_hw_stop() cleanup on error paths in rapoo_probe() causes hardware resource leaks and a Use-After-Free.
--

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

HID: rapoo: fix missing hid_is_usb() check

This commit adds a hid_is_usb() check before calling to_usb_interface() to
handle uhid devices reporting BUS_USB without an actual USB parent. It also
adds a USB_HID dependency for hid_is_usb().

[ ... ]

> diff --git a/drivers/hid/hid-rapoo.c b/drivers/hid/hid-rapoo.c
> index 4c81f3086de43..5c9c396fabf77 100644
> --- a/drivers/hid/hid-rapoo.c
> +++ b/drivers/hid/hid-rapoo.c
> @@ -36,7 +36,7 @@ static int rapoo_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		return ret;
>  	}
>  
> -	if (hdev->bus == BUS_USB) {
> +	if (hid_is_usb(hdev)) {
>  		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
>  
>  		if (intf->cur_altsetting->desc.bInterfaceNumber != 1)

[Severity: High]
This is a pre-existing issue, but are we missing hid_hw_stop() calls on the
error paths later in rapoo_probe()?

If devm_input_allocate_device() fails, the code returns -ENOMEM directly:

	input = devm_input_allocate_device(&hdev->dev);
	if (!input)
		return -ENOMEM;

Similarly, if input_register_device() fails, it returns the error:

	ret = input_register_device(input);
	if (ret)
		return ret;

When returning these errors after hid_hw_start() has succeeded, the HID core
probe fails and sets hdev->driver to NULL, but hid_hw_stop() is never called
to stop the hardware and kill active URBs.

When the device is later unplugged, usbhid_disconnect() frees the usbhid
structure, but skips hid_hw_stop() because hdev->driver is NULL.

Because active USB URBs are not killed, when the USB core later flushes the
endpoints and cancels the URBs, the completion handler hid_irq_in() attempts
to clear state bits in the already-freed usbhid structure, like calling
clear_bit(HID_IN_RUNNING, &usbhid->iofl).

This results in a use-after-free write memory corruption. While this requires
local physical access to the USB port and specific conditions like memory
pressure to induce a probe failure, it guarantees memory corruption when
triggered.

Would it be better to add an error label that calls hid_hw_stop() when the
input device allocation or registration fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-hid-usbcheck-v2-0-c5ed7bc94772@google.com?part=3