[PATCH v4] usb-storage: ene_ub6250: don't let the card-type probe hang forever

[PATCH v4] usb-storage: ene_ub6250: don't let the card-type probe hang forever

[Hongyu Xie]

ene_ub6250_probe() reads the card type with ene_get_card_type() right
after usb_stor_probe2() has added the SCSI host.  The query is a
bulk-only transaction (CBW, one-byte data-in, CSW) sent through
ene_send_scsi_cmd(), which uses usb_stor_bulk_transfer_buf() and
therefore waits for URB completion without any timeout.  That is safe
only for transfers belonging to a SCSI command, where the command's
timeout and abort handling terminate the wait.  A card-type query has
no SCSI command behind it, so a device which enumerates but never
services bulk transfers blocks the probe forever.

commit 445fc368c6bc ("usb-storage: ene_ub6250: fix race between scan
work and probe") added us->dev_mutex around the query to keep it from
racing with the scan work for us->current_urb.  usb_stor_probe2()
queues that work with a one-second delay, so it now waits for a
transfer which may never finish.  The query itself has been unbounded
since commit 41e568d14ec0 ("Staging: Merge ENE UB6250 SD card codes
from keucr to drivers/usb/storage"), but only the latter turns the
hang into the lockup reported here, hence the Fixes tag.
syzbot reports:

 INFO: task kworker/0:2:53 blocked for more than 143 seconds.
 ...
 __mutex_lock
 usb_stor_scan_dwork
 ...
 INFO: task syz-executor339:5675 blocked for more than 143 seconds.
 ...
 device_lock
 usbdev_open

with the hub worker holding both locks stalled in
ene_ub6250_probe() -> ene_get_card_type() -> usb_stor_msg_common().

Run the query from ->initFunction instead.  usb_stor_acquire_resources()
calls it before the control thread is started and before the scan work
is queued, so nothing else can be using us->current_urb and the
dev_mutex from that commit can go away.  A failed query now unwinds
through usb_stor_probe2() before the SCSI host is registered, instead
of going through usb_stor_disconnect() and cancel_delayed_work_sync()
on a scan work which is itself stuck on the dead device.

Also bound the whole query with a 10-second budget covering the CBW,
the data stage and the CSW.  Per-transfer timeouts would not be
enough, since a device can stretch the query over the data stage and
the two CSW retries; each transfer gets the time remaining in the
budget.

Reported-by: syzbot+30552b4cbe99d6d91306@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=30552b4cbe99d6d91306
Fixes: 445fc368c6bc ("usb-storage: ene_ub6250: fix race between scan work and probe")
Cc: Liu Qi <liuqi@longcheer.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn>
---

v4:
 - Run the query from ->initFunction and drop the dev_mutex added by
   445fc368c6bc.  usb_stor_acquire_resources() calls it before the control
   thread is started and before the scan work is queued, so nothing else
   can be using us->current_urb, and a failed query unwinds before the
   SCSI host and the scan work exist.  The previous failure path went
   through usb_stor_disconnect() and cancel_delayed_work_sync() on a scan
   work stuck on the dead device.
 - Replace the 30-second per-transfer timeout with a single 10-second
   budget for the whole CBW/data/CSW sequence, including the CSW retries.
   Each transfer gets the time remaining in the budget, computed from one
   jiffies sample per step; the start time and the budget stay separate
   so that a wrapped deadline cannot be mistaken for the zero "no limit"
   value.
 - Return -EIO instead of -ENODEV when the query fails, like the other
   usb-storage initFunctions.  -ENODEV is only logged at dev_dbg as a
   rejected match, which would hide a real transfer failure at the
   default log level.
 - Keep Fixes: 445fc368c6bc, and spell out that the unbounded query
   predates it (41e568d14ec0); 445fc368c6bc is what turns the hang into
   the lockup reported here.
 - Fold the coding-style cleanup into this patch: the timeout code was
   rewritten and is checkpatch clean, so the separate 2/2 style patch is
   dropped.

v3: https://lore.kernel.org/r/20261001123135.38831-1-xy521521@gmail.com/
v2: https://lore.kernel.org/r/20261001091523.16934-1-xy521521@gmail.com/
v1: https://lore.kernel.org/r/20260929144640.2028-1-xy521521@gmail.com/
---
 drivers/usb/storage/ene_ub6250.c         | 153 +++++++++++++++++------
 drivers/usb/storage/transport.c          |  31 +++--
 drivers/usb/storage/transport.h          |   5 +
 drivers/usb/storage/unusual_ene_ub6250.h |   2 +-
 4 files changed, 143 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index fea2614e767c7..2b4fab31e900d 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -24,6 +24,15 @@
 
 #define DRV_NAME "ums_eneub6250"
 
+/*
+ * Time budget for the card-type query done while probing, in jiffies.  It
+ * is a bulk transfer with no SCSI command behind it, so nothing else would
+ * terminate the wait if the device stopped responding.
+ */
+#define ENE_PROBE_TIMEOUT (10 * HZ)
+
+static int ene_ub6250_init(struct us_data *us);
+
 MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
 MODULE_LICENSE("GPL");
 MODULE_IMPORT_NS("USB_STORAGE");
@@ -487,39 +496,76 @@ static void ene_ub6250_info_destructor(void *extra)
 	kfree(info->bbuf);
 }
 
-static int ene_send_scsi_cmd(struct us_data *us, u8 fDir, void *buf, int use_sg)
+/*
+ * Return what is left of a budget of @timeout jiffies which started at
+ * @start, in jiffies, or -ETIMEDOUT if the budget has been used up.  A zero
+ * @timeout means no limit.
+ */
+static int ene_time_left(unsigned long start, unsigned int timeout)
+{
+	unsigned long elapsed;
+
+	if (!timeout)
+		return 0;
+
+	elapsed = jiffies - start;
+	if (elapsed >= timeout)
+		return -ETIMEDOUT;
+
+	return min_t(unsigned long, timeout - elapsed, INT_MAX);
+}
+
+/*
+ * Send an ENE SCSI command.  @timeout is the time budget for the whole
+ * transaction (CBW, data and CSW), in jiffies; zero means no limit.
+ */
+static int ene_send_scsi_cmd_timeout(struct us_data *us, u8 fdir, void *buf,
+				     int use_sg, unsigned int timeout)
 {
 	struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) us->iobuf;
 	struct bulk_cs_wrap *bcs = (struct bulk_cs_wrap *) us->iobuf;
-
-	int result;
+	unsigned long start = jiffies;
+	int result, left;
 	unsigned int residue;
 	unsigned int cswlen = 0, partial = 0;
 	unsigned int transfer_length = bcb->DataTransferLength;
 
+	left = ene_time_left(start, timeout);
+	if (left < 0)
+		return USB_STOR_TRANSPORT_ERROR;
+
 	/* usb_stor_dbg(us, "transport --- ene_send_scsi_cmd\n"); */
 	/* send cmd to out endpoint */
-	result = usb_stor_bulk_transfer_buf(us, us->send_bulk_pipe,
-					    bcb, US_BULK_CB_WRAP_LEN, NULL);
+	result = usb_stor_bulk_transfer_buf_timeout(us, us->send_bulk_pipe,
+						    bcb, US_BULK_CB_WRAP_LEN,
+						    NULL, left);
 	if (result != USB_STOR_XFER_GOOD) {
 		usb_stor_dbg(us, "send cmd to out endpoint fail ---\n");
 		return USB_STOR_TRANSPORT_ERROR;
 	}
 
 	if (buf) {
-		unsigned int pipe = fDir;
+		unsigned int pipe = fdir;
 
-		if (fDir  == FDIR_READ)
+		if (fdir == FDIR_READ)
 			pipe = us->recv_bulk_pipe;
 		else
 			pipe = us->send_bulk_pipe;
 
 		/* Bulk */
 		if (use_sg) {
+			/*
+			 * Scatter-gather transfers belong to SCSI commands,
+			 * whose timeout and abort machinery bound the wait.
+			 */
 			result = usb_stor_bulk_srb(us, pipe, us->srb);
 		} else {
-			result = usb_stor_bulk_transfer_sg(us, pipe, buf,
-						transfer_length, 0, &partial);
+			left = ene_time_left(start, timeout);
+			if (left < 0)
+				return USB_STOR_TRANSPORT_ERROR;
+			result = usb_stor_bulk_transfer_buf_timeout(us, pipe,
+								    buf, transfer_length, &partial,
+								    left);
 		}
 		if (result != USB_STOR_XFER_GOOD) {
 			usb_stor_dbg(us, "data transfer fail ---\n");
@@ -527,21 +573,36 @@ static int ene_send_scsi_cmd(struct us_data *us, u8 fDir, void *buf, int use_sg)
 		}
 	}
 
+	left = ene_time_left(start, timeout);
+	if (left < 0)
+		return USB_STOR_TRANSPORT_ERROR;
+
 	/* Get CSW for device status */
-	result = usb_stor_bulk_transfer_buf(us, us->recv_bulk_pipe, bcs,
-					    US_BULK_CS_WRAP_LEN, &cswlen);
+	result = usb_stor_bulk_transfer_buf_timeout(us, us->recv_bulk_pipe,
+						    bcs, US_BULK_CS_WRAP_LEN,
+						    &cswlen, left);
 
 	if (result == USB_STOR_XFER_SHORT && cswlen == 0) {
 		usb_stor_dbg(us, "Received 0-length CSW; retrying...\n");
-		result = usb_stor_bulk_transfer_buf(us, us->recv_bulk_pipe,
-					    bcs, US_BULK_CS_WRAP_LEN, &cswlen);
+		left = ene_time_left(start, timeout);
+		if (left < 0)
+			return USB_STOR_TRANSPORT_ERROR;
+		result = usb_stor_bulk_transfer_buf_timeout(us,
+							    us->recv_bulk_pipe, bcs,
+							    US_BULK_CS_WRAP_LEN, &cswlen,
+							    left);
 	}
 
 	if (result == USB_STOR_XFER_STALLED) {
 		/* get the status again */
 		usb_stor_dbg(us, "Attempting to get CSW (2nd try)...\n");
-		result = usb_stor_bulk_transfer_buf(us, us->recv_bulk_pipe,
-						bcs, US_BULK_CS_WRAP_LEN, NULL);
+		left = ene_time_left(start, timeout);
+		if (left < 0)
+			return USB_STOR_TRANSPORT_ERROR;
+		result = usb_stor_bulk_transfer_buf_timeout(us,
+							    us->recv_bulk_pipe, bcs,
+							    US_BULK_CS_WRAP_LEN, NULL,
+							    left);
 	}
 
 	if (result != USB_STOR_XFER_GOOD)
@@ -567,6 +628,15 @@ static int ene_send_scsi_cmd(struct us_data *us, u8 fDir, void *buf, int use_sg)
 	return USB_STOR_TRANSPORT_GOOD;
 }
 
