[PATCH 0/3] usb: uas / scsi: fail inflight and queued commands on dead links

[PATCH 0/3] usb: uas / scsi: fail inflight and queued commands on dead links

[Sarthak Kukreti]

Unplugging a UAS device during system suspend after hub_wq is frozen
defers usb_disconnect() until hub_wq thaws. When sd_suspend_system()
issues SYNCHRONIZE_CACHE to the disconnected target:

1. If the HCD accepts the URBs, the status URB fails in uas_stat_cmplt()
   without completing inflight commands, stranding SYNCHRONIZE_CACHE for
   the 60s SCSI timeout and triggering the 25s DPM watchdog. Patch 1
   completes inflight commands with DID_NO_CONNECT on non-unlink status
   URB errors, unlinks inflight data URBs, and preserves non-DID_OK host
   bytes in uas_data_cmplt().
2. If usb_submit_urb() fails with -ESHUTDOWN, -ENOENT, or -EHOSTUNREACH,
   uas_queuecommand_lck() requeues the command instead of failing it,
   and leaks unsubmitted URBs if status URB submission succeeded first.
   Patch 2 fails the command with DID_NO_CONNECT and frees unsubmitted
   URBs.
3. When the command completes with DID_NO_CONNECT,
   scsi_check_passthrough() matches SCMD_FAILURE_RESULT_ANY and retries
   it against the disconnected target. Patch 3 skips retries on
   DID_NO_CONNECT, DID_BAD_TARGET, and DID_TRANSPORT_FAILFAST.

Tested using a USB/IP (vhci_hcd) UAS target emulator that freezes
hub_wq, injects -EPROTO on the status URB during inflight
SYNCHRONIZE_CACHE and READ(10) commands, and verifies immediate
DID_NO_CONNECT completion without retries or DPM watchdog timeouts,
along with the scsi_lib KUnit test suite.

Sarthak Kukreti (3):
  usb: uas: complete inflight commands on status URB error
  usb: uas: fail command submission on fatal USB errors
  scsi: core: do not retry passthrough requests on disconnected targets

 drivers/scsi/scsi_lib.c      |  5 ++++
 drivers/scsi/scsi_lib_test.c |  9 ++++++-
 drivers/usb/storage/uas.c    | 51 +++++++++++++++++++++++++++++++++---
 3 files changed, 60 insertions(+), 5 deletions(-)

-- 
2.56.0.611.g1030f0b940-goog

[PATCH 1/3] usb: uas: complete inflight commands on status URB error

[Sarthak Kukreti]

When a UAS device is unplugged while hub_wq is frozen during system
suspend, usb_disconnect() is deferred and sd_sync_cache() issues
SYNCHRONIZE_CACHE to the disconnected target. If the status URB fails in
uas_stat_cmplt() with a non-unlink error (such as -EPROTO, -EILSEQ,
-ETIME, or -ESHUTDOWN), uas_stat_cmplt() logs the error and frees the
URB without completing inflight commands, leaving SYNCHRONIZE_CACHE
pending until the 60s SCSI timeout and triggering the 25s DPM watchdog.

Complete inflight commands with DID_NO_CONNECT in uas_stat_cmplt() on
non-unlink status URB errors while not resetting, and unlink inflight
data URBs outside devinfo->lock. Preserve non-DID_OK host bytes in
uas_data_cmplt() when the unlinked data URB completes.

