Re: [PATCH v4 08/10] HID: steelseries: Manage battery lifetime with refcounting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Dangling pointer left in device driver data on probe error path leads to Use-After-Free.
- [High] Race condition in sibling interface lookup during probe leading to Use-After-Free of `hid_device`.
--

commit 5a206f2162dc642386285174c7af698f767dad15
Author: Sriman Achanta <srimanachanta@gmail.com>

HID: steelseries: Manage battery lifetime with refcounting

The next change shares one steelseries_device between two HID interfaces,
so the state can outlive either interface. Stop using devm for it. Reference
count the struct with a kref and free it from steelseries_device_release().

> diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
> index 1f0e9cb5138f6..734cf1eb87894 100644
> --- a/drivers/hid/hid-steelseries-arctis.c
> +++ b/drivers/hid/hid-steelseries-arctis.c
[ ... ]
> @@ -361,12 +373,15 @@ static int steelseries_arctis_probe(struct hid_device *hdev,
>  
>  err_stop:
>  	hid_hw_stop(hdev);
> +err_put:
> +	kref_put(&sd->refcnt, steelseries_device_release);

[Severity: High]
Does this error path leave a dangling pointer in the device's driver data?

If hid_hw_start() or hid_hw_open() fails during the sync interface probe,
the driver explicitly frees the sd struct via kref_put(). However, it leaves
a dangling pointer because hid_set_drvdata(hdev, sd) was already called
earlier and is not reverted here.

If the async interface is probed concurrently (e.g., via sysfs bind),
it calls steelseries_get_sibling_hdev() to find the sync interface's HID
device and reads its driver data. It might receive this dangling pointer
and call kref_get() on freed memory, resulting in a use-after-free.

>  	return ret;
>  }

[Severity: High]
Is there a race condition when looking up the sibling interface during probe
that could lead to a use-after-free of the hid_device?

When probing the async interface, steelseries_get_sibling_hdev() retrieves
the sibling interface's hid_device pointer locklessly:

drivers/hid/hid-steelseries-arctis.c:steelseries_get_sibling_hdev() {
    ...
    sibling_hdev = usb_get_intfdata(sibling_intf);
    return sibling_hdev;
}

Then in steelseries_arctis_probe():

    master_hdev = steelseries_get_sibling_hdev(hdev, info->sync_interface);
    if (!master_hdev || !hid_get_drvdata(master_hdev))
        return -EPROBE_DEFER;

If the sync interface is unbound concurrently (e.g., via sysfs unbind),
hid_destroy_device() can free the hid_device. If the async probe reads
the intfdata pointer just before it is cleared and dereferences it via
hid_get_drvdata(master_hdev), it accesses freed memory. The HID device
probe path does not seem to hold the parent USB device lock to prevent
this concurrent execution.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623172310.272708-1-srimanachanta@gmail.com?part=8