[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

[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

[PATCH v3] 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.

Worse, this is not just log noise: a short packet that happens to
carry a plausible header is matched to an inflight command, and the
untouched bytes of the zeroed buffer then stand in for fields that
were never transferred — sense_iu.status reads as SAM_STAT_GOOD,
falsely completing the command, and response_iu.response_code reads
as RC_TMF_COMPLETE.

Enforce the IU header minimum before parsing the tag, then enforce
per-IU minimums before dereferencing: STATUS IUs need at least the
status byte, RESPONSE IUs need response_code; READ_READY and
WRITE_READY IUs legitimately carry only the 4-byte header.

Signed-off-by: Olivier Kaloudoff <olivier.kaloudoff@gmail.com>
---
v3: address Sashiko review — the bare header check still let a
    4-15 byte STATUS/RESPONSE IU match an inflight tag and read the
    zeroed buffer as SAM_STAT_GOOD / RC_TMF_COMPLETE.  Now enforce
    per-IU minimums (status byte / response_code) before
    dereferencing; READ/WRITE_READY legitimately carry only the
    4-byte header.
v2: use dev_err_ratelimited() so a device emitting a stream of
    short status URBs cannot spam the kernel log (per Greg KH).

    Tested: module reload + 16 forced disconnect/re-enumerate cycles
    under write load on ASMT135x (Alpine Ridge TB path) and Samsung
    T7 (AMD onboard xhci); compile-verified on 7.3-rc5 tree.

 drivers/usb/storage/uas.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 265162981..4e099f4a3 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);
@@ -337,6 +344,12 @@ static void uas_stat_cmplt(struct urb *urb)
 
 	switch (iu->iu_id) {
 	case IU_ID_STATUS:
+		if (urb->actual_length < offsetofend(struct sense_iu, status)) {
+			dev_err_ratelimited(&urb->dev->dev,
+				"stat urb: truncated STATUS IU (%u bytes)\n",
+				urb->actual_length);
+			break;
+		}
 		uas_sense(urb, cmnd);
 		if (cmnd->result != 0) {
 			/* cancel data transfers on error */
@@ -363,6 +376,13 @@ static void uas_stat_cmplt(struct urb *urb)
 		uas_xfer_data(urb, cmnd, SUBMIT_DATA_OUT_URB);
 		break;
 	case IU_ID_RESPONSE:
+		if (urb->actual_length < offsetofend(struct response_iu,
+						     response_code)) {
+			dev_err_ratelimited(&urb->dev->dev,
+				"stat urb: truncated RESPONSE IU (%u bytes)\n",
+				urb->actual_length);
+			break;
+		}
 		cmdinfo->state &= ~COMMAND_INFLIGHT;
 		success = uas_evaluate_response_iu((struct response_iu *)iu, cmnd);
 		if (!success) {
-- 
2.47.3