# Re: \[PATCH 1/5\] HID: steam: Initial 2026 Steam Controller support

- From: sashiko-bot@kernel.org
- Date: 2026-08-05 02:13:04 
- List: linux-input
- Page: https://ratatoskr.run/linux-input/2026/08/17366347
- Thread: https://ratatoskr.run/linux-input/2026/08/17366290/t.md

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

New issues:
- [High] Battery registration fails due to thermal zone name length limits if the device serial number is long.
- [Medium] Incorrect initialization of battery temperature results in reporting 0.2 °C.
- [Low] Duplicate error logging block introduced in steam_recv_report_id.

Pre-existing issues:
- [High] Probe failure leaves a registered power supply with dangling devm pointers.
- [High] Unbinding the driver while the device is active can lead to a Use-After-Free in steam_work_connect_cb.
--

--- Patch [1]: [PATCH 1/5] HID: steam: Initial 2026 Steam Controller support ---
commit 10f5a4093529f8973ab3200723fc61d0c419c6bb
Author: Vicki Pfau [ref]

HID: steam: Initial 2026 Steam Controller support

This brings support for the 2026 Steam Controller, comparably featureful to
the existing support for the Steam Deck.

> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 6199f67f3c4cd..111e57926d049 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c

[ ... ]

> @@ -378,21 +459,38 @@ static int steam_recv_report(struct steam_device *steam,
>  		return -ENOMEM;
>  
>  	/*
> -	 * The report ID is always 0, so strip the first byte from the output.
> +	 * The report ID is consistent, so strip the first byte from the output.
>  	 * hid_report_len() is not counting the report ID, so +1 to the length
>  	 * or else we get a EOVERFLOW. We are safe from a buffer overflow
>  	 * because hid_alloc_report_buf() allocates +7 bytes.
>  	 */
> -	ret = hid_hw_raw_request(steam->hdev, 0x00,
> -			buf, hid_report_len(r) + 1,
> -			HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> +	if (!(steam->quirks & STEAM_QUIRK_IBEX))
> +		len += 1;
> +
> +	/*
> +	 * Sometimes the wireless controller fails with EPIPE
> +	 * when sending a feature report.
> +	 * Doing a HID_REQ_GET_REPORT and waiting for a while
> +	 * seems to fix that.
> +	 */
> +	do {
> +		ret = hid_hw_raw_request(steam->hdev, report_id,
> +				buf, len,
> +				HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> +		if (ret != -EPIPE)
> +			break;
> +		msleep(20);
> +	} while (--retries);
>  	if (ret > 0) {
>  		/* Remove the report ID from the return buffer */
>  		ret--;
>  		size = min(size, ret);
>  		memcpy(data, buf + 1, size);
>  	}
> +
>  	kfree(buf);
> +	if (ret < 0)
> +		hid_err(steam->hdev, "%s: error %d\n", __func__, ret);
>  
>  	if (ret < 0)
>  		hid_err(steam->hdev, "%s: error %d\n", __func__, ret);

[Severity: Low]
Is this error logging block accidentally duplicated? There are two identical
checks and hid_err() calls sequentially in steam_recv_report_id(), which will
cause the exact same error message to be printed twice upon a single
hid_hw_raw_request() failure.

[ ... ]

> @@ -854,19 +1051,35 @@ static int steam_battery_register(struct steam_device *steam)
>  	int ret;
>  
>  	steam->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> -	steam->battery_desc.properties = steam_battery_props;
> -	steam->battery_desc.num_properties = ARRAY_SIZE(steam_battery_props);
> +	if (steam->quirks & STEAM_QUIRK_IBEX) {
> +		steam->battery_desc.properties = steam_ibex_battery_props;
> +		steam->battery_desc.num_properties = ARRAY_SIZE(steam_ibex_battery_props);
> +		/*
> +		 * Ibex needs a shorter name as it has a temperature and the
> +		 * thermal zone name length limit is 20 characters. It's more
> +		 * ambiguous sounding, so let's only use it when needed.
> +		 */
> +		steam->battery_desc.name = devm_kasprintf(&steam->hdev->dev,
> +				GFP_KERNEL, "steam-%s",
> +				steam->serial_no);

[Severity: High]
Will this name exceed the 20-character maximum permitted for a thermal zone
device type if the serial number is long?

Because STEAM_SERIAL_LEN is 21, a serial number of 14 characters or
longer combined with the "steam-" prefix will exceed 20 characters. This
causes thermal_zone_device_register_with_trips() to return -EINVAL,
resulting in power_supply_register() and steam_battery_register() failing
entirely.

> +	} else {
> +		steam->battery_desc.properties = steam_battery_props;
> +		steam->battery_desc.num_properties = ARRAY_SIZE(steam_battery_props);
> +		steam->battery_desc.name = devm_kasprintf(&steam->hdev->dev,
> +				GFP_KERNEL, "steam-controller-%s-battery",
> +				steam->serial_no);
> +	}
>  	steam->battery_desc.get_property = steam_battery_get_property;
> -	steam->battery_desc.name = devm_kasprintf(&steam->hdev->dev,
> -			GFP_KERNEL, "steam-controller-%s-battery",
> -			steam->serial_no);
>  	if (!steam->battery_desc.name)
>  		return -ENOMEM;
>  
>  	/* avoid the warning of 0% battery while waiting for the first info */
>  	spin_lock_irqsave(&steam->lock, flags);
> -	steam->voltage = 3000;
> +	steam->battery_voltage = 3000;
>  	steam->battery_charge = 100;
> +	steam->battery_current = 0;
> +	steam->battery_temp = 200;

[Severity: Medium]
Does this initialize the default temperature to 0.2 degrees Celsius?

In steam_battery_get_property(), this value is divided by 100:
val->intval = temp / 100;

Since the power supply subsystem expects POWER_SUPPLY_PROP_TEMP in tenths
of a degree Celsius, returning 2 represents 0.2 C. If the intended default
temperature was 20.0 C, should this be initialized to 20000?

> +	steam->battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
>  	spin_unlock_irqrestore(&steam->lock, flags);
>  
>  	battery = power_supply_register(&steam->hdev->dev,

[ ... ]

> @@ -1149,51 +1380,42 @@ static int steam_register(struct steam_device *steam)
> -	spin_lock_irqsave(&steam->lock, flags);
> -	client_opened = steam->client_opened;
> -	spin_unlock_irqrestore(&steam->lock, flags);
> +	/* ignore battery errors, we can live without it */
> +	if (steam->quirks & (STEAM_QUIRK_WIRELESS | STEAM_QUIRK_IBEX))
> +		steam_battery_register(steam);
>  
> -	if (!client_opened) {
> -		steam_set_lizard_mode(steam, lizard_mode);
> -		ret = steam_input_register(steam);
> -		if (ret != 0)
> -			goto steam_register_input_fail;
> -		ret = steam_sensors_register(steam);
> -		if (ret != 0)
> -			goto steam_register_sensors_fail;
> -	}
> +	steam_set_lizard_mode(steam, lizard_mode);
> +	ret = steam_input_register(steam);
> +	if (ret != 0)
> +		goto steam_register_input_fail;
> +	ret = steam_sensors_register(steam);
> +	if (ret != 0)
> +		goto steam_register_sensors_fail;

[Severity: High]
This is a pre-existing issue, but does this error path leave the battery
registered if steam_input_register() or steam_sensors_register() fails?

If steam_register() fails, steam_probe() jumps to the err_hw_close label,
which bypasses steam_unregister(). Because steam_probe() returns an error,
devres will free the steam_device structure.

Since the battery was not unregistered, it remains active in sysfs, and any
subsequent userspace read of the battery's properties will dereference the
freed steam_device pointer, causing a kernel use-after-free.

[ ... ]

> @@ -1989,8 +2462,84 @@ static int steam_raw_event(struct hid_device *hdev,
>  		return 0;
>  
>  	if (steam->client_opened)
> -		hid_input_report(steam->client_hdev, HID_FEATURE_REPORT,
> -				data, size, 0);
> +		hid_input_report(steam->client_hdev, report->type, data, size, 0);
> +
> +	/* Ibex uses a different report format */
> +	if (steam->quirks & STEAM_QUIRK_IBEX) {
> +		if (report->type != HID_INPUT_REPORT)
> +			return 0;
> +
> +		switch (report->id) {
> +		case REPORT_ID_INPUT:
> +			if (size != 54)

[ ... ]

> +		case REPORT_ID_WIRELESS_EVENT:
> +			if (size != 2)
> +				return 0;
> +			switch (data[1]) {
> +			case WIRELESS_EVENT_DISCONNECT:
> +				steam_do_connect_event(steam, false);
> +				break;
> +			case WIRELESS_EVENT_CONNECT:
> +				steam_do_connect_event(steam, true);
> +				break;
> +			}
> +			break;

[Severity: High]
This is a pre-existing issue, but can this lead to a use-after-free during
driver unbind?

When the driver is unbound, steam_remove() cancels the steam->work_connect
work item and calls steam_unregister(), but hid_hw_stop() is not called
until the very end.

In this window, the device can still stream data and trigger
steam_raw_event(), which can call steam_do_connect_event(). This
reschedules steam->work_connect back onto the workqueue. Once
steam_remove() returns, devres frees the steam_device structure, causing
the scheduled work item to execute steam_work_connect_cb() on freed memory.

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