Re: [PATCH v5 4/6] HID: steam: Don't set feature reports when disconnecting

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-after-free in steam_remove() due to incorrect cancellation order of deferred work.
--

--- Patch [4]: [PATCH v5 4/6] HID: steam: Don't set feature reports when disconnecting ---
commit 460f363e27307cbdb952f6004dd3826de0898a07
Author: Vicki Pfau <vi@endrift.com>

HID: steam: Don't set feature reports when disconnecting

When an input device is closed, we set a feature report to reset lizard
mode and IMU mode. However, if the input device is closed because it was
removed, then we will necessarily error out when sending this, resulting in
logged errors. Since an error here is expected, we should just fail
silently.

> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 461ebf37171b7..7226c0681ed18 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
> @@ -490,9 +490,13 @@ static int steam_recv_report_id(struct steam_device *steam,
>  	}
>  
>  	kfree(buf);
> -	if (ret < 0)
> +	/*
> +	 * Don't log if the failure is -ENODEV, as this
> +	 * can happen normally on disconnect.
> +	 */
> +	if (ret < 0 && ret != -ENODEV)
>  		hid_err(steam->hdev, "%s: error %d\n", __func__, ret);

[Severity: High]
This is a pre-existing issue, but while reviewing device disconnects, I
noticed a potential use-after-free during device removal in steam_remove().

Looking at steam_remove(), cancel_work_sync(&steam->work_connect) is called
before hid_hw_stop(hdev):

drivers/hid/hid-steam.c:steam_remove() {
	...
	cancel_work_sync(&steam->work_connect);
	if (steam->quirks & STEAM_QUIRK_WIRELESS) {
		hid_info(hdev, "Steam wireless receiver disconnected");
	}
	steam_unregister(steam);
	hid_hw_stop(hdev);
}

Because hid_hw_stop() is called last, hardware interrupts can still trigger
steam_raw_event() after the work item is cancelled. If a wireless connection
event (ID_CONTROLLER_WIRELESS) or an unexpected input report arrives during
this window, steam_raw_event() will invoke steam_do_connect_event(), which
re-schedules steam->work_connect:

drivers/hid/hid-steam.c:steam_raw_event() {
	...
	case ID_CONTROLLER_WIRELESS:
		...
		switch (data[4]) {
		...
		case 0x02:
			steam_do_connect_event(steam, true);
			break;
		...
}

After steam_remove() returns and the device memory is freed by devres, the
workqueue retains the pending work, which could access the freed memory when
it runs.

Should hid_hw_stop(hdev) be called before cancel_work_sync() to ensure no new
events can queue the work?

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