Re: [PATCH v6 7/9] HID: steelseries: Manage battery lifetime with refcounting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lockless access to `sd->battery` in `steelseries_arctis_raw_event()` creates a TOCTOU race with `remove()`, potentially causing a Use-After-Free.
--

--- Patch [7]: [PATCH v6 7/9] HID: steelseries: Manage battery lifetime with refcounting ---
commit acac88b3e3a741b01e199d1d54e37c2b85ec3b35
Author: Sriman Achanta <srimanachanta@gmail.com>

HID: steelseries: Manage battery lifetime with refcounting

This patch converts the steelseries_device structure to use a kref so its state
can outlive a single HID interface. It also registers and unregisters the power
supply explicitly instead of using devm, and clears sd->battery under a
spinlock in the remove function.

> 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

[ ... ]

> @@ -387,13 +405,20 @@ static void steelseries_arctis_remove(struct hid_device *hdev)
>  	if (interface_num == sd->info->sync_interface) {
>  		spin_lock_irqsave(&sd->lock, flags);
>  		sd->removed = true;
> +		battery = sd->battery;
> +		sd->battery = NULL;
>  		spin_unlock_irqrestore(&sd->lock, flags);
>  
>  		cancel_delayed_work_sync(&sd->status_work);
> +
> +		if (battery)
> +			power_supply_unregister(battery);
>  	}
>  
>  	hid_hw_close(hdev);
>  	hid_hw_stop(hdev);

[Severity: High]
Can this lead to a use-after-free if a battery status report arrives just as
the device is being removed?

In steelseries_arctis_raw_event(), sd->battery is accessed locklessly:

    if (sd->battery) {
        steelseries_headset_set_wireless_status(sd->hdev,
                               sd->headset_connected);
        power_supply_changed(sd->battery);
    }

If a raw event checks sd->battery and then gets preempted, this remove() path
can clear sd->battery and unregister the power supply because hid_hw_stop()
hasn't been called yet to halt the delivery of USB URBs.

When raw_event() resumes, wouldn't it call power_supply_changed() on the newly
freed battery pointer?

> +
> +	kref_put(&sd->refcnt, steelseries_device_release);
>  }

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