[PATCH] usb-storage: prevent concurrent URB submission in usb_stor_msg_common

[PATCH] usb-storage: prevent concurrent URB submission in usb_stor_msg_common

[liuqi]

[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]

From 054ed55aaa0d80edc93d25e4750d67e9064bb77f Mon Sep 17 00:00:00 2001
From: liuqi <liuqi@longcheer.com>
Date: Thu, 20 Aug 2026 11:28:43 +0800
Subject: [PATCH] usb-storage: prevent concurrent URB submission in
 usb_stor_msg_common

usb_stor_msg_common() only checks the ABORTING flag before submitting
the URB, but does not guard against concurrent submissions when a
previous URB is still in-flight (e.g., from a BULK_MAX_LUN probe
running in scan_dwork while scsi_eh also invokes the same function).

This leads to "URB submitted while active" warnings and -EINPROGRESS
returns when usb_submit_urb() detects an already-queued/in-flight URB.

Fix by using test_and_set_bit() on US_FLIDX_URB_ACTIVE to mutually
exclude concurrent submissions. If a URB is already active, return
-EAGAIN to the caller.

Reported-by: syzbot+22ea20ef3afb6785b122@syzkaller.appspotmail.com
Signed-off-by: liuqi <liuqi@longcheer.com>
---
 drivers/usb/storage/transport.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index f79b449d0..ffb1459d0 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -122,6 +122,16 @@ static int usb_stor_msg_common(struct us_data *us, int timeout)
 	if (test_bit(US_FLIDX_ABORTING, &us->dflags))
 		return -EIO;
 
+	/*
+	 * Prevent concurrent submissions of the same URB.
+	 * If a URB is already in-flight (e.g., from a previous
+	 * BULK_MAX_LUN probe running in scan_dwork while scsi_eh
+	 * also invokes the same function), reject new submissions.
+	 */
+	if (test_and_set_bit(US_FLIDX_URB_ACTIVE, &us->dflags))
+		return -EAGAIN;
+
+
 	/* set up data structures for the wakeup system */
 	init_completion(&urb_done);
 
-- 
2.43.0

Re: [PATCH] usb-storage: prevent concurrent URB submission in usb_stor_msg_common

[Alan Stern]

On Thu, Aug 20, 2026 at 12:03:45PM +0800, liuqi wrote:
> From 054ed55aaa0d80edc93d25e4750d67e9064bb77f Mon Sep 17 00:00:00 2001
> From: liuqi <liuqi@longcheer.com>
> Date: Thu, 20 Aug 2026 11:28:43 +0800
> Subject: [PATCH] usb-storage: prevent concurrent URB submission in
>  usb_stor_msg_common
> 
> usb_stor_msg_common() only checks the ABORTING flag before submitting
> the URB, but does not guard against concurrent submissions when a
> previous URB is still in-flight (e.g., from a BULK_MAX_LUN probe
> running in scan_dwork while scsi_eh also invokes the same function).

Why would the scsi_eh (or any other part of the SCSI stack) submit 
commands before scsi_scan_host() has started?

> This leads to "URB submitted while active" warnings and -EINPROGRESS
> returns when usb_submit_urb() detects an already-queued/in-flight URB.

Have you actually observed this happening?

Alan Stern

> Fix by using test_and_set_bit() on US_FLIDX_URB_ACTIVE to mutually
> exclude concurrent submissions. If a URB is already active, return
> -EAGAIN to the caller.
> 
> Reported-by: syzbot+22ea20ef3afb6785b122@syzkaller.appspotmail.com
> Signed-off-by: liuqi <liuqi@longcheer.com>
> ---
>  drivers/usb/storage/transport.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index f79b449d0..ffb1459d0 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -122,6 +122,16 @@ static int usb_stor_msg_common(struct us_data *us, int timeout)
>  	if (test_bit(US_FLIDX_ABORTING, &us->dflags))
>  		return -EIO;
>  
> +	/*
> +	 * Prevent concurrent submissions of the same URB.
> +	 * If a URB is already in-flight (e.g., from a previous
> +	 * BULK_MAX_LUN probe running in scan_dwork while scsi_eh
> +	 * also invokes the same function), reject new submissions.
> +	 */
> +	if (test_and_set_bit(US_FLIDX_URB_ACTIVE, &us->dflags))
> +		return -EAGAIN;
> +
> +
>  	/* set up data structures for the wakeup system */
>  	init_completion(&urb_done);
>  
> -- 
> 2.43.0
> 