Signed-off-by: Sarthak Kukreti <sarthakkukreti@google.com>
---
 drivers/usb/storage/uas.c | 47 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 265162981269..5aa369087295 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -310,9 +310,49 @@ static void uas_stat_cmplt(struct urb *urb)
 	bool success;
 
 	if (status) {
-		if (status != -ENOENT && status != -ECONNRESET && status != -ESHUTDOWN)
+		if (status == -ENOENT || status == -ECONNRESET)
+			goto bail;
+		if (status != -ESHUTDOWN)
 			dev_err(&urb->dev->dev, "stat urb: status %d\n", status);
-		goto bail;
+
+		spin_lock_irqsave(&devinfo->lock, flags);
+		if (devinfo->resetting)
+			goto out;
+
+		for (idx = 0; idx < devinfo->qdepth; idx++) {
+			cmnd = devinfo->cmnd[idx];
+			if (!cmnd)
+				continue;
+			cmdinfo = scsi_cmd_priv(cmnd);
+			if (!(cmdinfo->state & COMMAND_INFLIGHT))
+				continue;
+
+			if (cmdinfo->state & DATA_IN_URB_INFLIGHT)
+				data_in_urb = usb_get_urb(cmdinfo->data_in_urb);
+			if (cmdinfo->state & DATA_OUT_URB_INFLIGHT)
+				data_out_urb = usb_get_urb(cmdinfo->data_out_urb);
+			cmdinfo->state &= ~COMMAND_INFLIGHT;
+			set_host_byte(cmnd, DID_NO_CONNECT);
+			uas_try_complete(cmnd, __func__);
+
+			if (data_in_urb || data_out_urb) {
+				spin_unlock_irqrestore(&devinfo->lock, flags);
+				if (data_in_urb) {
+					usb_unlink_urb(data_in_urb);
+					usb_put_urb(data_in_urb);
+					data_in_urb = NULL;
+				}
+				if (data_out_urb) {
+					usb_unlink_urb(data_out_urb);
+					usb_put_urb(data_out_urb);
+					data_out_urb = NULL;
+				}
+				spin_lock_irqsave(&devinfo->lock, flags);
+				if (devinfo->resetting)
+					break;
+			}
+		}
+		goto out;
 	}
 
 	idx = be16_to_cpup(&iu->tag) - 1;
@@ -428,7 +468,8 @@ static void uas_data_cmplt(struct urb *urb)
 			uas_log_cmd_state(cmnd, "data cmplt err", status);
 		/* error: no data transfered */
 		scsi_set_resid(cmnd, sdb->length);
-		set_host_byte(cmnd, DID_ERROR);
+		if (host_byte(cmnd->result) == DID_OK)
+			set_host_byte(cmnd, DID_ERROR);
 	} else {
 		scsi_set_resid(cmnd, sdb->length - urb->actual_length);
 	}
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH 1/3] usb: uas: complete inflight commands on status URB error

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] usb: uas: uas_stat_cmplt() orphans status URBs in
  devinfo->sense_urbs
- [Medium] usb: uas: uas_stat_cmplt() sets DID_NO_CONNECT on
  non-disconnect errors
- [Medium] usb: uas: uas_stat_cmplt() skips commands without
  COMMAND_INFLIGHT
- [Medium] usb: uas: uas_stat_cmplt() drops lock mid-loop and aborts new
  commands
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Critical] usb: uas: unvalidated IU length causes false success in
  uas_sense() [1]
--

commit feee884d21c2440f5410ec8ac6400834915e4ef7
Author: Sarthak Kukreti <sarthakkukreti@google.com>

usb: uas: complete inflight commands on status URB error

This commit completes inflight SCSI commands with DID_NO_CONNECT and unlinks
their data URBs when a status URB fails with a non-unlink error in
uas_stat_cmplt(), while preserving existing non-DID_OK host status bytes in
uas_data_cmplt().

> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 8655edbd66b16..3aadf4c88c5c2 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -310,9 +310,49 @@ static void uas_stat_cmplt(struct urb *urb)
>  	bool success;
>  
>  	if (status) {
> -		if (status != -ENOENT && status != -ECONNRESET && status != -ESHUTDOWN)
> +		if (status == -ENOENT || status == -ECONNRESET)
> +			goto bail;
> +		if (status != -ESHUTDOWN)
>  			dev_err(&urb->dev->dev, "stat urb: status %d\n", status);
> -		goto bail;
> +
> +		spin_lock_irqsave(&devinfo->lock, flags);
> +		if (devinfo->resetting)
> +			goto out;
> +
> +		for (idx = 0; idx < devinfo->qdepth; idx++) {
> +			cmnd = devinfo->cmnd[idx];
> +			if (!cmnd)
> +				continue;
> +			cmdinfo = scsi_cmd_priv(cmnd);
> +			if (!(cmdinfo->state & COMMAND_INFLIGHT))
> +				continue;

[Severity: Medium]
Does skipping commands without COMMAND_INFLIGHT leave partially submitted
commands in devinfo->cmnd[] hung?

In uas_submit_urbs(), uas_submit_sense_urb() submits the status URB and
clears SUBMIT_STATUS_URB before the data and command URBs are submitted.
If a data URB is submitted and cmd_urb submission then fails with -ENODEV,
uas_queuecommand_lck() records cmnd in devinfo->cmnd[idx] with
SUBMIT_CMD_URB still set and COMMAND_INFLIGHT unset:

drivers/usb/storage/uas.c:uas_queuecommand_lck() {
	...
	if (err == -ENODEV) {
		if (cmdinfo->state & (COMMAND_INFLIGHT | DATA_IN_URB_INFLIGHT |
				DATA_OUT_URB_INFLIGHT))
			goto out;
	...
out:
	devinfo->cmnd[idx] = cmnd;
}

When that command's status URB fails here, uas_stat_cmplt() skips it
because COMMAND_INFLIGHT is not set, and when its data URB finishes in
uas_data_cmplt(), the SUBMIT_CMD_URB check also skips uas_try_complete(),
leaving the command pending until the SCSI timeout fires.  Similarly, if
uas_submit_urbs() queued the command to devinfo->work after submitting the
status URB, skipping it here consumes the status URB before uas_do_work()
submits cmd_urb, and if an inflight command has IS_IN_WORK_LIST set, not
clearing IS_IN_WORK_LIST before dropping devinfo->lock below allows
uas_do_work() to resubmit URBs for the command being aborted.

> +
> +			if (cmdinfo->state & DATA_IN_URB_INFLIGHT)
> +				data_in_urb = usb_get_urb(cmdinfo->data_in_urb);
> +			if (cmdinfo->state & DATA_OUT_URB_INFLIGHT)
> +				data_out_urb = usb_get_urb(cmdinfo->data_out_urb);
> +			cmdinfo->state &= ~COMMAND_INFLIGHT;
> +			set_host_byte(cmnd, DID_NO_CONNECT);

[Severity: Medium]
Should non-disconnect status URB errors set DID_ERROR instead of
DID_NO_CONNECT?

This branch runs for any status URB error other than -ENOENT and
-ECONNRESET, including endpoint stalls (-EPIPE) and transient USB transfer
errors (-EPROTO, -EILSEQ, -ETIME) on a still-connected device.
Because scsi_decide_disposition() treats DID_NO_CONNECT as an immediate
non-retryable failure without invoking SCSI error recovery, all inflight
block I/O fails immediately with -EIO, a stalled status endpoint is never
reset, and sd_sync_cache() (which ignores DID_NO_CONNECT) silently reports
a failed SYNCHRONIZE_CACHE as successful.

> +			uas_try_complete(cmnd, __func__);

[Severity: High]
When multiple commands are in flight and one status URB fails, does
completing every command in devinfo->cmnd[] without unlinking
devinfo->sense_urbs orphan the remaining commands' status URBs?

Each submitted command allocates its own status URB anchored in
devinfo->sense_urbs (bound to stream_id = cmdinfo->uas_tag when USB 3.0
streams are enabled).  When one status URB fails, this loop clears
devinfo->cmnd[idx] and calls scsi_done() for all other inflight commands
while their status URBs remain active in the HCD.

Those still-anchored status URBs cause uas_wait_for_pending_cmnds() in
uas_suspend() and uas_pre_reset() to time out waiting on
devinfo->sense_urbs after 5 seconds.  In addition, if another SCSI command
is queued before the orphaned status URB finishes, it reuses the freed
tag slot in devinfo->cmnd[idx]:

CPU0                                  CPU1
uas_stat_cmplt(urb_A)
  /* fails with -EPROTO */
  /* completes cmnd_B at idx = 1 */
  devinfo->cmnd[1] = NULL
  scsi_done(cmnd_B)
                                      uas_queuecommand_lck(cmnd_C)
                                        /* reuses idx = 1, submits urb_C */
                                        devinfo->cmnd[1] = cmnd_C
uas_stat_cmplt(urb_B)
  /* orphaned status URB for idx = 1 */
  uas_try_complete(cmnd_C)            <-- completes cmnd_C prematurely

> +
> +			if (data_in_urb || data_out_urb) {
> +				spin_unlock_irqrestore(&devinfo->lock, flags);
> +				if (data_in_urb) {
> +					usb_unlink_urb(data_in_urb);
> +					usb_put_urb(data_in_urb);
> +					data_in_urb = NULL;
> +				}
> +				if (data_out_urb) {
> +					usb_unlink_urb(data_out_urb);
> +					usb_put_urb(data_out_urb);
> +					data_out_urb = NULL;
> +				}
> +				spin_lock_irqsave(&devinfo->lock, flags);
> +				if (devinfo->resetting)
> +					break;
> +			}
> +		}
> +		goto out;
>  	}
>  
>  	idx = be16_to_cpup(&iu->tag) - 1;

[Severity: Medium]
Can dropping devinfo->lock inside the loop cause later iterations to abort
newly queued commands?

When a command at slot idx has a data URB in flight, uas_stat_cmplt()
drops devinfo->lock to call usb_unlink_urb() and then continues scanning
higher indices:

CPU0                                  CPU1
uas_stat_cmplt()
  /* idx = 0 has data_in_urb */
  spin_unlock_irqrestore(&devinfo->lock)
  usb_unlink_urb(data_in_urb)
                                      uas_queuecommand_lck()
                                        /* places new cmd at idx = 1 */
                                        devinfo->cmnd[1] = new_cmnd
  spin_lock_irqsave(&devinfo->lock)
  /* loop advances to idx = 1 */
  set_host_byte(new_cmnd, DID_NO_CONNECT)
  uas_try_complete(new_cmnd, ...)     <-- aborts newly queued command

[Severity: Critical]
This problem wasn't introduced by this patch, but can a truncated status
IU in uas_stat_cmplt() cause uas_sense() to report a failed command as
SAM_STAT_GOOD [1]?

When status is 0, uas_stat_cmplt() dispatches IU_ID_STATUS to uas_sense()
without checking urb->actual_length:

drivers/usb/storage/uas.c:uas_sense() {
	struct sense_iu *sense_iu = urb->transfer_buffer;
	struct scsi_device *sdev = cmnd->device;

	if (urb->actual_length > 16) {
		...
	}

	cmnd->result = sense_iu->status;
}

Because the status URB transfer buffer is allocated with kzalloc(), a
short packet of 4 to 6 bytes has a valid iu_id and tag while
sense_iu->status at offset 6 remains zero (SAM_STAT_GOOD), completing the
command as successful and silently corrupting read or write I/O.

[1] https://sashiko.dev/#/bug/linux-78c5dae0-c194-40ff-a554-3a4de1859b86

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113003536.3622755-1-sarthakkukreti@google.com?part=1

[PATCH 2/3] usb: uas: fail command submission on fatal USB errors

[Sarthak Kukreti]

uas_queuecommand_lck() only treats -ENODEV from uas_submit_urbs() as a
fatal submission error. When a link drops or an endpoint is disabled
while hub_wq is frozen, usb_submit_urb() can also fail with -ESHUTDOWN,
-ENOENT, or -EHOSTUNREACH, causing uas_queuecommand_lck() to return
SCSI_MLQUEUE_DEVICE_BUSY or queue retry work. In addition, if status URB
submission succeeds before command URB submission fails with no inflight
URBs, calling scsi_done() directly leaks the unsubmitted cmd_urb.

Fail commands with DID_NO_CONNECT on -ESHUTDOWN, -ENOENT, and
-EHOSTUNREACH in uas_queuecommand_lck(), and free unsubmitted URBs
before calling scsi_done().

Signed-off-by: Sarthak Kukreti <sarthakkukreti@google.com>
---
 drivers/usb/storage/uas.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 5aa369087295..6b573de89162 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -743,11 +743,13 @@ static enum scsi_qc_status uas_queuecommand_lck(struct scsi_cmnd *cmnd)
 	 * a command that has finished is a success for the purpose
 	 * of queueing, no matter how fatal the error
 	 */
-	if (err == -ENODEV) {
+	if (err == -ENODEV || err == -ESHUTDOWN || err == -ENOENT ||
+	    err == -EHOSTUNREACH) {
 		if (cmdinfo->state & (COMMAND_INFLIGHT | DATA_IN_URB_INFLIGHT |
 				DATA_OUT_URB_INFLIGHT))
 			goto out;
 
+		uas_free_unsubmitted_urbs(cmnd);
 		set_host_byte(cmnd, DID_NO_CONNECT);
 		scsi_done(cmnd);
 		goto zombie;
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH 2/3] usb: uas: fail command submission on fatal USB errors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] usb: uas: command hang and cmd_urb leak in
  uas_queuecommand_lck()
- [Medium] usb: uas: orphaned sense URB on error in
  uas_queuecommand_lck()
- [Low] usb: uas: missing Fixes trailer for uas_queuecommand_lck() fix
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Critical] usb: uas: use-after-free in uas_queuecommand_lck() via
  negative qdepth [1]
