Re: [PATCH v4 09/11] HID: steam: Improve logging and other cleanup

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

Pre-existing issues:
- [High] The error path in `steam_register()` fails to remove the device from the global `steam_devices` list, leading to a Use-After-Free when probe fails.
--

--- Patch [9]: [PATCH v4 09/11] HID: steam: Improve logging and other cleanup ---
commit e2534775e852c76ea14b6b8605e36139ce7654b1
Author: Vicki Pfau <[email protected]>

    HID: steam: Improve logging and other cleanup
    
    Adds more logging as appropriate, reindents an enum to match surrounding
    style, as well as cleaning up some places where we can use guard() instead
    of doing locking and unlocking manually.

> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 5ab4396a241e0..3738a4de3bffb 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c

[ ... ]

> @@ -1838,6 +1847,7 @@ static void steam_do_deck_input_event(struct steam_device *steam,
>  		steam->did_mode_switch = false;
>  		cancel_delayed_work(&steam->mode_switch);
>  	} else if (!steam->client_opened && start_pressed && !steam->did_mode_switch) {
> +		hid_dbg(steam->hdev, "%s: doing mode switch\n", __func__);
>  		steam->did_mode_switch = true;
>  		schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100);
>  	}

[Severity: High]
This is a pre-existing issue, but the error path in steam_register() fails to
remove the device from the global steam_devices list, which could lead to a
use-after-free when probe fails.

If steam_register() fails during steam_probe() for a wired device (e.g., if
steam_sensors_register() returns an error), the steam object is added to the
global steam_devices list but is not removed on the error path:

drivers/hid/hid-steam.c:steam_register() {
    ...
steam_register_sensors_fail:
	steam_input_unregister(steam);
steam_register_input_fail:
	return ret;
}

steam_probe() then returns the error, jumping to the err_hw_stop label and
bypassing steam_unregister(), which would have removed it from the list:

drivers/hid/hid-steam.c:steam_probe() {
    ...
		ret = steam_register(steam);
		if (ret) {
			hid_err(hdev, "%s:steam_register failed with error %d\n", __func__, ret);
			goto err_hw_stop;
		}
    ...
}

Because the steam object was allocated with devm_kzalloc(), the kernel frees
it upon probe failure, leaving a dangling pointer in the steam_devices list.

Can this memory corruption be triggered by plugging in a second Steam
Controller, or by writing to the lizard_mode sysfs parameter?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9