[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

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

[Greg KH]

On Wed, Oct 14, 2026 at 12:19:37AM +0200, Olivier Kaloudoff wrote:
> 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);

Do not allow broken / malicious devices to spam the kernel log.

And how was this tested?

thanks,

greg k-h

[PATCH v2] 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>
---
v2: use dev_err_ratelimited() so a device emitting a stream of
    short status URBs cannot spam the kernel log (per Greg KH).

    Tested: patched uas module reloaded on a live system carrying an
    ASMT135x enclosure (behind an Alpine Ridge TB dock) and a Samsung
    PSSD T7 (AMD onboard xhci, HS link); verified clean re-probe,
    mount, and sustained I/O, plus 16 forced disconnect/re-enumerate
    cycles under write load with no regressions and no spurious
    short-transfer reports.

 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..d1725ef8f954 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_ratelimited(&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 v2] usb: uas: validate status URB length before parsing the IU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb/uas: short STATUS IUs (4-15 bytes) falsely complete as SUCCESS
--

commit 1a208f3daab26d0bf253bfe97e68d1636d90a96c
Author: Olivier Kaloudoff <olivier.kaloudoff@gmail.com>

usb: uas: validate status URB length before parsing the IU

The commit attempts to fix log spam caused by parsing a short or empty status
URB transfer. It does so by bailing out if actual_length is smaller than
sizeof(struct iu), which is only 4 bytes long.

> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 8655edbd66b16..b5d3071d86c84 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)) {

[Severity: High]
Does this allow 4-15 byte packets to proceed to type-specific processing?

Because sizeof(struct iu) is only 4 bytes, a packet of 4-15 bytes with
iu_id == IU_ID_STATUS (3) passes this check.

If that occurs, uas_stat_cmplt() delegates to uas_sense():

drivers/usb/storage/uas.c:uas_sense() {
...
	if (urb->actual_length > 16) {
...
	}

	cmnd->result = sense_iu->status;
}

Since the transfer buffer is zero-allocated and not completely overwritten by
the short transfer, sense_iu->status at offset 6 reads as 0 (SAM_STAT_GOOD).

Does this result in the command falsely completing as SUCCESS and hiding
device errors from the SCSI block layer?

> +		dev_err_ratelimited(&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);

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