Re: [PATCH v2 04/10] HID: steam: Add support for sensor events on the Steam Controller (2015)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] If `steam_register()` fails during probe for a wired device, the `steam` struct is left on the global `steam_devices` list but its memory is freed, leading to a Use-After-Free.
--

--- Patch [4]: [PATCH v2 04/10] HID: steam: Add support for sensor events on the Steam Controller (2015) ---
commit d6e009f462fb9aec26d7157d6389455fb438d8e1
Author: Vicki Pfau <vi@endrift.com>

HID: steam: Add support for sensor events on the Steam Controller (2015)

Sensor support was added for the Steam Deck previously, but Steam Controller
sensor events were never added. This adds that missing support, bringing Steam
Controller support much closer to feature parity with things like SDL and Steam
itself.

> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index add9f4f27fff9..6029c055a6cf3 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c

[ ... ]

> @@ -968,6 +1077,12 @@ static int steam_register(struct steam_device *steam)
>  			strscpy(steam->serial_no, "XXXXXXXXXX",
>  					sizeof(steam->serial_no));
>  
> +		ret = steam_get_attributes(steam);
> +		if (ret < 0)
> +			hid_err(steam->hdev,
> +				"%s:steam_get_attributes failed with error %d\n",
> +				__func__, ret);
> +
>  		hid_info(steam->hdev, "Steam Controller '%s' connected",
>  				steam->serial_no);
>  

[Severity: High]
This is a pre-existing issue, but can the error path in steam_probe() lead to
a use-after-free if steam_register() fails?

In steam_register(), the device is added to the global steam_devices list:

		mutex_lock(&steam_devices_lock);
		if (list_empty(&steam->list))
			list_add(&steam->list, &steam_devices);
		mutex_unlock(&steam_devices_lock);

If a subsequent registration step (like steam_input_register() or
steam_sensors_register()) fails, steam_register() returns an error without
removing the device from the list.

Then, back in steam_probe(), if steam_register() fails, it jumps to
err_hw_close, bypassing the unregister cleanup:

err_steam_unregister:
	if (steam->connected)
		steam_unregister(steam);
err_hw_close:
	hid_hw_close(hdev);

Because the steam device was allocated with devm_kzalloc(), devres will
automatically free the structure upon probe failure. Does this leave a dangling
pointer in the global steam_devices list, which could cause memory corruption
when the list is later iterated (e.g., when setting the lizard_mode module
parameter)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707233241.2644971-1-vi@endrift.com?part=4