Re: [PATCH v7 3/8] HID: steelseries: Refactor Arctis driver to use device_info framework

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:
- [High] Race condition exposing ERR_PTR leading to kernel panic in raw_eve=
nt.
--

--- Patch [3]: [PATCH v7 3/8] HID: steelseries: Refactor Arctis driver to u=
se device_info framework ---
commit e1b5fdcaec23ba5cc7a2a2139ba1c9fa983d7894
Author: Sriman Achanta <[email protected]>

HID: steelseries: Refactor Arctis driver to use device_info framework

This commit refactors the Arctis driver to use a device_info struct
instead of a quirk bitmap for device capabilities. It also changes
battery polling to use a periodic delayed work instead of being
requested from raw_event.

> diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steel=
series-arctis.c
> index 6cae2e00bd8ba..c54c56db9ddd7 100644
> --- a/drivers/hid/hid-steelseries-arctis.c
> +++ b/drivers/hid/hid-steelseries-arctis.c
[ ... ]
> @@ -177,25 +271,27 @@ static int steelseries_headset_battery_register(str=
uct steelseries_device *sd)
[ ... ]
>  	sd->battery =3D devm_power_supply_register(&sd->hdev->dev,
>  			&sd->battery_desc, &battery_cfg);
>  	if (IS_ERR(sd->battery)) {
>  		ret =3D PTR_ERR(sd->battery);
> +		sd->battery =3D NULL;
>  		hid_err(sd->hdev,
>  				"%s:power_supply_register failed with error %d\n",
>  				__func__, ret);

[Severity: High]
Could this assignment expose an ERR_PTR to a concurrent event handler?

Because hid_hw_start() and hid_hw_open() are called before=20
steelseries_battery_register() in steelseries_arctis_probe(), USB interrupt=
s=20
can trigger steelseries_arctis_raw_event() concurrently.

If devm_power_supply_register() fails due to memory pressure or other error=
s,=20
sd->battery will briefly hold an ERR_PTR before it is set to NULL. A=20
concurrent execution of raw_event() evaluates if (sd->battery). Since an=20
ERR_PTR is not NULL, the check passes, and power_supply_changed(sd->battery=
)=20
will dereference the invalid pointer, causing a kernel panic.

Would it be safer to assign the return value to a local variable first, and=
=20
only assign it to sd->battery upon success?

Additionally, does the unconditional assignment of sd->battery_capacity =3D=
 100=20
just before registration risk silently overwriting a real status update=20
received by raw_event() during this window?

[ ... ]
> @@ -272,142 +365,92 @@ static void steelseries_arctis_remove(struct hid_d=
evice *hdev)
[ ... ]
> -	if (connected !=3D sd->headset_connected) {
> -		hid_dbg(sd->hdev,
> +	if (sd->headset_connected !=3D old_connected) {
> +		hid_dbg(hdev,
>  			"Connected status changed from %sconnected to %sconnected\n",
> -			sd->headset_connected ? "" : "not ",
> -			connected ? "" : "not ");
> -		sd->headset_connected =3D connected;
> -		steelseries_headset_set_wireless_status(hdev, connected);
> +			old_connected ? "" : "not ",
> +			sd->headset_connected ? "" : "not ");
> +
> +		if (sd->battery) {
> +			steelseries_headset_set_wireless_status(sd->hdev,
> +							       sd->headset_connected);
> +			power_supply_changed(sd->battery);
> +		}
>  	}

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803191553.6636=
[email protected]?part=3D3