mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] usb-storage: option_ms: bound probe-time bulk transfers
@ 2026-10-13 13:20 Lee Yongjun
  2026-10-13 16:18 ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Lee Yongjun @ 2026-10-13 13:20 UTC (permalink / raw)
  To: stern, gregkh
  Cc: digidietze, xiehongyu1, linux-usb, usb-storage, linux-kernel,
	syzkaller-bugs, syzbot+7317196294d50d6e55c3, Lee Yongjun

The Option device initialization path issues bulk transfers
without a timeout. If a device does not complete a transfer,
the USB hub worker can remain blocked while holding the device
mutex, causing usb_device_read() to hang.

Use the timeout-aware bulk transfer helper for the transfers
in option_inquiry() and option_rezero(). This bounds the wait
for each probe-time transfer to 30 seconds.

Tested with the syzbot C reproducer under QEMU/KVM. The
patched kernel repeatedly completed USB device initialization
attempts after approximately 30 seconds without triggering
the previously observed hung-task warning.

This patch depends on Hongyu Xie's pending v3 series:
https://lore.kernel.org/r/20261001123135.38831-1-xy521521@gmail.com/

Fixes: 32ebbe7b6ad4 ("USB: usb-storage: add filter to "option_ms" to leave unrecognized devices alone")
Reported-by: syzbot+7317196294d50d6e55c3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7317196294d50d6e55c3
Assisted-by: LLM

Signed-off-by: Lee Yongjun <jun85566@gmail.com>
---
 drivers/usb/storage/option_ms.c | 38 +++++++++++++++++----------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/storage/option_ms.c b/drivers/usb/storage/option_ms.c
index 7c0b05a36554..1f3d984d6932 100644
--- a/drivers/usb/storage/option_ms.c
+++ b/drivers/usb/storage/option_ms.c
@@ -10,6 +10,7 @@
 #include <linux/usb.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/jiffies.h>
 
 #include "usb.h"
 #include "transport.h"
@@ -18,6 +19,7 @@
 
 #define ZCD_FORCE_MODEM			0x01
 #define ZCD_ALLOW_MS 			0x02
+#define OPTION_MS_PROBE_TIMEOUT		(30 * HZ)
 
 static unsigned int option_zero_cd = ZCD_FORCE_MODEM;
 module_param(option_zero_cd, uint, S_IRUGO | S_IWUSR);
@@ -44,9 +46,9 @@ static int option_rezero(struct us_data *us)
 		return USB_STOR_TRANSPORT_ERROR;
 
 	memcpy(buffer, rezero_msg, sizeof(rezero_msg));
