Thread (17 messages) 17 messages, 2 authors, 14d ago

Re: [PATCH v2 09/10] HID: steam: Reject short reads

From: sashiko-bot@kernel.org
Date: 2026-07-07 23:47:05

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Out-of-bounds stack memory read in `steam_recv_report` when logging debug information.
- [Medium] Uninitialized kernel stack memory read on the error path for short reports.
--
--- Patch [9]: [PATCH v2 09/10] HID: steam: Reject short reads ---
commit ea3743d28300ad972f70af0939882012d7dbf763
Author: Vicki Pfau [off-list ref]

HID: steam: Reject short reads

Steam Controller FEATURE reports encode the size of the message in the
message itself. Previously we were trusting that the size reported matched
the size we actually read, leading to a potential issue with short reads.
Instead, we should actually verify the length of the read.

Fixes: c164d6abf384 ("HID: add driver for Valve Steam Controller")
Reported-by: syzbot+75f3f9bff8c510602d36@syzkaller.appspotmail.com
Closes: 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 1f2cf15d40831..49f6733d89359 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -380,8 +387,10 @@ 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);
 
@@ -389,7 +398,15 @@ static int steam_recv_report(struct steam_device *steam,
 		hid_err(steam->hdev, "%s: error %d\n", __func__, ret);
 	else
 		hid_dbg(steam->hdev, "Received report %*ph\n", ret, data);
[Severity: Medium]
Since the clamping of ret to size was removed above, ret can now be larger
than the caller-provided buffer size.

If a caller provides a small stack buffer (e.g. 25 bytes in
steam_get_serial()) and the device returns a larger report, will hid_dbg()
read out-of-bounds stack memory from the data buffer when dynamic debug is
enabled?
-	return ret;
+	if (ret < 0)
+		return ret;
+
+	if (ret < 2 || ret < data[1] + 2) {
+		hid_err(steam->hdev, "%s: expected %u bytes, read %i\n",
+				__func__, data[1] + 2, ret);
[Severity: Medium]
If the device returns a short read of 0 or 1 bytes, memcpy() copies less
than 2 bytes to data, leaving data[1] uninitialized.

Since hid_err() arguments are unconditionally evaluated before the function
is called, could this read uninitialized stack memory from data[1] if
ret < 2 evaluates to true?
+		return -EPROTO;
+	}
+	return size;
 }
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707233241.2644971-1-vi@endrift.com?part=9
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help