Re: [PATCH v7 6/8] HID: steelseries: Manage battery lifetime with refcounting
From: sashiko-bot@kernel.org
Date: 2026-08-03 19:29:53
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Concurrent execution of `steelseries_arctis_raw_event()` and explicit battery unregistration in `steelseries_arctis_remove()` introduces a Use-After-Free and NULL pointer dereference. - [Medium] The power supply name string is allocated with `devm_kasprintf` but is embedded inside a manually reference-counted object `sd`, leading to a lifecycle mismatch. --
--- Patch [6]: [PATCH v7 6/8] HID: steelseries: Manage battery lifetime with refcounting ---commit fd37e70cee8d0953a9d3de05f0149d2fc76d513a Author: Sriman Achanta [off-list ref] HID: steelseries: Manage battery lifetime with refcounting This commit changes the steelseries_device structure to use a kref for lifetime management instead of devm. It explicitly registers and unregisters the battery power supply and nullifies the battery pointer during removal.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c index b54f20f081a26..e534aa44e70a0 100644 --- a/drivers/hid/hid-steelseries-arctis.c +++ b/drivers/hid/hid-steelseries-arctis.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -277,9 +288,10 @@ static int steelseries_battery_register(struct steelseries_device *sd) sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_battery_props); sd->battery_desc.get_property = steelseries_battery_get_property; sd->battery_desc.use_for_apm = 0; - n = atomic_inc_return(&battery_no) - 1; sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL, - "steelseries_headset_battery_%ld", n); + "steelseries_headset_battery_%s", + sd->hdev->uniq[0] ? sd->hdev->uniq : + dev_name(&sd->hdev->dev));
[Severity: Medium] Since the steelseries_device struct sd is now refcounted and can outlive the device, will using devm_kasprintf() for sd->battery_desc.name leave a dangling pointer inside sd when the device unbinds and devres frees the string? [ ... ]
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 an incoming HID event trigger steelseries_arctis_raw_event() concurrently with this teardown, leading to a NULL pointer dereference or use-after-free? Since hid_hw_stop() hasn't been called yet, hardware events can still fire. If steelseries_arctis_raw_event() locklessly checks sd->battery and then calls power_supply_changed(sd->battery), could remove() nullify and free the battery in between the check and the use? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260803191553.66368-1-srimanachanta@gmail.com?part=6