+/*
+ * Unbounded variant for the command path: a SCSI command's timeout and
+ * abort machinery terminate the transfer if the device stops responding.
+ */
+static int ene_send_scsi_cmd(struct us_data *us, u8 fdir, void *buf, int use_sg)
+{
+	return ene_send_scsi_cmd_timeout(us, fdir, buf, use_sg, 0);
+}
+
 static int do_scsi_request_sense(struct us_data *us, struct scsi_cmnd *srb)
 {
 	struct ene_ub6250_info *info = (struct ene_ub6250_info *) us->extra;
@@ -1824,7 +1894,8 @@ static int ms_scsi_write(struct us_data *us, struct scsi_cmnd *srb)
  * ENE MS Card
  */
 
-static int ene_get_card_type(struct us_data *us, u16 index, void *buf)
+static int ene_get_card_type(struct us_data *us, u16 index, void *buf,
+			     unsigned int timeout)
 {
 	struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) us->iobuf;
 	int result;
@@ -1837,7 +1908,7 @@ static int ene_get_card_type(struct us_data *us, u16 index, void *buf)
 	bcb->CDB[2]			= (unsigned char)(index>>8);
 	bcb->CDB[3]			= (unsigned char)index;
 
-	result = ene_send_scsi_cmd(us, FDIR_READ, buf, 0);
+	result = ene_send_scsi_cmd_timeout(us, FDIR_READ, buf, 0, timeout);
 	return result;
 }
 
@@ -2191,7 +2262,7 @@ static int ene_init(struct us_data *us)
 	struct ene_ub6250_info *info = (struct ene_ub6250_info *)(us->extra);
 	u8 *bbuf = info->bbuf;
 
-	result = ene_get_card_type(us, REG_CARD_STATUS, bbuf);
+	result = ene_get_card_type(us, REG_CARD_STATUS, bbuf, 0);
 	if (result != USB_STOR_XFER_GOOD)
 		return USB_STOR_TRANSPORT_ERROR;
 
