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

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 [off-list 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.

[ ... ]

[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.

[ ... ]

[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,

[ ... ]

[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.

[ ... ]

[ ... ]

+		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