* [PATCH] usb: storage: bound timeout=0 URB waits to 30s
@ 2026-10-14 9:38 Hongyu Xie
2026-10-14 9:55 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Hongyu Xie @ 2026-10-14 9:38 UTC (permalink / raw)
To: stern, gregkh; +Cc: linux-usb, usb-storage, linux-kernel, Hongyu Xie, stable
usb_stor_msg_common() is the common URB submission/wait helper for
usb-storage transfers. Callers that pass a positive timeout get that
timeout, but the three internal transfer helpers (usb_stor_ctrl_transfer,
usb_stor_intr_transfer and usb_stor_bulk_transfer_buf) all pass 0,
which selects MAX_SCHEDULE_TIMEOUT: the control thread waits for the URB
forever.
A device that stops responding while such a transfer is pending hangs the
control thread permanently, and the hang propagates into the SCSI error
handler:
scsi command timeout -> eh_abort_handler -> command_abort()
-> usb_stor_stop_transport() /* URB unlink issued, but the URB of
a device wedged mid-transition
never completes */
-> wait_for_completion(&us->notify) /* no timeout */
The unbounded wait also closes a deadlock cycle observed on arm64 servers
running 5.4.x, where a Realtek USB WiFi dongle in CD-ROM mode (sr0)
wedges during usb_modeswitch's re-enumeration transition: the system
shutdown hook (mdadm --wait-clean --scan, which opens every block device)
blocks in disk_block_events() holding ev->block_mutex, waiting for the
disk events work whose TEST UNIT READY timed out into the
command_abort() wait above. No timeout anywhere in the cycle means the
shutdown hangs forever -- hung_task warnings every 120s until power loss,
two production incidents with identical stacks.
Bound the fallback wait to 30s, matching the SCSI layer's default command
timeout for disk and CD-ROM LUs (SD_TIMEOUT / SR_TIMEOUT). When the bound
fires, the URB is killed, the transport reports USB_STOR_XFER_ERROR, and
the command fails through the regular paths: the control thread
completes, notify is signalled, the abort returns, and the events work
(and with it the shutdown) can finish.
For disk and CD-ROM LUs this bound introduces no new policy: a transfer
that produced no URB completion for 30s has already exhausted the SCSI
layer's patience, and the error handler trying to abort it cannot make
progress while the control thread is stuck in the unbounded wait.
A blanket bound for this helper was suggested before [1] but dropped over
concern that legitimate SCSI commands (format, tape rewind) can exceed
60 seconds. Those commands are issued to tape LUs whose SCSI timeouts
are much longer than 30s; usb-storage-attached tapes holding back a CSW
for more than 30 seconds remain the residual false-positive window of
this change (modern tape bridges use UAS, which is unaffected).
The scatter-gather data path (usb_stor_bulk_transfer_sglist(), which
waits in usb_sg_wait()) is a separate unbounded wait and is not covered
by this change; a device that stalls mid data phase can still wedge the
control thread there.
Validation: on the affected production machine the hang occurred twice
across ~4800 hibernate cycles with identical stacks; with this bound in
place, 990+ hibernate cycles completed without a recurrence.
[1] https://lkml.rescloud.iu.edu/hypermail/linux/kernel/2207.0/05816.html
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn>
---
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -112,6 +112,15 @@
* command _must_ pass through this function (or something like it) for the
* abort mechanisms to work properly.
*/
+
+/*
+ * Bound for the URB wait when the caller passes timeout == 0: matches the
+ * SCSI layer's default command timeout for disk and CD-ROM LUs (SD_TIMEOUT /
+ * SR_TIMEOUT), so a wedged device surfaces as a regular command failure
+ * instead of hanging the control thread forever.
+ */
+#define USB_STOR_URB_TIMEOUT (30 * HZ)
+
static int usb_stor_msg_common(struct us_data *us, int timeout)
{
struct completion urb_done;
@@ -164,7 +173,7 @@
/* wait for the completion of the URB */
timeleft = wait_for_completion_interruptible_timeout(
- &urb_done, timeout ? : MAX_SCHEDULE_TIMEOUT);
+ &urb_done, timeout ? : USB_STOR_URB_TIMEOUT);
clear_bit(US_FLIDX_URB_ACTIVE, &us->dflags);
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] usb: storage: bound timeout=0 URB waits to 30s
2026-10-14 9:38 [PATCH] usb: storage: bound timeout=0 URB waits to 30s Hongyu Xie
@ 2026-10-14 9:55 ` sashiko-bot
2026-10-14 11:47 ` Greg KH
2026-10-14 14:44 ` [usb-storage] " Alan Stern
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-10-14 9:55 UTC (permalink / raw)
To: Hongyu Xie; +Cc: linux-usb
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb-storage: hardcoded 30s URB timeout breaks long-running SCSI commands
--
commit e1a1498b59035fae65ddf1b4b34a943e9c28791c
Author: Hongyu Xie <xiehongyu1@kylinos.cn>
usb: storage: bound timeout=0 URB waits to 30s
This commit bounds timeout=0 URB waits in usb_stor_msg_common() to 30 seconds
to prevent permanent hangs when a device stops responding mid-transfer. It
replaces MAX_SCHEDULE_TIMEOUT with a 30-second timeout, matching the SCSI
layer's default command timeout for disk and CD-ROM LUs.
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index 8de3015bce050..a6588e55312c2 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
[ ... ]
> @@ -164,7 +173,7 @@ static int usb_stor_msg_common(struct us_data *us, int timeout)
>
> /* wait for the completion of the URB */
> timeleft = wait_for_completion_interruptible_timeout(
> - &urb_done, timeout ? : MAX_SCHEDULE_TIMEOUT);
> + &urb_done, timeout ? : USB_STOR_URB_TIMEOUT);
[Severity: High]
Does this hardcoded 30-second timeout break legitimate long-running SCSI
commands like FORMAT UNIT or tape rewinds?
This logic unconditionally truncates unbounded transport waits to 30 seconds,
ignoring dynamic timeouts present in us->srb->request->timeout. A BOT SCSI
command taking longer than 30 seconds could legitimately withhold the CSW
until the operation finishes.
Could this cause usb_stor_msg_common() to kill the URB and return
USB_STOR_XFER_ERROR before the SCSI layer's own command timeout (which could
be hours) expires?
>
> clear_bit(US_FLIDX_URB_ACTIVE, &us->dflags);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261014093856.415812-1-xiehongyu1@kylinos.cn?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] usb: storage: bound timeout=0 URB waits to 30s
2026-10-14 9:38 [PATCH] usb: storage: bound timeout=0 URB waits to 30s Hongyu Xie
2026-10-14 9:55 ` sashiko-bot
@ 2026-10-14 11:47 ` Greg KH
2026-10-14 14:44 ` [usb-storage] " Alan Stern
2 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2026-10-14 11:47 UTC (permalink / raw)
To: Hongyu Xie; +Cc: stern, linux-usb, usb-storage, linux-kernel, stable
On Wed, Oct 14, 2026 at 05:38:56PM +0800, Hongyu Xie wrote:
> usb_stor_msg_common() is the common URB submission/wait helper for
> usb-storage transfers. Callers that pass a positive timeout get that
> timeout, but the three internal transfer helpers (usb_stor_ctrl_transfer,
> usb_stor_intr_transfer and usb_stor_bulk_transfer_buf) all pass 0,
> which selects MAX_SCHEDULE_TIMEOUT: the control thread waits for the URB
> forever.
>
> A device that stops responding while such a transfer is pending hangs the
> control thread permanently, and the hang propagates into the SCSI error
> handler:
>
> scsi command timeout -> eh_abort_handler -> command_abort()
> -> usb_stor_stop_transport() /* URB unlink issued, but the URB of
> a device wedged mid-transition
> never completes */
> -> wait_for_completion(&us->notify) /* no timeout */
>
> The unbounded wait also closes a deadlock cycle observed on arm64 servers
> running 5.4.x, where a Realtek USB WiFi dongle in CD-ROM mode (sr0)
> wedges during usb_modeswitch's re-enumeration transition: the system
> shutdown hook (mdadm --wait-clean --scan, which opens every block device)
> blocks in disk_block_events() holding ev->block_mutex, waiting for the
> disk events work whose TEST UNIT READY timed out into the
> command_abort() wait above. No timeout anywhere in the cycle means the
> shutdown hangs forever -- hung_task warnings every 120s until power loss,
> two production incidents with identical stacks.
>
> Bound the fallback wait to 30s, matching the SCSI layer's default command
> timeout for disk and CD-ROM LUs (SD_TIMEOUT / SR_TIMEOUT). When the bound
> fires, the URB is killed, the transport reports USB_STOR_XFER_ERROR, and
> the command fails through the regular paths: the control thread
> completes, notify is signalled, the abort returns, and the events work
> (and with it the shutdown) can finish.
>
> For disk and CD-ROM LUs this bound introduces no new policy: a transfer
> that produced no URB completion for 30s has already exhausted the SCSI
> layer's patience, and the error handler trying to abort it cannot make
> progress while the control thread is stuck in the unbounded wait.
>
> A blanket bound for this helper was suggested before [1] but dropped over
> concern that legitimate SCSI commands (format, tape rewind) can exceed
> 60 seconds. Those commands are issued to tape LUs whose SCSI timeouts
> are much longer than 30s; usb-storage-attached tapes holding back a CSW
> for more than 30 seconds remain the residual false-positive window of
> this change (modern tape bridges use UAS, which is unaffected).
>
> The scatter-gather data path (usb_stor_bulk_transfer_sglist(), which
> waits in usb_sg_wait()) is a separate unbounded wait and is not covered
> by this change; a device that stalls mid data phase can still wedge the
> control thread there.
>
> Validation: on the affected production machine the hang occurred twice
> across ~4800 hibernate cycles with identical stacks; with this bound in
> place, 990+ hibernate cycles completed without a recurrence.
>
> [1] https://lkml.rescloud.iu.edu/hypermail/linux/kernel/2207.0/05816.html
Please only use lore.kernel.org links.
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn>
> ---
No diffstat? How did you generate this?
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -112,6 +112,15 @@
> * command _must_ pass through this function (or something like it) for the
> * abort mechanisms to work properly.
> */
> +
> +/*
> + * Bound for the URB wait when the caller passes timeout == 0: matches the
> + * SCSI layer's default command timeout for disk and CD-ROM LUs (SD_TIMEOUT /
> + * SR_TIMEOUT), so a wedged device surfaces as a regular command failure
> + * instead of hanging the control thread forever.
Please rewrite this from scratch and do not use a LLM for it, as this
doesn't make much sense to me at all.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [usb-storage] [PATCH] usb: storage: bound timeout=0 URB waits to 30s
2026-10-14 9:38 [PATCH] usb: storage: bound timeout=0 URB waits to 30s Hongyu Xie
2026-10-14 9:55 ` sashiko-bot
2026-10-14 11:47 ` Greg KH
@ 2026-10-14 14:44 ` Alan Stern
2 siblings, 0 replies; 4+ messages in thread
From: Alan Stern @ 2026-10-14 14:44 UTC (permalink / raw)
To: Hongyu Xie; +Cc: gregkh, linux-usb, usb-storage, linux-kernel, stable
On Wed, Oct 14, 2026 at 05:38:56PM +0800, Hongyu Xie wrote:
> usb_stor_msg_common() is the common URB submission/wait helper for
> usb-storage transfers. Callers that pass a positive timeout get that
> timeout, but the three internal transfer helpers (usb_stor_ctrl_transfer,
> usb_stor_intr_transfer and usb_stor_bulk_transfer_buf) all pass 0,
> which selects MAX_SCHEDULE_TIMEOUT: the control thread waits for the URB
> forever.
>
> A device that stops responding while such a transfer is pending hangs the
> control thread permanently,
Why do you say it hangs _permanently_? I've seen plenty of examples
where the hang ended when the transfer was aborted.
> and the hang propagates into the SCSI error
> handler:
>
> scsi command timeout -> eh_abort_handler -> command_abort()
> -> usb_stor_stop_transport() /* URB unlink issued, but the URB of
> a device wedged mid-transition
> never completes */
Why do you say the URB never completes? Unlinking an URB should always
cause it to complete, whether the device is responding or not.
> -> wait_for_completion(&us->notify) /* no timeout */
>
> The unbounded wait also closes a deadlock cycle observed on arm64 servers
> running 5.4.x, where a Realtek USB WiFi dongle in CD-ROM mode (sr0)
> wedges during usb_modeswitch's re-enumeration transition: the system
> shutdown hook (mdadm --wait-clean --scan, which opens every block device)
> blocks in disk_block_events() holding ev->block_mutex, waiting for the
> disk events work whose TEST UNIT READY timed out into the
> command_abort() wait above. No timeout anywhere in the cycle means the
> shutdown hangs forever -- hung_task warnings every 120s until power loss,
> two production incidents with identical stacks.
Then the appropriate fix is to add a suitable timeout at the appropriate
point in this cycle. Not to force every command to be limited to a
pre-defined 30-second timeout.
> Bound the fallback wait to 30s, matching the SCSI layer's default command
> timeout for disk and CD-ROM LUs (SD_TIMEOUT / SR_TIMEOUT). When the bound
> fires, the URB is killed, the transport reports USB_STOR_XFER_ERROR, and
> the command fails through the regular paths: the control thread
> completes, notify is signalled, the abort returns, and the events work
> (and with it the shutdown) can finish.
>
> For disk and CD-ROM LUs this bound introduces no new policy: a transfer
> that produced no URB completion for 30s has already exhausted the SCSI
> layer's patience, and the error handler trying to abort it cannot make
> progress while the control thread is stuck in the unbounded wait.
>
> A blanket bound for this helper was suggested before [1] but dropped over
> concern that legitimate SCSI commands (format, tape rewind) can exceed
> 60 seconds. Those commands are issued to tape LUs whose SCSI timeouts
> are much longer than 30s; usb-storage-attached tapes holding back a CSW
> for more than 30 seconds remain the residual false-positive window of
> this change (modern tape bridges use UAS, which is unaffected).
>
> The scatter-gather data path (usb_stor_bulk_transfer_sglist(), which
> waits in usb_sg_wait()) is a separate unbounded wait and is not covered
> by this change; a device that stalls mid data phase can still wedge the
> control thread there.
Again, why doesn't the scsi error handler procedure unwedge the thread?
Alan Stern
> Validation: on the affected production machine the hang occurred twice
> across ~4800 hibernate cycles with identical stacks; with this bound in
> place, 990+ hibernate cycles completed without a recurrence.
>
> [1] https://lkml.rescloud.iu.edu/hypermail/linux/kernel/2207.0/05816.html
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn>
> ---
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -112,6 +112,15 @@
> * command _must_ pass through this function (or something like it) for the
> * abort mechanisms to work properly.
> */
> +
> +/*
> + * Bound for the URB wait when the caller passes timeout == 0: matches the
> + * SCSI layer's default command timeout for disk and CD-ROM LUs (SD_TIMEOUT /
> + * SR_TIMEOUT), so a wedged device surfaces as a regular command failure
> + * instead of hanging the control thread forever.
> + */
> +#define USB_STOR_URB_TIMEOUT (30 * HZ)
> +
> static int usb_stor_msg_common(struct us_data *us, int timeout)
> {
> struct completion urb_done;
> @@ -164,7 +173,7 @@
>
> /* wait for the completion of the URB */
> timeleft = wait_for_completion_interruptible_timeout(
> - &urb_done, timeout ? : MAX_SCHEDULE_TIMEOUT);
> + &urb_done, timeout ? : USB_STOR_URB_TIMEOUT);
>
> clear_bit(US_FLIDX_URB_ACTIVE, &us->dflags);
>
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "USB Mass Storage on Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to usb-storage+unsubscribe@lists.one-eyed-alien.net.
> To view this discussion visit https://groups.google.com/a/lists.one-eyed-alien.net/d/msgid/usb-storage/20261014093856.415812-1-xiehongyu1%40kylinos.cn.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-14 14:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-14 9:38 [PATCH] usb: storage: bound timeout=0 URB waits to 30s Hongyu Xie
2026-10-14 9:55 ` sashiko-bot
2026-10-14 11:47 ` Greg KH
2026-10-14 14:44 ` [usb-storage] " Alan Stern
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox