Re: [PATCH v3 09/10] HID: steam: Reject short reads
From: sashiko-bot@kernel.org
Date: 2026-07-10 02:27:42
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: New issues: - [Medium] If `hid_hw_raw_request` returns 0, the function prints uninitialized caller stack memory to the kernel debug log. Pre-existing issues: - [High] The driver enables hardware events via `hid_hw_start()` before initializing locks and works, leading to TOCTOU and uninitialized lock usage. - [Medium] The newly added length validation `ret < data[1] + 2` is insufficient for `steam_get_serial`, leaving an uninitialized stack byte read via `strscpy`. --
--- Patch [9]: [PATCH v3 09/10] HID: steam: Reject short reads ---commit 9194c57f44b65844dcf5d6df8e1faa2846607590 Author: Vicki Pfau [off-list ref] Subject: HID: steam: Reject short reads This commit adds length validation to steam_recv_report() to ensure that the reported size of a Steam Controller message matches the bytes actually read. It also addresses stripping the report ID from the buffer more explicitly. Link: https://syzkaller.appspot.com/bug?extid=75f3f9bff8c510602d36
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c index 298a2c17e1123..a80d54ed9784b 100644 --- a/drivers/hid/hid-steam.c +++ b/drivers/hid/hid-steam.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -380,16 +387,30 @@ static int steam_recv_report(struct steam_device *steam, buf, hid_report_len(r) + 1, HID_FEATURE_REPORT, HID_REQ_GET_REPORT); if (ret > 0) { - ret = min(size, ret - 1); - memcpy(data, buf + 1, ret); + /* 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); else - hid_dbg(steam->hdev, "Received report %*ph\n", ret, data); - return ret; + hid_dbg(steam->hdev, "Received report %*ph\n", size, data);
[Severity: Medium] If hid_hw_raw_request() returns 0, the if (ret > 0) block is bypassed, leaving size unmodified. Will this cause hid_dbg() to print uninitialized stack memory from the data array?
+ if (ret < 0)
+ return ret;
+
+ if (ret < 2) {
+ hid_err(steam->hdev, "%s: reply too short\n", __func__);
+ return -EPROTO;
+ }
+ if (ret < data[1] + 2) {
+ hid_err(steam->hdev, "%s: expected %u bytes, read %i\n",
+ __func__, data[1] + 2, ret);
+ return -EPROTO;
+ }[Severity: Medium] This is a pre-existing issue, but is this length validation sufficient for steam_get_serial()? If a device sends a response where the returned length exactly equals data[1] + 2, steam_recv_report() accepts the packet. However, steam_get_serial() calls strscpy(..., reply + 3, reply[1]). This instructs strscpy() to read up to reply[1] bytes starting from index 3, which accesses indices 3 through 3 + reply[1] - 1 (which is reply[1] + 2). Since only indices 0 through data[1] + 1 were validated and initialized, does this result in an uninitialized stack byte read?
+ return size; }
[Severity: High] This is a pre-existing issue, but does steam_probe() enable hardware events before initializing locks and workqueues? In drivers/hid/hid-steam.c:steam_probe(): ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW); if (ret) return ret; steam->hdev = hdev; hid_set_drvdata(hdev, steam); spin_lock_init(&steam->lock); mutex_init(&steam->report_mutex); steam->quirks = id->driver_data; INIT_WORK(&steam->work_connect, steam_work_connect_cb); If a connection event arrives immediately from the hardware after hid_set_drvdata() but before spin_lock_init() or INIT_WORK(), could steam_raw_event() attempt to acquire an uninitialized spinlock or schedule an uninitialized work struct? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260710020048.2337655-1-vi@endrift.com?part=9