@@ -2319,13 +2390,34 @@ static int ene_transport(struct scsi_cmnd *srb, struct us_data *us)
 
 static struct scsi_host_template ene_ub6250_host_template;
 
+/*
+ * Probe-time card-type query.  It runs from usb_stor_acquire_resources(),
+ * before the usb-storage control thread and the SCSI scan work exist, so
+ * nothing else can be using us->current_urb.  Bound it explicitly: there
+ * is no SCSI command whose timeout or abort handling could end the wait.
+ */
+static int ene_ub6250_init(struct us_data *us)
+{
+	struct ene_ub6250_info *info = (struct ene_ub6250_info *)us->extra;
+	int result;
+
+	result = ene_get_card_type(us, REG_CARD_STATUS, info->bbuf,
+				   ENE_PROBE_TIMEOUT);
+	if (result != USB_STOR_XFER_GOOD)
+		return -EIO;
+
+	if (!(info->bbuf[0] & 0x01))
+		pr_info("ums_eneub6250: SM cards are not supported\n");
+
+	return 0;
+}
+
 static int ene_ub6250_probe(struct usb_interface *intf,
 			 const struct usb_device_id *id)
 {
-	int result;
-	u8  misc_reg03;
 	struct us_data *us;
 	struct ene_ub6250_info *info;
+	int result;
 
 	result = usb_stor_probe1(&us, intf, id,
 		   (id - ene_ub6250_usb_ids) + ene_ub6250_unusual_dev_list,
@@ -2352,26 +2444,7 @@ static int ene_ub6250_probe(struct usb_interface *intf,
 	us->transport_reset = usb_stor_Bulk_reset;
 	us->max_lun = 0;
 
-	result = usb_stor_probe2(us);
-	if (result)
-		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;
-	}
-
-	misc_reg03 = info->bbuf[0];
-	if (!(misc_reg03 & 0x01)) {
-		pr_info("ums_eneub6250: This driver only supports SD/MS cards. "
-			"It does not support SM cards.\n");
-	}
-
-	return result;
+	return usb_stor_probe2(us);
 }
 
 
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 8de3015bce050..424293658918a 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -378,12 +378,17 @@ static int usb_stor_intr_transfer(struct us_data *us, void *buf,
 }
 
 /*
- * Transfer one buffer via bulk pipe, without timeouts, but allowing early
- * termination.  Return codes are USB_STOR_XFER_xxx.  If the bulk pipe
- * stalls during the transfer, the halt is automatically cleared.
+ * Transfer one buffer via bulk pipe, allowing early termination.  Return
+ * codes are USB_STOR_XFER_xxx.  If the bulk pipe stalls during the
+ * transfer, the halt is automatically cleared.
+ *
+ * A nonzero @timeout bounds the wait for URB completion, in jiffies.  It
+ * must only be used for transfers which are not part of a SCSI command:
+ * those rely on the command's timeout and abort handling.
  */
-int usb_stor_bulk_transfer_buf(struct us_data *us, unsigned int pipe,
-	void *buf, unsigned int length, unsigned int *act_len)
+int usb_stor_bulk_transfer_buf_timeout(struct us_data *us, unsigned int pipe,
+				       void *buf, unsigned int length,
+				       unsigned int *act_len, int timeout)
 {
 	int result;
 
@@ -392,14 +397,26 @@ int usb_stor_bulk_transfer_buf(struct us_data *us, unsigned int pipe,
 	/* fill and submit the URB */
 	usb_fill_bulk_urb(us->current_urb, us->pusb_dev, pipe, buf, length,
 		      usb_stor_blocking_completion, NULL);
-	result = usb_stor_msg_common(us, 0);
+	result = usb_stor_msg_common(us, timeout);
 
 	/* store the actual length of the data transferred */
 	if (act_len)
 		*act_len = us->current_urb->actual_length;
-	return interpret_urb_result(us, pipe, length, result, 
+	return interpret_urb_result(us, pipe, length, result,
 			us->current_urb->actual_length);
 }
+EXPORT_SYMBOL_GPL(usb_stor_bulk_transfer_buf_timeout);
+
+/*
+ * Same as usb_stor_bulk_transfer_buf_timeout() with an unbounded wait.
+ */
+int usb_stor_bulk_transfer_buf(struct us_data *us, unsigned int pipe,
+			       void *buf, unsigned int length,
+			       unsigned int *act_len)
+{
+	return usb_stor_bulk_transfer_buf_timeout(us, pipe, buf, length,
+			act_len, 0);
+}
 EXPORT_SYMBOL_GPL(usb_stor_bulk_transfer_buf);
 
 /*
diff --git a/drivers/usb/storage/transport.h b/drivers/usb/storage/transport.h
index 74ffd0d7e7b6c..361dbd77f15e3 100644
--- a/drivers/usb/storage/transport.h
+++ b/drivers/usb/storage/transport.h
@@ -77,6 +77,11 @@ extern int usb_stor_clear_halt(struct us_data *us, unsigned int pipe);
 extern int usb_stor_ctrl_transfer(struct us_data *us, unsigned int pipe,
 		u8 request, u8 requesttype, u16 value, u16 index,
 		void *data, u16 size);
+extern int usb_stor_bulk_transfer_buf_timeout(struct us_data *us,
+					      unsigned int pipe, void *buf,
+					      unsigned int length,
+					      unsigned int *act_len,
+					      int timeout);
 extern int usb_stor_bulk_transfer_buf(struct us_data *us, unsigned int pipe,
 		void *buf, unsigned int length, unsigned int *act_len);
 extern int usb_stor_bulk_transfer_sg(struct us_data *us, unsigned int pipe,
diff --git a/drivers/usb/storage/unusual_ene_ub6250.h b/drivers/usb/storage/unusual_ene_ub6250.h
index a3b32abc2b2ff..bc9d7ea4e398b 100644
--- a/drivers/usb/storage/unusual_ene_ub6250.h
+++ b/drivers/usb/storage/unusual_ene_ub6250.h
@@ -5,6 +5,6 @@
 UNUSUAL_DEV(0x0cf2, 0x6250, 0x0000, 0x9999,
 		"ENE",
 		"ENE UB6250 reader",
-		USB_SC_DEVICE, USB_PR_DEVICE, NULL, 0),
+		USB_SC_DEVICE, USB_PR_DEVICE, ene_ub6250_init, 0),
 
 #endif /* defined(CONFIG_USB_STORAGE_ENE_UB6250) || ... */
-- 
2.25.1

Re: [PATCH v4] usb-storage: ene_ub6250: don't let the card-type probe hang forever

[sashiko-bot]

> ene_ub6250_probe() reads the card type with ene_get_card_type() right
> after usb_stor_probe2() has added the SCSI host.  The query is a
> bulk-only transaction (CBW, one-byte data-in, CSW) sent through
> ene_send_scsi_cmd(), which uses usb_stor_bulk_transfer_buf() and
> therefore waits for URB completion without any timeout.  That is safe
> only for transfers belonging to a SCSI command, where the command's
> timeout and abort handling terminate the wait.  A card-type query has
> no SCSI command behind it, so a device which enumerates but never
> services bulk transfers blocks the probe forever.
> 
> commit 445fc368c6bc ("usb-storage: ene_ub6250: fix race between scan
> work and probe") added us->dev_mutex around the query to keep it from
> racing with the scan work for us->current_urb.  usb_stor_probe2()
> queues that work with a one-second delay, so it now waits for a
> transfer which may never finish.  The query itself has been unbounded
> [ ... ]
> Closes: https://syzkaller.appspot.com/bug?extid=30552b4cbe99d6d91306
> Fixes: 445fc368c6bc ("usb-storage: ene_ub6250: fix race between scan work and probe")
> Cc: Liu Qi <liuqi@longcheer.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109085449.83117-1-xiehongyu1@kylinos.cn?part=1

Re: [usb-storage] [PATCH v4] usb-storage: ene_ub6250: don't let the card-type probe hang forever

[Alan Stern]

On Mon, Nov 09, 2026 at 04:54:49PM +0800, Hongyu Xie wrote:
> ene_ub6250_probe() reads the card type with ene_get_card_type() right
> after usb_stor_probe2() has added the SCSI host.  The query is a
> bulk-only transaction (CBW, one-byte data-in, CSW) sent through
> ene_send_scsi_cmd(), which uses usb_stor_bulk_transfer_buf() and
> therefore waits for URB completion without any timeout.  That is safe
> only for transfers belonging to a SCSI command, where the command's
> timeout and abort handling terminate the wait.  A card-type query has
> no SCSI command behind it, so a device which enumerates but never
> services bulk transfers blocks the probe forever.
> 
> commit 445fc368c6bc ("usb-storage: ene_ub6250: fix race between scan
> work and probe") added us->dev_mutex around the query to keep it from
> racing with the scan work for us->current_urb.  usb_stor_probe2()
> queues that work with a one-second delay, so it now waits for a
> transfer which may never finish.  The query itself has been unbounded
> since commit 41e568d14ec0 ("Staging: Merge ENE UB6250 SD card codes
> from keucr to drivers/usb/storage"), but only the latter turns the
> hang into the lockup reported here, hence the Fixes tag.
> syzbot reports:
> 
>  INFO: task kworker/0:2:53 blocked for more than 143 seconds.
>  ...
>  __mutex_lock
>  usb_stor_scan_dwork
>  ...
>  INFO: task syz-executor339:5675 blocked for more than 143 seconds.
>  ...
>  device_lock
>  usbdev_open
> 
> with the hub worker holding both locks stalled in
> ene_ub6250_probe() -> ene_get_card_type() -> usb_stor_msg_common().
> 
> Run the query from ->initFunction instead.  usb_stor_acquire_resources()
> calls it before the control thread is started and before the scan work
> is queued, so nothing else can be using us->current_urb and the
> dev_mutex from that commit can go away.  A failed query now unwinds
> through usb_stor_probe2() before the SCSI host is registered, instead
> of going through usb_stor_disconnect() and cancel_delayed_work_sync()
> on a scan work which is itself stuck on the dead device.

Makes sense.

> Also bound the whole query with a 10-second budget covering the CBW,
> the data stage and the CSW.  Per-transfer timeouts would not be
> enough, since a device can stretch the query over the data stage and
> the two CSW retries; each transfer gets the time remaining in the
> budget.

I don't think this matters.  A timeout at any stage should cause the 
entire transfer to fail immediately.  As for the possibility of slow 
stages completing just before the timeout expires...  How often do you 
expect that case to come up?  Does we care if the initialization 
sometimes is a little slow, so long as the total time is still limited?

However, it's not a big deal one way or the other.

> diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
> index fea2614e767c7..2b4fab31e900d 100644
> --- a/drivers/usb/storage/ene_ub6250.c
> +++ b/drivers/usb/storage/ene_ub6250.c
> @@ -24,6 +24,15 @@
>  
>  #define DRV_NAME "ums_eneub6250"
>  
> +/*
> + * Time budget for the card-type query done while probing, in jiffies.  It
> + * is a bulk transfer with no SCSI command behind it, so nothing else would
> + * terminate the wait if the device stopped responding.
> + */

Detailed explanation not needed here; you explain the reasoning in 
ene_ub6250_init() below.

> +#define ENE_PROBE_TIMEOUT (10 * HZ)
> +
> +static int ene_ub6250_init(struct us_data *us);

Unnecessary forward declaration.

> +
>  MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
>  MODULE_LICENSE("GPL");
>  MODULE_IMPORT_NS("USB_STORAGE");
> @@ -487,39 +496,76 @@ static void ene_ub6250_info_destructor(void *extra)
>  	kfree(info->bbuf);
>  }
>  
> -static int ene_send_scsi_cmd(struct us_data *us, u8 fDir, void *buf, int use_sg)
> +/*
> + * Return what is left of a budget of @timeout jiffies which started at
> + * @start, in jiffies, or -ETIMEDOUT if the budget has been used up.  A zero
> + * @timeout means no limit.
> + */
> +static int ene_time_left(unsigned long start, unsigned int timeout)
> +{
> +	unsigned long elapsed;
> +
> +	if (!timeout)
> +		return 0;
> +
> +	elapsed = jiffies - start;
> +	if (elapsed >= timeout)
> +		return -ETIMEDOUT;
> +
> +	return min_t(unsigned long, timeout - elapsed, INT_MAX);

You're mixing unsigned long and unsigned int in a confusing way.  Why 
can't elapsed be an unsigned int?  You don't expect ever to encounter 
delays longer than 4 billion jiffies, do you?

Also, if you don't care about an overall timeout, this routine isn't 
needed.

> +}
> +
> +/*
> + * Send an ENE SCSI command.  @timeout is the time budget for the whole
> + * transaction (CBW, data and CSW), in jiffies; zero means no limit.
> + */
> +static int ene_send_scsi_cmd_timeout(struct us_data *us, u8 fdir, void *buf,
> +				     int use_sg, unsigned int timeout)
>  {
>  	struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) us->iobuf;
>  	struct bulk_cs_wrap *bcs = (struct bulk_cs_wrap *) us->iobuf;
> -
> -	int result;
> +	unsigned long start = jiffies;
> +	int result, left;
>  	unsigned int residue;
>  	unsigned int cswlen = 0, partial = 0;
>  	unsigned int transfer_length = bcb->DataTransferLength;
>  
> +	left = ene_time_left(start, timeout);
> +	if (left < 0)
> +		return USB_STOR_TRANSPORT_ERROR;

What point is there in checking for a timeout before anything has 
happened?

> +
>  	/* usb_stor_dbg(us, "transport --- ene_send_scsi_cmd\n"); */
>  	/* send cmd to out endpoint */
> -	result = usb_stor_bulk_transfer_buf(us, us->send_bulk_pipe,
> -					    bcb, US_BULK_CB_WRAP_LEN, NULL);
> +	result = usb_stor_bulk_transfer_buf_timeout(us, us->send_bulk_pipe,
> +						    bcb, US_BULK_CB_WRAP_LEN,
> +						    NULL, left);
>  	if (result != USB_STOR_XFER_GOOD) {
>  		usb_stor_dbg(us, "send cmd to out endpoint fail ---\n");
>  		return USB_STOR_TRANSPORT_ERROR;
>  	}
>  
>  	if (buf) {
> -		unsigned int pipe = fDir;
> +		unsigned int pipe = fdir;

Unnecessary initialization.

>  
> -		if (fDir  == FDIR_READ)
> +		if (fdir == FDIR_READ)
>  			pipe = us->recv_bulk_pipe;
>  		else
>  			pipe = us->send_bulk_pipe;
>  
>  		/* Bulk */
>  		if (use_sg) {
> +			/*
> +			 * Scatter-gather transfers belong to SCSI commands,
> +			 * whose timeout and abort machinery bound the wait.
> +			 */
>  			result = usb_stor_bulk_srb(us, pipe, us->srb);
>  		} else {
> -			result = usb_stor_bulk_transfer_sg(us, pipe, buf,
> -						transfer_length, 0, &partial);
> +			left = ene_time_left(start, timeout);
> +			if (left < 0)
> +				return USB_STOR_TRANSPORT_ERROR;
> +			result = usb_stor_bulk_transfer_buf_timeout(us, pipe,
> +								    buf, transfer_length, &partial,

Things like this are why I maintain that the policy of aligning 
continuation lines with the preceding open paren is a mistake.  Grumble.

The rest of the material looks pretty much okay.  However, I would 
prefer this to be made into a series of two patches, one with the 
modifications to transport.[ch] and the other with the modifications to 
ene_ub6250.

Alan Stern

[PATCH v5 0/2] usb-storage: bound the ene_ub6250 probe-time card-type query

[Hongyu Xie]

This is v5 of the fix for the ene_ub6250 card-type probe hang.  Changes
since v4, following Alan's review:

 - Split the series: the transport helper comes first, the driver change
   second.
 - Drop the shared 10-second budget.  Each transfer now gets the full
   timeout; a timeout at any stage fails the query immediately and the
   query is bounded to about 50 seconds at most (five transfers),
   without the remaining-time bookkeeping.
 - Shorten the comment above ENE_PROBE_TIMEOUT, as the reasoning is in
   ene_ub6250_init().
 - Drop the unused initializer of the bulk pipe variable.

The transport helper is recorded as a stable prerequisite, so the
dependency is available when the fix is backported.

The forward declaration of ene_ub6250_init() is still needed: the
unusual-device table is generated by including unusual_ene_ub6250.h
before the function definition and takes its address, so removing the
declaration breaks the build ("'ene_ub6250_init' undeclared here").
realtek_cr.c uses the same declaration for init_realtek_cr().

Testing: with the syzbot C reproducer, the unpatched kernel blocks in
the probe for more than 143 seconds and the shutdown hangs; with the fix
the probe fails after about 10 seconds with -EIO, before the SCSI host
and the scan work exist, and the system shuts down cleanly.

Hongyu Xie (2):
  usb-storage: add usb_stor_bulk_transfer_buf_timeout()
  usb-storage: ene_ub6250: don't let the card-type probe hang forever

 drivers/usb/storage/ene_ub6250.c         | 110 +++++++++++++++--------
 drivers/usb/storage/transport.c          |  31 +++++--
 drivers/usb/storage/transport.h          |   5 ++
 drivers/usb/storage/unusual_ene_ub6250.h |   2 +-
 4 files changed, 102 insertions(+), 46 deletions(-)

-- 
2.25.1

[PATCH v5 1/2] usb-storage: add usb_stor_bulk_transfer_buf_timeout()

[Hongyu Xie]

usb_stor_bulk_transfer_buf() waits for URB completion without a timeout.
That is correct for transfers belonging to a SCSI command, whose timeout
and abort handling terminate the wait, but not for transfers issued
outside a SCSI command.

Add a variant which takes a timeout, in jiffies; zero means no limit.
usb_stor_bulk_transfer_buf() becomes a wrapper with a zero timeout, so
existing callers are unchanged.

Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn>
---
 drivers/usb/storage/transport.c | 31 ++++++++++++++++++++++++-------
 drivers/usb/storage/transport.h |  5 +++++
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 8de3015bce050..424293658918a 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -378,12 +378,17 @@ static int usb_stor_intr_transfer(struct us_data *us, void *buf,
 }
 
 /*
- * Transfer one buffer via bulk pipe, without timeouts, but allowing early
- * termination.  Return codes are USB_STOR_XFER_xxx.  If the bulk pipe
- * stalls during the transfer, the halt is automatically cleared.
+ * Transfer one buffer via bulk pipe, allowing early termination.  Return
+ * codes are USB_STOR_XFER_xxx.  If the bulk pipe stalls during the
+ * transfer, the halt is automatically cleared.
+ *
+ * A nonzero @timeout bounds the wait for URB completion, in jiffies.  It
+ * must only be used for transfers which are not part of a SCSI command:
+ * those rely on the command's timeout and abort handling.
  */
-int usb_stor_bulk_transfer_buf(struct us_data *us, unsigned int pipe,
-	void *buf, unsigned int length, unsigned int *act_len)
+int usb_stor_bulk_transfer_buf_timeout(struct us_data *us, unsigned int pipe,
+				       void *buf, unsigned int length,
+				       unsigned int *act_len, int timeout)
 {
 	int result;
 
@@ -392,14 +397,26 @@ int usb_stor_bulk_transfer_buf(struct us_data *us, unsigned int pipe,
 	/* fill and submit the URB */
 	usb_fill_bulk_urb(us->current_urb, us->pusb_dev, pipe, buf, length,
 		      usb_stor_blocking_completion, NULL);
-	result = usb_stor_msg_common(us, 0);
+	result = usb_stor_msg_common(us, timeout);
 
 	/* store the actual length of the data transferred */
 	if (act_len)
 		*act_len = us->current_urb->actual_length;
-	return interpret_urb_result(us, pipe, length, result, 
+	return interpret_urb_result(us, pipe, length, result,
 			us->current_urb->actual_length);
 }
+EXPORT_SYMBOL_GPL(usb_stor_bulk_transfer_buf_timeout);
+
+/*
+ * Same as usb_stor_bulk_transfer_buf_timeout() with an unbounded wait.
+ */
+int usb_stor_bulk_transfer_buf(struct us_data *us, unsigned int pipe,
+			       void *buf, unsigned int length,
+			       unsigned int *act_len)
+{
+	return usb_stor_bulk_transfer_buf_timeout(us, pipe, buf, length,
+			act_len, 0);
+}
 EXPORT_SYMBOL_GPL(usb_stor_bulk_transfer_buf);
 
 /*
diff --git a/drivers/usb/storage/transport.h b/drivers/usb/storage/transport.h
index 74ffd0d7e7b6c..361dbd77f15e3 100644
--- a/drivers/usb/storage/transport.h
+++ b/drivers/usb/storage/transport.h
@@ -77,6 +77,11 @@ extern int usb_stor_clear_halt(struct us_data *us, unsigned int pipe);
 extern int usb_stor_ctrl_transfer(struct us_data *us, unsigned int pipe,
 		u8 request, u8 requesttype, u16 value, u16 index,
 		void *data, u16 size);
+extern int usb_stor_bulk_transfer_buf_timeout(struct us_data *us,
+					      unsigned int pipe, void *buf,
+					      unsigned int length,
+					      unsigned int *act_len,
+					      int timeout);
 extern int usb_stor_bulk_transfer_buf(struct us_data *us, unsigned int pipe,
 		void *buf, unsigned int length, unsigned int *act_len);
 extern int usb_stor_bulk_transfer_sg(struct us_data *us, unsigned int pipe,
-- 
2.25.1

[PATCH v5 2/2] usb-storage: ene_ub6250: don't let the card-type probe hang forever

[Hongyu Xie]

ene_ub6250_probe() reads the card type with ene_get_card_type() right
after usb_stor_probe2() has added the SCSI host.  The query is a
bulk-only transaction (CBW, one-byte data-in, CSW) sent through
ene_send_scsi_cmd(), which uses usb_stor_bulk_transfer_buf() and
therefore waits for URB completion without any timeout.  That is safe
only for transfers belonging to a SCSI command, where the command's
timeout and abort handling terminate the wait.  A card-type query has
no SCSI command behind it, so a device which enumerates but never
services bulk transfers blocks the probe forever.

commit 445fc368c6bc ("usb-storage: ene_ub6250: fix race between scan
work and probe") added us->dev_mutex around the query to keep it from
racing with the scan work for us->current_urb.  usb_stor_probe2()
queues that work with a one-second delay, so it now waits for a
transfer which may never finish.  The query itself has been unbounded
since commit 41e568d14ec0 ("Staging: Merge ENE UB6250 SD card codes
from keucr to drivers/usb/storage"), but only the latter turns the
hang into the lockup reported here, hence the Fixes tag.
syzbot reports:

 INFO: task kworker/0:2:53 blocked for more than 143 seconds.
 Workqueue: events_freezable usb_stor_scan_dwork
 Call Trace:
  __mutex_lock+0x7c4/0x15a0
  usb_stor_scan_dwork+0xec/0x2c0
 ...
 INFO: task syz-executor339:5675 blocked for more than 143 seconds.
 Call Trace:
  __mutex_lock+0x7c4/0x15a0
  device_lock
  usbdev_open+0x151/0x760

with the hub worker holding both locks stalled in
ene_ub6250_probe() -> ene_get_card_type() -> usb_stor_msg_common().

Run the query from ->initFunction instead.  usb_stor_acquire_resources()
calls it before the control thread is started and before the scan work
is queued, so nothing else can be using us->current_urb and the
dev_mutex from that commit can go away.  A failed query now unwinds
through usb_stor_probe2() before the SCSI host is registered, instead
of going through usb_stor_disconnect() and cancel_delayed_work_sync()
on a scan work which is itself stuck on the dead device.

Also give each transfer of the query a 10-second timeout.  A timeout
at any stage fails the query immediately, so the probe cannot block
forever.  The retry chain can run five transfers at most (CBW, data,
CSW and the two CSW retries), which bounds the query to about 50
seconds without keeping a shared budget.

Reported-by: syzbot+30552b4cbe99d6d91306@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=30552b4cbe99d6d91306
Fixes: 445fc368c6bc ("usb-storage: ene_ub6250: fix race between scan work and probe")
Cc: Liu Qi <liuqi@longcheer.com>
Cc: stable@vger.kernel.org # 7.3.x: e350e5b223e1: add usb_stor_bulk_transfer_buf_timeout()
Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn>

---

v5:
 - Split into a two-patch series: the transport helper first, the driver
   change second.
 - Replace the shared 10-second budget with a per-transfer timeout.  A
   timeout in any stage fails the query immediately and the total time
   stays bounded, so the remaining-time bookkeeping is not needed.
 - State the worst-case bound of that choice (about 50 seconds, at most
   five transfers) in the message; record the transport helper as a
   stable prerequisite so the dependency is available there as well.
 - Shorten the comment above ENE_PROBE_TIMEOUT, as the reasoning is in
   ene_ub6250_init().
 - Drop the unused initializer of the bulk pipe variable.
 - Keep the forward declaration of ene_ub6250_init(): the unusual-device
   table is built by including unusual_ene_ub6250.h before the function
   definition and takes its address, so removing the declaration fails to
   build; realtek_cr.c has the same declaration for init_realtek_cr().

v4: https://lore.kernel.org/all/20261109085449.83117-1-xiehongyu1@kylinos.cn/
v3: https://lore.kernel.org/r/20261001123135.38831-1-xy521521@gmail.com/
v2: https://lore.kernel.org/r/20261001091523.16934-1-xy521521@gmail.com/
v1: https://lore.kernel.org/r/20260929144640.2028-1-xy521521@gmail.com/
---
 drivers/usb/storage/ene_ub6250.c         | 110 +++++++++++++++--------
 drivers/usb/storage/unusual_ene_ub6250.h |   2 +-
 2 files changed, 73 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index fea2614e767c7..bf661d8e2c540 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -24,6 +24,9 @@
 
 #define DRV_NAME "ums_eneub6250"
 
+/* Timeout for each transfer of the probe-time card-type query, in jiffies. */
+#define ENE_PROBE_TIMEOUT (10 * HZ)
+
 MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
 MODULE_LICENSE("GPL");
 MODULE_IMPORT_NS("USB_STORAGE");
@@ -34,6 +37,8 @@ MODULE_FIRMWARE(MS_INIT_FIRMWARE);
 MODULE_FIRMWARE(MSP_RW_FIRMWARE);
 MODULE_FIRMWARE(MS_RW_FIRMWARE);
 
+static int ene_ub6250_init(struct us_data *us);
+
 /*
  * The table of devices
  */
@@ -487,7 +492,12 @@ static void ene_ub6250_info_destructor(void *extra)
 	kfree(info->bbuf);
 }
 
-static int ene_send_scsi_cmd(struct us_data *us, u8 fDir, void *buf, int use_sg)
+/*
+ * Send an ENE SCSI command.  @timeout bounds the wait for each transfer of
+ * the transaction, in jiffies; zero means no limit.
+ */
+static int ene_send_scsi_cmd_timeout(struct us_data *us, u8 fdir, void *buf,
+				     int use_sg, unsigned int timeout)
 {
 	struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) us->iobuf;
 	struct bulk_cs_wrap *bcs = (struct bulk_cs_wrap *) us->iobuf;
@@ -499,27 +509,33 @@ static int ene_send_scsi_cmd(struct us_data *us, u8 fDir, void *buf, int use_sg)
 
 	/* usb_stor_dbg(us, "transport --- ene_send_scsi_cmd\n"); */
 	/* send cmd to out endpoint */
-	result = usb_stor_bulk_transfer_buf(us, us->send_bulk_pipe,
-					    bcb, US_BULK_CB_WRAP_LEN, NULL);
+	result = usb_stor_bulk_transfer_buf_timeout(us, us->send_bulk_pipe,
+						    bcb, US_BULK_CB_WRAP_LEN,
+						    NULL, timeout);
 	if (result != USB_STOR_XFER_GOOD) {
 		usb_stor_dbg(us, "send cmd to out endpoint fail ---\n");
 		return USB_STOR_TRANSPORT_ERROR;
 	}
 
 	if (buf) {
-		unsigned int pipe = fDir;
+		unsigned int pipe;
 
-		if (fDir  == FDIR_READ)
+		if (fdir == FDIR_READ)
 			pipe = us->recv_bulk_pipe;
 		else
 			pipe = us->send_bulk_pipe;
 
 		/* Bulk */
 		if (use_sg) {
+			/*
+			 * Scatter-gather transfers belong to SCSI commands,
+			 * whose timeout and abort machinery bound the wait.
+			 */
 			result = usb_stor_bulk_srb(us, pipe, us->srb);
 		} else {
-			result = usb_stor_bulk_transfer_sg(us, pipe, buf,
-						transfer_length, 0, &partial);
+			result = usb_stor_bulk_transfer_buf_timeout(us, pipe,
+								    buf, transfer_length,
+								    &partial, timeout);
 		}
 		if (result != USB_STOR_XFER_GOOD) {
 			usb_stor_dbg(us, "data transfer fail ---\n");
@@ -528,20 +544,25 @@ static int ene_send_scsi_cmd(struct us_data *us, u8 fDir, void *buf, int use_sg)
 	}
 
 	/* Get CSW for device status */
-	result = usb_stor_bulk_transfer_buf(us, us->recv_bulk_pipe, bcs,
-					    US_BULK_CS_WRAP_LEN, &cswlen);
+	result = usb_stor_bulk_transfer_buf_timeout(us, us->recv_bulk_pipe,
+						    bcs, US_BULK_CS_WRAP_LEN,
+						    &cswlen, timeout);
 
 	if (result == USB_STOR_XFER_SHORT && cswlen == 0) {
 		usb_stor_dbg(us, "Received 0-length CSW; retrying...\n");
-		result = usb_stor_bulk_transfer_buf(us, us->recv_bulk_pipe,
-					    bcs, US_BULK_CS_WRAP_LEN, &cswlen);
+		result = usb_stor_bulk_transfer_buf_timeout(us,
+							    us->recv_bulk_pipe, bcs,
+							    US_BULK_CS_WRAP_LEN,
+							    &cswlen, timeout);
 	}
 
 	if (result == USB_STOR_XFER_STALLED) {
 		/* get the status again */
 		usb_stor_dbg(us, "Attempting to get CSW (2nd try)...\n");
-		result = usb_stor_bulk_transfer_buf(us, us->recv_bulk_pipe,
-						bcs, US_BULK_CS_WRAP_LEN, NULL);
+		result = usb_stor_bulk_transfer_buf_timeout(us,
+							    us->recv_bulk_pipe, bcs,
+							    US_BULK_CS_WRAP_LEN, NULL,
+							    timeout);
 	}
 
 	if (result != USB_STOR_XFER_GOOD)
@@ -567,6 +588,15 @@ static int ene_send_scsi_cmd(struct us_data *us, u8 fDir, void *buf, int use_sg)
 	return USB_STOR_TRANSPORT_GOOD;
 }
 
+/*
+ * Unbounded variant for the command path: a SCSI command's timeout and
+ * abort machinery terminate the transfer if the device stops responding.
+ */
+static int ene_send_scsi_cmd(struct us_data *us, u8 fdir, void *buf, int use_sg)
+{
+	return ene_send_scsi_cmd_timeout(us, fdir, buf, use_sg, 0);
+}
+
 static int do_scsi_request_sense(struct us_data *us, struct scsi_cmnd *srb)
 {
 	struct ene_ub6250_info *info = (struct ene_ub6250_info *) us->extra;
@@ -1824,7 +1854,8 @@ static int ms_scsi_write(struct us_data *us, struct scsi_cmnd *srb)
  * ENE MS Card
  */
 
-static int ene_get_card_type(struct us_data *us, u16 index, void *buf)
+static int ene_get_card_type(struct us_data *us, u16 index, void *buf,
+			     unsigned int timeout)
 {
 	struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) us->iobuf;
 	int result;
@@ -1837,7 +1868,7 @@ static int ene_get_card_type(struct us_data *us, u16 index, void *buf)
 	bcb->CDB[2]			= (unsigned char)(index>>8);
 	bcb->CDB[3]			= (unsigned char)index;
 
-	result = ene_send_scsi_cmd(us, FDIR_READ, buf, 0);
+	result = ene_send_scsi_cmd_timeout(us, FDIR_READ, buf, 0, timeout);
 	return result;
 }
 
@@ -2191,7 +2222,7 @@ static int ene_init(struct us_data *us)
 	struct ene_ub6250_info *info = (struct ene_ub6250_info *)(us->extra);
 	u8 *bbuf = info->bbuf;
 
-	result = ene_get_card_type(us, REG_CARD_STATUS, bbuf);
+	result = ene_get_card_type(us, REG_CARD_STATUS, bbuf, 0);
 	if (result != USB_STOR_XFER_GOOD)
 		return USB_STOR_TRANSPORT_ERROR;
 
@@ -2319,13 +2350,35 @@ static int ene_transport(struct scsi_cmnd *srb, struct us_data *us)
 
 static struct scsi_host_template ene_ub6250_host_template;
 
+/*
+ * Probe-time card-type query.  It runs from usb_stor_acquire_resources(),
+ * before the usb-storage control thread and the SCSI scan work exist, so
+ * nothing else can be using us->current_urb.  Give every transfer a
+ * timeout: there is no SCSI command whose timeout or abort handling could
+ * end the wait, and a timeout at any stage fails the whole query.
+ */
+static int ene_ub6250_init(struct us_data *us)
+{
+	struct ene_ub6250_info *info = (struct ene_ub6250_info *)us->extra;
+	int result;
+
+	result = ene_get_card_type(us, REG_CARD_STATUS, info->bbuf,
+				   ENE_PROBE_TIMEOUT);
+	if (result != USB_STOR_XFER_GOOD)
+		return -EIO;
+
+	if (!(info->bbuf[0] & 0x01))
+		pr_info("ums_eneub6250: SM cards are not supported\n");
+
+	return 0;
+}
+
 static int ene_ub6250_probe(struct usb_interface *intf,
 			 const struct usb_device_id *id)
 {
-	int result;
-	u8  misc_reg03;
 	struct us_data *us;
 	struct ene_ub6250_info *info;
+	int result;
 
 	result = usb_stor_probe1(&us, intf, id,
 		   (id - ene_ub6250_usb_ids) + ene_ub6250_unusual_dev_list,
@@ -2352,26 +2405,7 @@ static int ene_ub6250_probe(struct usb_interface *intf,
 	us->transport_reset = usb_stor_Bulk_reset;
 	us->max_lun = 0;
 
-	result = usb_stor_probe2(us);
-	if (result)
-		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;
-	}
-
-	misc_reg03 = info->bbuf[0];
-	if (!(misc_reg03 & 0x01)) {
-		pr_info("ums_eneub6250: This driver only supports SD/MS cards. "
-			"It does not support SM cards.\n");
-	}
-
-	return result;
+	return usb_stor_probe2(us);
 }
 
 
diff --git a/drivers/usb/storage/unusual_ene_ub6250.h b/drivers/usb/storage/unusual_ene_ub6250.h
index a3b32abc2b2ff..bc9d7ea4e398b 100644
--- a/drivers/usb/storage/unusual_ene_ub6250.h
+++ b/drivers/usb/storage/unusual_ene_ub6250.h
@@ -5,6 +5,6 @@
 UNUSUAL_DEV(0x0cf2, 0x6250, 0x0000, 0x9999,
 		"ENE",
 		"ENE UB6250 reader",
-		USB_SC_DEVICE, USB_PR_DEVICE, NULL, 0),
+		USB_SC_DEVICE, USB_PR_DEVICE, ene_ub6250_init, 0),
 
 #endif /* defined(CONFIG_USB_STORAGE_ENE_UB6250) || ... */
-- 
2.25.1

Re: [PATCH v5 1/2] usb-storage: add usb_stor_bulk_transfer_buf_timeout()

[sashiko-bot]

> usb_stor_bulk_transfer_buf() waits for URB completion without a timeout.
> That is correct for transfers belonging to a SCSI command, whose timeout
> and abort handling terminate the wait, but not for transfers issued
> outside a SCSI command.
> 
> Add a variant which takes a timeout, in jiffies; zero means no limit.
> usb_stor_bulk_transfer_buf() becomes a wrapper with a zero timeout, so
> existing callers are unchanged.
> 
> Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113073638.1225998-1-xiehongyu1@kylinos.cn?part=1

Re: [PATCH v5 2/2] usb-storage: ene_ub6250: don't let the card-type probe hang forever

[sashiko-bot]

> ene_ub6250_probe() reads the card type with ene_get_card_type() right
> after usb_stor_probe2() has added the SCSI host.  The query is a
> bulk-only transaction (CBW, one-byte data-in, CSW) sent through
> ene_send_scsi_cmd(), which uses usb_stor_bulk_transfer_buf() and
> therefore waits for URB completion without any timeout.  That is safe
> only for transfers belonging to a SCSI command, where the command's
> timeout and abort handling terminate the wait.  A card-type query has
> no SCSI command behind it, so a device which enumerates but never
> services bulk transfers blocks the probe forever.
> 
> commit 445fc368c6bc ("usb-storage: ene_ub6250: fix race between scan
> work and probe") added us->dev_mutex around the query to keep it from
> racing with the scan work for us->current_urb.  usb_stor_probe2()
> queues that work with a one-second delay, so it now waits for a
> transfer which may never finish.  The query itself has been unbounded
> [ ... ]
> Closes: https://syzkaller.appspot.com/bug?extid=30552b4cbe99d6d91306
> Fixes: 445fc368c6bc ("usb-storage: ene_ub6250: fix race between scan work and probe")
> Cc: Liu Qi <liuqi@longcheer.com>
> Cc: stable@vger.kernel.org # 7.3.x: e350e5b223e1: add usb_stor_bulk_transfer_buf_timeout()
> Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113073638.1225998-1-xiehongyu1@kylinos.cn?part=2