Re: [PATCH v7 7/8] HID: steelseries: Add async status interface support
From: sashiko-bot@kernel.org
Date: 2026-08-03 19:30:00
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The `steelseries_get_sibling_sd` function uses `usb_get_intfdata` on a sibling USB interface and unconditionally casts the result to `struct hid_device *`, which leads to type confusion if a different driver binds to it. - [High] A concurrent sysfs unbind of the HID driver from the sync interface can cause `steelseries_get_sibling_sd` to return a freed `steelseries_device` (sd) and call `kref_get` on it, leading to a Use-After-Free. --
--- Patch [7]: [PATCH v7 7/8] HID: steelseries: Add async status interface support ---commit 816a244e480f8271ebf1dbc58fbdbe5e0bdbafab Author: Sriman Achanta [off-list ref] HID: steelseries: Add async status interface support Some headsets expose a second HID interface that sends battery and connection updates on its own. Watching that interface lets the driver stop polling the sync interface.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c --- a/drivers/hid/hid-steelseries-arctis.c +++ b/drivers/hid/hid-steelseries-arctis.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -318,6 +320,53 @@ static int steelseries_battery_register(struct steelseries_device *sd) +static struct steelseries_device * +steelseries_get_sibling_sd(struct hid_device *hdev, int interface_num) +{ + struct usb_interface *intf = to_usb_interface(hdev->dev.parent); + struct usb_device *usb_dev = interface_to_usbdev(intf); + struct usb_interface *sibling_intf; + struct hid_device *sibling_hdev; + struct steelseries_device *sd = NULL; + + sibling_intf = usb_ifnum_to_if(usb_dev, interface_num); + if (!sibling_intf) + return NULL;
[ ... ]
+ if (!device_trylock(&sibling_intf->dev))
+ return NULL;
+ if (sibling_intf->dev.driver) {
+ sibling_hdev = usb_get_intfdata(sibling_intf);
+ if (sibling_hdev &&
+ sibling_hdev->driver == &steelseries_arctis_driver) {[Severity: High] Is it safe to assume sibling_intf is bound to usbhid here? If an attacker plugs in a crafted USB device, the async interface could be bound to a different driver (like usbfs or usbmouse). Since usb_get_intfdata() is unconditionally treated as a struct hid_device pointer, checking sibling_hdev->driver could result in an out-of-bounds read if the bound driver's data structure is smaller.
+ sd = hid_get_drvdata(sibling_hdev); + if (sd) + kref_get(&sd->refcnt);
[Severity: High] Could a concurrent unbind of the HID driver cause a use-after-free on sd? While the USB interface lock (&sibling_intf->dev) is held here, this doesn't serialize against a sysfs unbind of the HID driver. If a user unbinds the HID driver from the sibling interface via sysfs, the HID core's hid_device_remove() calls the driver's remove callback (which drops the refcount and frees sd), and only clears hdev->driver afterwards. This could allow hid_get_drvdata() to return a pointer to a freed steelseries_device, leading to a kref_get() on freed memory.
+ } + } + device_unlock(&sibling_intf->dev); + + return sd; +}
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260803191553.66368-1-srimanachanta@gmail.com?part=7