Re: [PATCH] HID: steam: reject short serial number reports

[Yousef Alhouseen]

Hi Vicki,

That broader fix sounds preferable. Please go ahead and split it out
and submit it; I will drop this version to avoid overlapping work.

Thanks,
Yousef

On Sat, 27 Jun 2026 17:47:40 -0700, Vicki Pfau <vi@xxxxxxxxxxx> wrote:
> Hi Yousef,
>
> On 6/27/26 5:41 PM, Yousef Alhouseen wrote:
> > steam_recv_report() may return a short positive response and copies
> > only the bytes actually received. steam_get_serial() nevertheless reads
> > the full three-byte header and trusts its length without checking that
> > the serial payload was returned.
> >
> > A malformed USB device can therefore make the driver read uninitialized
> > stack bytes. With a complete-looking short header, those bytes can also
> > be copied into steam->serial_no and printed.
> >
> > Account for the stripped report ID in the return value and reject replies
> > that do not contain both the header and its declared payload.
> >
> > Fixes: c164d6abf384 ("HID: add driver for Valve Steam Controller")
> > Reported-by: syzbot+75f3f9bff8c510602d36@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://syzkaller.appspot.com/bug?extid=75f3f9bff8c510602d36
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>
> > ---
> > drivers/hid/hid-steam.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> > index 197126d6e081..8c8bfb10e8b8 100644
> > --- a/drivers/hid/hid-steam.c
> > +++ b/drivers/hid/hid-steam.c
> > @@ -454,11 +454,20 @@ static int steam_get_serial(struct steam_device *steam)
> > ret = steam_recv_report(steam, reply, sizeof(reply));
> > if (ret < 0)
> > goto out;
> > + /* hid_hw_raw_request() counts the stripped report ID byte. */
> > + if (ret < 4) {
> > + ret = -EIO;
> > + goto out;
> > + }
> > if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
> > reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
> > ret = -EIO;
> > goto out;
> > }
> > + if (ret - 1 < 3 + reply[1]) {
> > + ret = -EIO;
> > + goto out;
> > + }
> > reply[3 + STEAM_SERIAL_LEN] = 0;
> > strscpy(steam->serial_no, reply + 3, reply[1]);
> > out:
>
> I already have locally a patch that fixes this as part of my pending Steam Controller 2 support. However, it chooses to fix it in a different way that would affect all uses of steam_recv_report instead of per-callsite (with only one callsite fixed). I am hoping to get this patchset submitted soon, once more widescale testing is done, but if you want in the meantime I can pull out that single fix and submit it separately; it's a bit more sprawling and involves adding a new function for combined send/recv.
>
> Vicki