Re: [PATCH v6 1/9] HID: steelseries: Fix ARCTIS_1_X device mislabeling

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The probe function leaks hardware state and input devices if `hid_hw_open()` fails, leading to Use-After-Free.
- [High] Failure to register the power supply leaves an ERR_PTR and an uninitialized delayed_work, causing kernel panics on incoming events or device removal.
--

--- Patch [1]: [PATCH v6 1/9] HID: steelseries: Fix ARCTIS_1_X device mislabeling ---
commit c38516edb3d8e3340b9141595b245b3a3b7f06a6
Author: Sriman Achanta <srimanachanta@gmail.com>

HID: steelseries: Fix ARCTIS_1_X device mislabeling

The SteelSeries Arctis 1 Wireless for Xbox (0x12b6) was labelled as the
plain Arctis 1 Wireless. Rename USB_DEVICE_ID_STEELSERIES_ARCTIS_1 to
USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X, along with the matching quirk flag
and device table entry. The device ID value is unchanged.

> diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
> index f98435631aa18..fd38ee3ea6fcb 100644
> --- a/drivers/hid/hid-steelseries.c
> +++ b/drivers/hid/hid-steelseries.c

[ ... ]

> @@ -638,7 +638,7 @@ static int steelseries_headset_raw_event(struct hid_device *hdev,
>  	if (hdev->product == USB_DEVICE_ID_STEELSERIES_SRWS1)
>  		return 0;
>  
> -	if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1) {
> +	if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X) {
>  		hid_dbg(sd->hdev,
>  			"Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in how
headset events are handled if battery registration fails.

Looking at steelseries_probe(), if the battery registration fails it logs an
error but returns success:

drivers/hid/hid-steelseries.c:steelseries_probe() {
    ...
    if (steelseries_headset_battery_register(sd) < 0)
        hid_err(sd->hdev,
            "Failed to register battery for headset\n");

    return ret;
}

This leaves sd->battery containing an error pointer, and skips initializing
the battery_work structure. When an event is received later, the headset raw
event handler proceeds to use these uninitialized members:

drivers/hid/hid-steelseries.c:steelseries_headset_raw_event() {
    ...
    power_supply_changed(sd->battery);
    ...
    schedule_delayed_work(&sd->battery_work,
            msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
}

Could this cause a panic from dereferencing the error pointer in
power_supply_changed(), and trigger an oops when schedule_delayed_work()
runs on the uninitialized work structure?

[Severity: High]
This is another pre-existing issue, but does steelseries_probe() properly
clean up resources if hardware open fails?

drivers/hid/hid-steelseries.c:steelseries_probe() {
    ...
    ret = hid_hw_open(hdev);
    if (ret)
        return ret;
    ...
}

If hid_hw_open() fails, the function returns directly. Would this bypass
calling hid_hw_stop() to unregister the devices allocated by the earlier
call to hid_hw_start()? 

Could this leak the input devices and cause a use-after-free when the
parent device is subsequently removed?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com?part=1