Re: [PATCH] usb-storage: prevent concurrent URB submission in usb_stor_msg_common

[liuqi]

[-- Attachment #1: Type: text/plain, Size: 1678 bytes --]

From 2104fc57aacc74197aa81a72d283cad3351e59f9 Mon Sep 17 00:00:00 2001
From: liuqi <liuqi@longcheer.com>
Date: Fri, 21 Aug 2026 13:08:16 +0800
Subject: [PATCH] usb-storage: serialize ene_get_card_type() with dev_mutex
To: gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org,
    stable@vger.kernel.org

In ene_ub6250_probe(), the card-type probe (ene_get_card_type()) uses the
usb-storage bulk transfer helpers which reuse us->current_urb. However,
usb_stor_probe2() schedules a delayed scan work (scan_dwork) that also
uses current_urb for Bulk_max_lun(). The scan work is serialized with
us->dev_mutex, but the probe's ene_get_card_type() call is not.

This leads to "URB submitted while active" warnings when the delayed
scan fires during the probe's ene_get_card_type() execution. The warning
trigger is:

  usb_stor_Bulk_max_lun() -> usb_stor_msg_common() -> usb_submit_urb()

No scsi_eh involvement was observed in syzbot logs.

Fix by serializing ene_get_card_type() with us->dev_mutex.

Signed-off-by: liuqi <liuqi@longcheer.com>
---
 drivers/usb/storage/ene_ub6250.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index 2cffc559a..13b332d1d 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -2358,7 +2358,9 @@ static int ene_ub6250_probe(struct usb_interface *intf,
 		return result;
 
 	/* probe card type */
+	mutex_lock(&us->dev_mutex);
 	result = ene_get_card_type(us, REG_CARD_STATUS, info->bbuf);
+	mutex_unlock(&us->dev_mutex);
 	if (result != USB_STOR_XFER_GOOD) {
 		usb_stor_disconnect(intf);
 		return USB_STOR_TRANSPORT_ERROR;
-- 
2.43.0

Re: [PATCH] usb-storage: prevent concurrent URB submission in usb_stor_msg_common

[Greg KH]

On Fri, Aug 21, 2026 at 01:14:11PM +0800, liuqi wrote:
> From 2104fc57aacc74197aa81a72d283cad3351e59f9 Mon Sep 17 00:00:00 2001
> From: liuqi <liuqi@longcheer.com>
> Date: Fri, 21 Aug 2026 13:08:16 +0800
> Subject: [PATCH] usb-storage: serialize ene_get_card_type() with dev_mutex
> To: gregkh@linuxfoundation.org
> Cc: linux-usb@vger.kernel.org,
>     stable@vger.kernel.org

Something went wrong here :(

Re: [PATCH] usb-storage: prevent concurrent URB submission in usb_stor_msg_common

[liuqi]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

Alan, thank you for the review. You were right to question my original explanation.

I rechecked the syzbot console logs, and I do not see evidence that scsi_eh is involved. The warning is triggered from the delayed scan work while issuing GET_MAX_LUN:

  usb_stor_scan_dwork()
    usb_stor_Bulk_max_lun()
      usb_stor_control_msg()
        usb_stor_msg_common()
          usb_submit_urb()

The device in the reports is the ENE UB6250 reader (0cf2:6250). Looking at the driver again, the more plausible issue is specific to ums_eneub6250: ene_ub6250_probe() calls usb_stor_probe2(), which schedules the delayed scan work, and then calls ene_get_card_type(). That path sends a command through ene_send_scsi_cmd() and the usb-storage transfer helpers, using us->current_urb, but it is not serialized with us->dev_mutex. The scan work does take us->dev_mutex around usb_stor_Bulk_max_lun().

So the syzbot report strongly suggests a race between the ENE card-type probe and the delayed scan work, rather than anything involving scsi_eh.

I will drop the generic usb_stor_msg_common() change and send a narrower v2 patch for ums_eneub6250 instead.

Sorry for the incorrect analysis in the original submission, and thanks again for pointing it out.

Regards,
liuqi