Thread (16 messages) 16 messages, 3 authors, 19h ago

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

From: sashiko-bot@kernel.org
Date: 2026-07-09 03:38:43

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 [off-list ref]

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.
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
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help