[PATCH] usb: uas: validate status URB length before parsing the IU

[PATCH] usb: uas: validate status URB length before parsing the IU

[Olivier Kaloudoff]

uas_stat_cmplt() dereferences iu->tag without verifying that the
status URB actually carried a full IU header.  The transfer buffer is
a freshly-zeroed sense_iu, so a status URB that completes
successfully but with a short or empty actual_length — observed in
the wild during link-level upsets (Thunderbolt-chain xhci blips) and
on a wedging bridge — reads back as iu_id=0/tag=0 and produces the
misleading

  stat urb: no pending cmd for uas-tag 0

log spam seen e.g. on an ASMT135x enclosure and a Samsung T7, right
before the affected commands time out into the error handler.

Beyond the noise, a short packet that happens to carry a
plausible-looking header could be matched to an inflight command and
drive the state machine off garbage.  Bail out on anything shorter
than a full struct iu.

Cc: stable@vger.kernel.org
Signed-off-by: Olivier Kaloudoff <olivier.kaloudoff@gmail.com>
---
 drivers/usb/storage/uas.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 265162981269..88fed50c262b 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -315,6 +315,13 @@ static void uas_stat_cmplt(struct urb *urb)
 		goto bail;
 	}
 
+	if (urb->actual_length < sizeof(struct iu)) {
+		dev_err(&urb->dev->dev,
+			"stat urb: short transfer (%u bytes)\n",
+			urb->actual_length);
+		goto bail;
+	}
+
 	idx = be16_to_cpup(&iu->tag) - 1;
 
 	spin_lock_irqsave(&devinfo->lock, flags);
-- 
2.47.3

Re: [PATCH] usb: uas: validate status URB length before parsing the IU

[sashiko-bot]

> uas_stat_cmplt() dereferences iu->tag without verifying that the
> status URB actually carried a full IU header.  The transfer buffer is
> a freshly-zeroed sense_iu, so a status URB that completes
> successfully but with a short or empty actual_length — observed in
> the wild during link-level upsets (Thunderbolt-chain xhci blips) and
> on a wedging bridge — reads back as iu_id=0/tag=0 and produces the
> misleading
> 
>   stat urb: no pending cmd for uas-tag 0
> 
> log spam seen e.g. on an ASMT135x enclosure and a Samsung T7, right
> before the affected commands time out into the error handler.
> 
> Beyond the noise, a short packet that happens to carry a
> plausible-looking header could be matched to an inflight command and
> drive the state machine off garbage.  Bail out on anything shorter
> than a full struct iu.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Olivier Kaloudoff <olivier.kaloudoff@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013221937.41490-1-olivier.kaloudoff@gmail.com?part=1