-	result = usb_stor_bulk_transfer_buf(us,
-			us->send_bulk_pipe,
-			buffer, sizeof(rezero_msg), NULL);
+	result = usb_stor_bulk_transfer_buf_timeout(us, us->send_bulk_pipe,
+						    buffer, sizeof(rezero_msg), NULL,
+						    OPTION_MS_PROBE_TIMEOUT);
 	if (result != USB_STOR_XFER_GOOD) {
 		result = USB_STOR_XFER_ERROR;
 		goto out;
@@ -56,14 +58,14 @@ static int option_rezero(struct us_data *us)
 	 * Some of the devices need to be asked for a response, but we don't
 	 * care what that response is.
 	 */
-	usb_stor_bulk_transfer_buf(us,
-			us->recv_bulk_pipe,
-			buffer, RESPONSE_LEN, NULL);
+	usb_stor_bulk_transfer_buf_timeout(us, us->recv_bulk_pipe,
+					   buffer, RESPONSE_LEN, NULL,
+					   OPTION_MS_PROBE_TIMEOUT);
 
 	/* Read the CSW */
-	usb_stor_bulk_transfer_buf(us,
-			us->recv_bulk_pipe,
-			buffer, 13, NULL);
+	usb_stor_bulk_transfer_buf_timeout(us, us->recv_bulk_pipe,
+					   buffer, 13, NULL,
+					   OPTION_MS_PROBE_TIMEOUT);
 
 	result = USB_STOR_XFER_GOOD;
 
@@ -90,17 +92,17 @@ static int option_inquiry(struct us_data *us)
 		return USB_STOR_TRANSPORT_ERROR;
 
 	memcpy(buffer, inquiry_msg, sizeof(inquiry_msg));
-	result = usb_stor_bulk_transfer_buf(us,
-			us->send_bulk_pipe,
-			buffer, sizeof(inquiry_msg), NULL);
+	result = usb_stor_bulk_transfer_buf_timeout(us, us->send_bulk_pipe,
+						    buffer, sizeof(inquiry_msg), NULL,
+						    OPTION_MS_PROBE_TIMEOUT);
 	if (result != USB_STOR_XFER_GOOD) {
 		result = USB_STOR_XFER_ERROR;
 		goto out;
 	}
 
-	result = usb_stor_bulk_transfer_buf(us,
-			us->recv_bulk_pipe,
-			buffer, 0x24, NULL);
+	result = usb_stor_bulk_transfer_buf_timeout(us, us->recv_bulk_pipe,
+						    buffer, 0x24, NULL,
+						    OPTION_MS_PROBE_TIMEOUT);
 	if (result != USB_STOR_XFER_GOOD) {
 		result = USB_STOR_XFER_ERROR;
 		goto out;
@@ -112,9 +114,9 @@ static int option_inquiry(struct us_data *us)
 		result = memcmp(buffer+8, "ZCOPTION", 8);
 
 	/* Read the CSW */
-	usb_stor_bulk_transfer_buf(us,
-			us->recv_bulk_pipe,
-			buffer, 13, NULL);
+	usb_stor_bulk_transfer_buf_timeout(us, us->recv_bulk_pipe,
+					   buffer, 13, NULL,
+					   OPTION_MS_PROBE_TIMEOUT);
 
 out:
 	kfree(buffer);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb-storage: option_ms: bound probe-time bulk transfers
  2026-10-13 13:20 [PATCH] usb-storage: option_ms: bound probe-time bulk transfers Lee Yongjun
@ 2026-10-13 16:18 ` Greg KH
  2026-10-21  7:25   ` Jun
  2026-10-21  7:48   ` Oliver Neukum
  0 siblings, 2 replies; 4+ messages in thread
From: Greg KH @ 2026-10-13 16:18 UTC (permalink / raw)
  To: Lee Yongjun
  Cc: stern, digidietze, xiehongyu1, linux-usb, usb-storage,
	linux-kernel, syzkaller-bugs, syzbot+7317196294d50d6e55c3

On Tue, Oct 13, 2026 at 10:20:33PM +0900, Lee Yongjun wrote:
> The Option device initialization path issues bulk transfers
> without a timeout. If a device does not complete a transfer,
> the USB hub worker can remain blocked while holding the device
> mutex, causing usb_device_read() to hang.

So a broken device?

> Use the timeout-aware bulk transfer helper for the transfers
> in option_inquiry() and option_rezero(). This bounds the wait
> for each probe-time transfer to 30 seconds.

That feels very long, are you sure that works well?

> Tested with the syzbot C reproducer under QEMU/KVM. The
> patched kernel repeatedly completed USB device initialization
> attempts after approximately 30 seconds without triggering
> the previously observed hung-task warning.
> 
> This patch depends on Hongyu Xie's pending v3 series:
> https://lore.kernel.org/r/20261001123135.38831-1-xy521521@gmail.com/

This should go below the --- line and is why sashiko is confused.

> Fixes: 32ebbe7b6ad4 ("USB: usb-storage: add filter to "option_ms" to leave unrecognized devices alone")
> Reported-by: syzbot+7317196294d50d6e55c3@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7317196294d50d6e55c3
> Assisted-by: LLM
> 
> Signed-off-by: Lee Yongjun <jun85566@gmail.com>

No blank line above here is needed.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb-storage: option_ms: bound probe-time bulk transfers
  2026-10-13 16:18 ` Greg KH
@ 2026-10-21  7:25   ` Jun
  2026-10-21  7:48   ` Oliver Neukum
  1 sibling, 0 replies; 4+ messages in thread
From: Jun @ 2026-10-21  7:25 UTC (permalink / raw)
  To: Greg KH
  Cc: stern, digidietze, xiehongyu1, linux-usb, usb-storage,
	linux-kernel, syzkaller-bugs, syzbot+7317196294d50d6e55c3

> So a broken device?

Yes, effectively. The reproducer emulates a USB device that completes
enumeration but does not respond to the subsequent bulk OUT transfer.
Without a timeout, the hub worker remains blocked while holding the
device mutex, preventing other operations such as usb_device_read()
from proceeding.

> That feels very long, are you sure that works well?

I've reduced the timeout to 5 seconds, using usb_modeswitch's 3
seconds timeout for similar bulk transfers as a reference, with some
additional margin.
I retested with the syzkaller reproducer under QEMU, and
initialization now proceeds after 5 seconds instead of 30.

> This should go below the --- line and is why sashiko is confused.

Thank you, I'll move the dependency note below the --- line

> No blank line above here is needed.

I'll remove the extra blank line.

Thanks for your review. I'll send v2 with these changes.

Lee Yongjun


2026년 10월 14일 (수) 오전 1:18, Greg KH <gregkh@linuxfoundation.org>님이 작성:
>
> On Tue, Oct 13, 2026 at 10:20:33PM +0900, Lee Yongjun wrote:
> > The Option device initialization path issues bulk transfers
> > without a timeout. If a device does not complete a transfer,
> > the USB hub worker can remain blocked while holding the device
> > mutex, causing usb_device_read() to hang.
>
> So a broken device?
>
> > Use the timeout-aware bulk transfer helper for the transfers
> > in option_inquiry() and option_rezero(). This bounds the wait
> > for each probe-time transfer to 30 seconds.
>
> That feels very long, are you sure that works well?
>
> > Tested with the syzbot C reproducer under QEMU/KVM. The
> > patched kernel repeatedly completed USB device initialization
> > attempts after approximately 30 seconds without triggering
> > the previously observed hung-task warning.
> >
> > This patch depends on Hongyu Xie's pending v3 series:
> > https://lore.kernel.org/r/20261001123135.38831-1-xy521521@gmail.com/
>
> This should go below the --- line and is why sashiko is confused.
>
> > Fixes: 32ebbe7b6ad4 ("USB: usb-storage: add filter to "option_ms" to leave unrecognized devices alone")
> > Reported-by: syzbot+7317196294d50d6e55c3@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=7317196294d50d6e55c3
> > Assisted-by: LLM
> >
> > Signed-off-by: Lee Yongjun <jun85566@gmail.com>
>
> No blank line above here is needed.
>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb-storage: option_ms: bound probe-time bulk transfers
  2026-10-13 16:18 ` Greg KH
  2026-10-21  7:25   ` Jun
@ 2026-10-21  7:48   ` Oliver Neukum
  1 sibling, 0 replies; 4+ messages in thread
From: Oliver Neukum @ 2026-10-21  7:48 UTC (permalink / raw)
  To: Greg KH, Lee Yongjun
  Cc: stern, digidietze, xiehongyu1, linux-usb, usb-storage,
	linux-kernel, syzkaller-bugs, syzbot+7317196294d50d6e55c3

On 13.10.26 18:18, Greg KH wrote:
> On Tue, Oct 13, 2026 at 10:20:33PM +0900, Lee Yongjun wrote:
>> The Option device initialization path issues bulk transfers
>> without a timeout. If a device does not complete a transfer,
>> the USB hub worker can remain blocked while holding the device
>> mutex, causing usb_device_read() to hang.
> 
> So a broken device?
On a generic note, the SCSI layer is ready to deal with broken devices.
In consequence, so have SCSI LLDs. We cannot hold a lock that will stall
the SCSI layer in error handling indefinitely.

	Regards
		Oliver


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-21  7:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-13 13:20 [PATCH] usb-storage: option_ms: bound probe-time bulk transfers Lee Yongjun
2026-10-13 16:18 ` Greg KH
2026-10-21  7:25   ` Jun
2026-10-21  7:48   ` Oliver Neukum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®