--

commit d4b0bad1326a981836c51fc29f205193a0a134fb
Author: Sarthak Kukreti <sarthakkukreti@google.com>

> usb: uas: fail command submission on fatal USB errors
> 
> uas_queuecommand_lck() only treats -ENODEV from uas_submit_urbs() as a
> fatal submission error. When a link drops or an endpoint is disabled
> while hub_wq is frozen, usb_submit_urb() can also fail with -ESHUTDOWN,
> -ENOENT, or -EHOSTUNREACH, causing uas_queuecommand_lck() to return
> SCSI_MLQUEUE_DEVICE_BUSY or queue retry work. In addition, if status URB
> submission succeeds before command URB submission fails with no inflight
> URBs, calling scsi_done() directly leaks the unsubmitted cmd_urb.
> 
> Fail commands with DID_NO_CONNECT on -ESHUTDOWN, -ENOENT, and
> -EHOSTUNREACH in uas_queuecommand_lck(), and free unsubmitted URBs
> before calling scsi_done().
> 
> Signed-off-by: Sarthak Kukreti <sarthakkukreti@google.com>

[Severity: Low]
Since this fixes the incomplete fatal submission error handling and URB
leak in uas_queuecommand_lck(), should it include Fixes: tags for
eb2a86ae8c54 ("USB: UAS: fix disconnect by unplugging a hub") and
cd5432c71235 ("USB: UAS: return ENODEV when submit urbs fail with device
not attached")?

> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 3aadf4c88c5c2..843860687542d 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -743,11 +743,13 @@ static enum scsi_qc_status uas_queuecommand_lck(struct scsi_cmnd *cmnd)
>  	 * a command that has finished is a success for the purpose
>  	 * of queueing, no matter how fatal the error
>  	 */
> -	if (err == -ENODEV) {
> +	if (err == -ENODEV || err == -ESHUTDOWN || err == -ENOENT ||
> +	    err == -EHOSTUNREACH) {
>  		if (cmdinfo->state & (COMMAND_INFLIGHT | DATA_IN_URB_INFLIGHT |
>  				DATA_OUT_URB_INFLIGHT))
>  			goto out;

[Severity: Medium]
Can jumping to out here leak cmdinfo->cmd_urb and hang the command when
uas_submit_urbs() fails after submitting a data URB on a streams-enabled
device?

In uas_submit_urbs(), data URBs are submitted before cmd_urb, and
SUBMIT_CMD_URB is only cleared (and COMMAND_INFLIGHT set) once cmd_urb is
submitted:

drivers/usb/storage/uas.c:uas_submit_urbs() {
	...
	if (cmdinfo->state & SUBMIT_CMD_URB) {
		usb_anchor_urb(cmdinfo->cmd_urb, &devinfo->cmd_urbs);
		err = usb_submit_urb(cmdinfo->cmd_urb, GFP_ATOMIC);
		if (err) {
			usb_unanchor_urb(cmdinfo->cmd_urb);
			uas_log_cmd_state(cmnd, "cmd submit err", err);
			return err;
		}
		cmdinfo->cmd_urb = NULL;
		cmdinfo->state &= ~SUBMIT_CMD_URB;
		cmdinfo->state |= COMMAND_INFLIGHT;
	}
	return 0;
}

When cmd_urb submission fails after DATA_IN_URB_INFLIGHT or
DATA_OUT_URB_INFLIGHT is set, jumping to out skips
uas_free_unsubmitted_urbs(cmnd) and leaves SUBMIT_CMD_URB set and
COMMAND_INFLIGHT clear.  When the in-flight data URB later completes while
devinfo->resetting is 0, uas_data_cmplt() sees SUBMIT_CMD_URB set and
returns early without calling uas_try_complete(), while uas_stat_cmplt()
skips commands without COMMAND_INFLIGHT.  The command remains stuck in
devinfo->cmnd[idx] until the SCSI error handler times out, and
uas_wait_for_pending_cmnds() in uas_suspend() times out after 5 seconds
with -ETIME.

>  
> +		uas_free_unsubmitted_urbs(cmnd);
>  		set_host_byte(cmnd, DID_NO_CONNECT);
>  		scsi_done(cmnd);
>  		goto zombie;

[Severity: Medium]
Does completing cmnd here leave an orphaned sense URB anchored on
devinfo->sense_urbs when uas_submit_sense_urb() succeeded before a later
URB submission failed?

uas_submit_urbs() submits the sense URB first and anchors it to
devinfo->sense_urbs.  If cmd_urb (or the first data URB) then fails with
no data URB in flight, uas_free_unsubmitted_urbs() frees only cmd_urb and
the data URBs, and scsi_done() completes cmnd without unlinking the sense
URB.  If status_pipe has not been flushed while hub_wq is frozen,
usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000) in
uas_wait_for_pending_cmnds() will time out with -ETIME during suspend or
reset, and the tag can be reused while the old sense URB is still queued.

[Severity: Critical]
This problem wasn't introduced by this patch, but can a negative
devinfo->qdepth [1] cause a use-after-free in uas_queuecommand_lck()?

If usb_alloc_streams() fails during reset or resume,
uas_configure_endpoints() stores the negative error code directly in
devinfo->qdepth:

drivers/usb/storage/uas.c:uas_configure_endpoints() {
	...
	} else {
		devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1,
						    3, MAX_CMNDS, GFP_NOIO);
		if (devinfo->qdepth < 0)
			return devinfo->qdepth;
	...
}

When subsequent commands enter uas_queuecommand_lck(), the loop condition
idx < devinfo->qdepth is immediately false with idx == 0, and the
queue-full check idx == devinfo->qdepth is bypassed:

drivers/usb/storage/uas.c:uas_queuecommand_lck() {
	...
	for (idx = 0; idx < devinfo->qdepth; idx++) {
		if (!devinfo->cmnd[idx])
			break;
	}
	if (idx == devinfo->qdepth) {
		spin_unlock_irqrestore(&devinfo->lock, flags);
		return SCSI_MLQUEUE_DEVICE_BUSY;
	}
	...
out:
	devinfo->cmnd[idx] = cmnd;
	...
}

Every new command then overwrites devinfo->cmnd[0] and reuses tag 1,
abandoning any in-flight command at slot 0 and causing a use-after-free
when its URBs complete; uas_sdev_configure() also passes devinfo->qdepth - 2
to scsi_change_queue_depth().

[1] https://sashiko.dev/#/bug/linux-30cdc430-800b-42d3-908f-c7a0b944e7f9

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113003536.3622755-1-sarthakkukreti@google.com?part=2

[PATCH 3/3] scsi: core: do not retry passthrough requests on disconnected targets

[Sarthak Kukreti]

Unlike scsi_decide_disposition(), scsi_check_passthrough() matches
SCMD_FAILURE_RESULT_ANY before checking the host byte and returns
-EAGAIN on DID_NO_CONNECT, DID_BAD_TARGET, and DID_TRANSPORT_FAILFAST.
Callers using SCMD_FAILURE_RESULT_ANY (such as sd_sync_cache())
therefore retry passthrough commands against disconnected targets.

Return 0 from scsi_check_passthrough() when the host byte is
DID_NO_CONNECT, DID_BAD_TARGET, or DID_TRANSPORT_FAILFAST, and update
scsi_lib_test_any_failure().

Signed-off-by: Sarthak Kukreti <sarthakkukreti@google.com>
---
 drivers/scsi/scsi_lib.c      | 5 +++++
 drivers/scsi/scsi_lib_test.c | 9 ++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index af27fd3df8d4..fa16a75e810c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -219,6 +219,11 @@ static int scsi_check_passthrough(struct scsi_cmnd *scmd,
 	if (!scmd->result)
 		return 0;
 
+	if (host_byte(scmd->result) == DID_NO_CONNECT ||
+	    host_byte(scmd->result) == DID_BAD_TARGET ||
+	    host_byte(scmd->result) == DID_TRANSPORT_FAILFAST)
+		return 0;
+
 	if (!failures)
 		return 0;
 
diff --git a/drivers/scsi/scsi_lib_test.c b/drivers/scsi/scsi_lib_test.c
index ae8af0e0047a..aafafe676098 100644
--- a/drivers/scsi/scsi_lib_test.c
+++ b/drivers/scsi/scsi_lib_test.c
@@ -184,8 +184,15 @@ static void scsi_lib_test_any_failure(struct kunit *test)
 
 	/* Match SCMD_FAILURE_RESULT_ANY */
 	failures.failure_definitions = any_failure_defs;
-	sc.result = DID_TRANSPORT_FAILFAST << 16;
+	sc.result = DID_ERROR << 16;
 	KUNIT_EXPECT_EQ(test, -EAGAIN, scsi_check_passthrough(&sc, &failures));
+
+	sc.result = DID_NO_CONNECT << 16;
+	KUNIT_EXPECT_EQ(test, 0, scsi_check_passthrough(&sc, &failures));
+	sc.result = DID_BAD_TARGET << 16;
+	KUNIT_EXPECT_EQ(test, 0, scsi_check_passthrough(&sc, &failures));
+	sc.result = DID_TRANSPORT_FAILFAST << 16;
+	KUNIT_EXPECT_EQ(test, 0, scsi_check_passthrough(&sc, &failures));
 }
 
 static void scsi_lib_test_any_status(struct kunit *test)
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH 3/3] scsi: core: do not retry passthrough requests on disconnected targets

[sashiko-bot]

> Unlike scsi_decide_disposition(), scsi_check_passthrough() matches
> SCMD_FAILURE_RESULT_ANY before checking the host byte and returns
> -EAGAIN on DID_NO_CONNECT, DID_BAD_TARGET, and DID_TRANSPORT_FAILFAST.
> Callers using SCMD_FAILURE_RESULT_ANY (such as sd_sync_cache())
> therefore retry passthrough commands against disconnected targets.
> 
> Return 0 from scsi_check_passthrough() when the host byte is
> DID_NO_CONNECT, DID_BAD_TARGET, or DID_TRANSPORT_FAILFAST, and update
> scsi_lib_test_any_failure().
> 
> Signed-off-by: Sarthak Kukreti <sarthakkukreti@google.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113003536.3622755-1-sarthakkukreti@google.com?part=3