RFC: Prevent long uninterruptible waits in usbcore

RFC: Prevent long uninterruptible waits in usbcore

[Alan Stern]

I'm asking for comments on the patch below.  The problem it fixes is 
that usb_start_wait_urb(), which is used by utility routines like 
usb_bulk_msg() and usb_control_msg(), can have arbitrarily long 
uninterruptible waits.  And of course this can trigger the kernel's 
warning about a hung task.

Normally this isn't a problem, because drivers calling these routines 
generally specify timeouts on the order of 10 seconds or less.  In the 
particular case found by syzbot, however, the usbtmc driver uses a 
timeout value that it gets from an ioctl with no checking for 
excessively large numbers (see usbtmc_ioctl_set_timeout()).  We could 
change this one function, but what about other drivers?  There should be 
a single policy on how we handle these things.

Now, I suppose the reason that the usb_start_wait_urb() uses 
uninterruptible waits is that drivers don't want their calls to be 
interrupted and aren't prepared for that possibility.  But we also don't 
want them to tie up a kernel thread in a ridiculously long 
uninterruptible wait state.

I thought that a reasonable compromise would be to keep the 
uninterruptible waits for timeout periods less than 60 seconds (somewhat 
arbitrary, but hopefully short enough not to trigger the hung-task 
detector) and make them interruptible if the timeout is longer.  The 
idea is that long USB timeouts don't normally arise in the kernel, so 
they are probably caused by a user request (or a bad device), which 
would mean that interrupting them wouldn't be such a bad thing.

Any other ideas or thoughts about this?

Alan Stern

 drivers/usb/core/message.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

Index: usb-devel/drivers/usb/core/message.c
===================================================================
--- usb-devel.orig/drivers/usb/core/message.c
+++ usb-devel/drivers/usb/core/message.c
@@ -25,6 +25,8 @@
 
 #include "usb.h"
 
+#define MAX_UNINTERRUPTIBLE_TIMEOUT_MS	60000
+
 static void cancel_async_set_config(struct usb_device *udev);
 
 struct api_context {
@@ -42,16 +44,16 @@ static void usb_api_blocking_completion(
 
 
 /*
- * Starts urb and waits for completion or timeout. Note that this call
- * is NOT interruptible. Many device driver i/o requests should be
- * interruptible and therefore these drivers should implement their
- * own interruptible routines.
+ * Starts urb and waits for completion or timeout.  Timeout lengths <= 0
+ * are taken to be as long as possible.
+ * The wait is NOT interruptible if the timeout period is no longer than
+ * MAX_UNINTERRUPTIBLE_TIMEOUT_MS, otherwise it IS interruptible.
  */
 static int usb_start_wait_urb(struct urb *urb, int timeout, int *actual_length)
 {
 	struct api_context ctx;
 	unsigned long expire;
-	int retval;
+	int rc, retval;
 
 	init_completion(&ctx.done);
 	urb->context = &ctx;
@@ -60,8 +62,14 @@ static int usb_start_wait_urb(struct urb
 	if (unlikely(retval))
 		goto out;
 
-	expire = timeout ? msecs_to_jiffies(timeout) : MAX_SCHEDULE_TIMEOUT;
-	if (!wait_for_completion_timeout(&ctx.done, expire)) {
+	expire = (timeout > 0) ? msecs_to_jiffies(timeout) :
+			MAX_SCHEDULE_TIMEOUT;
+	if (expire <= msecs_to_jiffies(MAX_UNINTERRUPTIBLE_TIMEOUT_MS))
+		rc = (wait_for_completion_timeout(&ctx.done, expire) > 0);
+	else
+		rc = (wait_for_completion_interruptible_timeout(
+				&ctx.done, expire) > 0);
+	if (!rc) {
 		usb_kill_urb(urb);
 		retval = (ctx.status == -ENOENT ? -ETIMEDOUT : ctx.status);
 

Re: RFC: Prevent long uninterruptible waits in usbcore

[Greg KH]

On Sun, Feb 08, 2026 at 09:33:02PM -0500, Alan Stern wrote:
> I'm asking for comments on the patch below.  The problem it fixes is 
> that usb_start_wait_urb(), which is used by utility routines like 
> usb_bulk_msg() and usb_control_msg(), can have arbitrarily long 
> uninterruptible waits.  And of course this can trigger the kernel's 
> warning about a hung task.
> 
> Normally this isn't a problem, because drivers calling these routines 
> generally specify timeouts on the order of 10 seconds or less.  In the 
> particular case found by syzbot, however, the usbtmc driver uses a 
> timeout value that it gets from an ioctl with no checking for 
> excessively large numbers (see usbtmc_ioctl_set_timeout()).  We could 
> change this one function, but what about other drivers?  There should be 
> a single policy on how we handle these things.
> 
> Now, I suppose the reason that the usb_start_wait_urb() uses 
> uninterruptible waits is that drivers don't want their calls to be 
> interrupted and aren't prepared for that possibility.  But we also don't 
> want them to tie up a kernel thread in a ridiculously long 
> uninterruptible wait state.
> 
> I thought that a reasonable compromise would be to keep the 
> uninterruptible waits for timeout periods less than 60 seconds (somewhat 
> arbitrary, but hopefully short enough not to trigger the hung-task 
> detector) and make them interruptible if the timeout is longer.  The 
> idea is that long USB timeouts don't normally arise in the kernel, so 
> they are probably caused by a user request (or a bad device), which 
> would mean that interrupting them wouldn't be such a bad thing.
> 
> Any other ideas or thoughts about this?

Having an upper-bound on this is a good idea, and also the usbtmc driver
should also be fixed to just reject extra-long delays in that ioctl as
that's a user-visable way to stall a cpu.

So no objection from me with this patch.

thanks,

greg k-h

Re: RFC: Prevent long uninterruptible waits in usbcore

[Alan Stern]

Replying to both Greg and Oliver:

On Mon, Feb 09, 2026 at 11:05:45AM +0100, Greg KH wrote:
> On Sun, Feb 08, 2026 at 09:33:02PM -0500, Alan Stern wrote:
> > I'm asking for comments on the patch below.  The problem it fixes is 
> > that usb_start_wait_urb(), which is used by utility routines like 
> > usb_bulk_msg() and usb_control_msg(), can have arbitrarily long 
> > uninterruptible waits.  And of course this can trigger the kernel's 
> > warning about a hung task.
> > 
> > Normally this isn't a problem, because drivers calling these routines 
> > generally specify timeouts on the order of 10 seconds or less.  In the 
> > particular case found by syzbot, however, the usbtmc driver uses a 
> > timeout value that it gets from an ioctl with no checking for 
> > excessively large numbers (see usbtmc_ioctl_set_timeout()).  We could 
> > change this one function, but what about other drivers?  There should be 
> > a single policy on how we handle these things.
> > 
> > Now, I suppose the reason that the usb_start_wait_urb() uses 
> > uninterruptible waits is that drivers don't want their calls to be 
> > interrupted and aren't prepared for that possibility.  But we also don't 
> > want them to tie up a kernel thread in a ridiculously long 
> > uninterruptible wait state.
> > 
> > I thought that a reasonable compromise would be to keep the 
> > uninterruptible waits for timeout periods less than 60 seconds (somewhat 
> > arbitrary, but hopefully short enough not to trigger the hung-task 
> > detector) and make them interruptible if the timeout is longer.  The 
> > idea is that long USB timeouts don't normally arise in the kernel, so 
> > they are probably caused by a user request (or a bad device), which 
> > would mean that interrupting them wouldn't be such a bad thing.
> > 
> > Any other ideas or thoughts about this?
> 
> Having an upper-bound on this is a good idea, and also the usbtmc driver
> should also be fixed to just reject extra-long delays in that ioctl as
> that's a user-visable way to stall a cpu.

Okay, I'll write a fix for the usbtmc ioctl routine too.  It doesn't 
really stall a CPU, but it does hang a kernel thread.

However, I don't know anything about the USB Test and Measurement class.  
Is 60 seconds a sufficiently large limit for timeouts?

It would be great if there was a way to find other places in the kernel 
where a timeout value comes from the user or the device with no 
checking.  But I don't know of any way to do this.

> So no objection from me with this patch.

On Mon, Feb 09, 2026 at 11:03:09AM +0100, Oliver Neukum wrote:
> Hi,
>
> is this a proof of concept or a test of the real fix?

Proof of concept, since I'm not yet convinced that this is the best way 
to fix the problem.

> I am afraid this is highly problematic.
>
> 1. You'll hit every user of the API. These are probably not ready to 
> handle signals. This includes usb_bulk_message() and the interrupt 
> version.

It won't hit _every_ user, only the ones that ask for timeouts longer 
than 60 seconds.  I don't know how common this _is_, but it _should_ be 
very uncommon since keeping tasks in uninterruptible wait states for 
prolonged periods of time is a bad idea.

In fact, it seems likely that the most common reason for this to happen 
is because of drivers using a timeout value that comes from the device 
itself or from userspace.

> 2. wait_for_completion_interruptible_timeout() reacts to _any_ signal. Do you really want to abort for SIGIO or SIGWINCH right after urb submission?

No; this was merely a simplistic first stab at a solution.

> 3. The error handling is broken. You cannot tell a true timeout and a 
> signal apart.

Correct.  But considering that the caller isn't expecting to be 
interrupted anyway, this doesn't matter.  A failure is a failure, no 
matter what the cause.

> It seems to me that really want a clean API introducing usb_X_message_killable()

Well, the original thought was that anyone needing to send a message 
interruptibly would program it themselves.  That's what the comment at 
the start of the routine says now.

Would it be more acceptable simply to reject outright any calls with a 
timeout longer than 60 seconds?  Perhaps along with a new API as you 
suggest (although I'm not sure that it's really needed)?

Alan Stern

Re: RFC: Prevent long uninterruptible waits in usbcore

[Oliver Neukum]

On 09.02.26 16:16, Alan Stern wrote:

> It would be great if there was a way to find other places in the kernel
> where a timeout value comes from the user or the device with no
> checking.  But I don't know of any way to do this.

Somebody will include it in code analysis tools. I wouldn't worry.

That said, SCSI worries me. Some operations will have quite some
timeout. I doubt you can assume that a tape will be rewound or
a disk formatted in 60 seconds. Currently this is safe because
usb-storage uses a kernel thread you cannot just send signals to.

> It won't hit _every_ user, only the ones that ask for timeouts longer
> than 60 seconds.  I don't know how common this _is_, but it _should_ be
> very uncommon since keeping tasks in uninterruptible wait states for
> prolonged periods of time is a bad idea.

Yes.

>> 2. wait_for_completion_interruptible_timeout() reacts to _any_ signal. Do you really want to abort for SIGIO or SIGWINCH right after urb submission?
> 
> No; this was merely a simplistic first stab at a solution.

Good.
  
>> 3. The error handling is broken. You cannot tell a true timeout and a
>> signal apart.
> 
> Correct.  But considering that the caller isn't expecting to be
> interrupted anyway, this doesn't matter.  A failure is a failure, no
> matter what the cause.

Well, no. If the failure is due to a harmless signal, you should retry.
If it was due to SIGHUP or SIGKILL, you should die.

> Would it be more acceptable simply to reject outright any calls with a
> timeout longer than 60 seconds?  Perhaps along with a new API as you
> suggest (although I'm not sure that it's really needed)?

That would be a clean solution. We cannot let user space force an
effectively unlimited uninterruptible sleep.

	Regards
		Oliver

[PATCH 1/2] usbcore: Reject excessively long uninterruptible timeouts

[Alan Stern]

The synchronous message API in usbcore (usb_control_msg(),
usb_bulk_msg(), and so on) allows unlimited timeout durations.  And
since it uses uninterruptible waits, this leaves open the possibility
of hanging a task for an indefinitely long time, with no way to kill
it short of unplugging the target device.

To prevent this sort of problem, enforce a maximum limit on the length
of synchronous timeouts.  The limit chosen here, somewhat arbitrarily,
is 60 seconds.  On many systems (although not all) this is short
enough to avoid triggering the kernel's hung-task detector.

Note that this will affect the timeouts accepted by the
USBDEVFS_CONTROL and USBDEVFS_BULK ioctls in usbfs, since they rely on
the synchronous message API.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Link: https://lore.kernel.org/linux-usb/3acfe838-6334-4f6d-be7c-4bb01704b33d@rowland.harvard.edu/
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
CC: stable@vger.kernel.org

---

 drivers/usb/core/message.c |   23 ++++++++++++++---------
 include/linux/usb.h        |    3 +++
 2 files changed, 17 insertions(+), 9 deletions(-)

Index: usb-devel/drivers/usb/core/message.c
===================================================================
--- usb-devel.orig/drivers/usb/core/message.c
+++ usb-devel/drivers/usb/core/message.c
@@ -46,6 +46,9 @@ static void usb_api_blocking_completion(
  * is NOT interruptible. Many device driver i/o requests should be
  * interruptible and therefore these drivers should implement their
  * own interruptible routines.
+ *
+ * Because the wait is uninterruptible, we enforce a maximum limit on
+ * the length of the timeout.
  */
 static int usb_start_wait_urb(struct urb *urb, int timeout, int *actual_length)
 {
@@ -56,11 +59,17 @@ static int usb_start_wait_urb(struct urb
 	init_completion(&ctx.done);
 	urb->context = &ctx;
 	urb->actual_length = 0;
+
+	if (timeout <= 0 || timeout > USB_MAX_SYNCHRONOUS_TIMEOUT) {
+		retval = -EINVAL;
+		goto out;
+	}
+
 	retval = usb_submit_urb(urb, GFP_NOIO);
 	if (unlikely(retval))
 		goto out;
 
-	expire = timeout ? msecs_to_jiffies(timeout) : MAX_SCHEDULE_TIMEOUT;
+	expire = msecs_to_jiffies(timeout);
 	if (!wait_for_completion_timeout(&ctx.done, expire)) {
 		usb_kill_urb(urb);
 		retval = (ctx.status == -ENOENT ? -ETIMEDOUT : ctx.status);
@@ -173,8 +182,7 @@ EXPORT_SYMBOL_GPL(usb_control_msg);
  * @index: USB message index value
  * @driver_data: pointer to the data to send
  * @size: length in bytes of the data to send
- * @timeout: time in msecs to wait for the message to complete before timing
- *	out (if 0 the wait is forever)
+ * @timeout: time in msecs to wait for the message to complete before timing out
  * @memflags: the flags for memory allocation for buffers
  *
  * Context: !in_interrupt ()
@@ -232,8 +240,7 @@ EXPORT_SYMBOL_GPL(usb_control_msg_send);
  * @index: USB message index value
  * @driver_data: pointer to the data to be filled in by the message
  * @size: length in bytes of the data to be received
- * @timeout: time in msecs to wait for the message to complete before timing
- *	out (if 0 the wait is forever)
+ * @timeout: time in msecs to wait for the message to complete before timing out
  * @memflags: the flags for memory allocation for buffers
  *
  * Context: !in_interrupt ()
@@ -304,8 +311,7 @@ EXPORT_SYMBOL_GPL(usb_control_msg_recv);
  * @len: length in bytes of the data to send
  * @actual_length: pointer to a location to put the actual length transferred
  *	in bytes
- * @timeout: time in msecs to wait for the message to complete before
- *	timing out (if 0 the wait is forever)
+ * @timeout: time in msecs to wait for the message to complete before timing out
  *
  * Context: task context, might sleep.
  *
@@ -337,8 +343,7 @@ EXPORT_SYMBOL_GPL(usb_interrupt_msg);
  * @len: length in bytes of the data to send
  * @actual_length: pointer to a location to put the actual length transferred
  *	in bytes
- * @timeout: time in msecs to wait for the message to complete before
- *	timing out (if 0 the wait is forever)
+ * @timeout: time in msecs to wait for the message to complete before timing out
  *
  * Context: task context, might sleep.
  *
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -1863,6 +1863,9 @@ void usb_free_noncoherent(struct usb_dev
  *                         SYNCHRONOUS CALL SUPPORT                  *
  *-------------------------------------------------------------------*/
 
+/* Maximum value allowed for timeout in synchronous routines below */
+#define USB_MAX_SYNCHRONOUS_TIMEOUT		60000	/* ms */
+
 extern int usb_control_msg(struct usb_device *dev, unsigned int pipe,
 	__u8 request, __u8 requesttype, __u16 value, __u16 index,
 	void *data, __u16 size, int timeout);

[PATCH 2/2] USB: usbtmc: Don't accept very long timeouts

[Alan Stern]

The usbtmc driver accepts timeout values specified by the user in an
ioctl command, and uses these timeouts for usb_bulk_msg() calls.
Since that function will reject timeouts that are too long, change the
driver to reject ioctls setting the timeout to a value above the limit.

If it turns out some users do need very long timeouts for these
messages, we will have to remove the limit and rewrite the calls to
usb_bulk_msg() to be interruptible.

Reported-by: syzbot+25ba18e2c5040447585d@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-usb/8e1c7ac5-e076-44b0-84b8-1b34b20f0ae1@suse.com/T/#t
Tested-by: syzbot+25ba18e2c5040447585d@syzkaller.appspotmail.com
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Fixes: 048c6d88a021 ("usb: usbtmc: Add ioctls to set/get usb timeout")
CC: stable@vger.kernel.org

---

 drivers/usb/class/usbtmc.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Index: usb-devel/drivers/usb/class/usbtmc.c
===================================================================
--- usb-devel.orig/drivers/usb/class/usbtmc.c
+++ usb-devel/drivers/usb/class/usbtmc.c
@@ -34,6 +34,8 @@
 #define USBTMC_MIN_TIMEOUT	100
 /* Default USB timeout (in milliseconds) */
 #define USBTMC_TIMEOUT		5000
+/* Maximum USB timeout (in milliseconds) */
+#define USBTMC_MAX_TIMEOUT	USB_MAX_SYNCHRONOUS_TIMEOUT
 
 /* Max number of urbs used in write transfers */
 #define MAX_URBS_IN_FLIGHT	16
@@ -2014,10 +2016,11 @@ static int usbtmc_ioctl_set_timeout(stru
 	if (get_user(timeout, (__u32 __user *)arg))
 		return -EFAULT;
 
-	/* Note that timeout = 0 means
-	 * MAX_SCHEDULE_TIMEOUT in usb_control_msg
+	/*
+	 * Impose a maximum limit to timeouts because the wait in
+	 * usb_bulk_msg is uninterruptible.
 	 */
-	if (timeout < USBTMC_MIN_TIMEOUT)
+	if (timeout < USBTMC_MIN_TIMEOUT || timeout > USBTMC_MAX_TIMEOUT)
 		return -EINVAL;
 
 	file_data->timeout = timeout;

Re: [PATCH 1/2] usbcore: Reject excessively long uninterruptible timeouts

[Oliver Neukum]

On 09.02.26 20:59, Alan Stern wrote:

> Note that this will affect the timeouts accepted by the
> USBDEVFS_CONTROL and USBDEVFS_BULK ioctls in usbfs, since they rely on
> the synchronous message API.

That means you cannot just change the semantics of the call.

> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Link: https://lore.kernel.org/linux-usb/3acfe838-6334-4f6d-be7c-4bb01704b33d@rowland.harvard.edu/
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> CC: stable@vger.kernel.org
> 
> ---
> 
>   drivers/usb/core/message.c |   23 ++++++++++++++---------
>   include/linux/usb.h        |    3 +++
>   2 files changed, 17 insertions(+), 9 deletions(-)
> 
> Index: usb-devel/drivers/usb/core/message.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/message.c
> +++ usb-devel/drivers/usb/core/message.c
> @@ -46,6 +46,9 @@ static void usb_api_blocking_completion(
>    * is NOT interruptible. Many device driver i/o requests should be
>    * interruptible and therefore these drivers should implement their
>    * own interruptible routines.
> + *
> + * Because the wait is uninterruptible, we enforce a maximum limit on
> + * the length of the timeout.
>    */
>   static int usb_start_wait_urb(struct urb *urb, int timeout, int *actual_length)

If you are touching this at all, we might look into the fundamental question
of why a timeout can be negative at all.

>   {
> @@ -56,11 +59,17 @@ static int usb_start_wait_urb(struct urb
>   	init_completion(&ctx.done);
>   	urb->context = &ctx;
>   	urb->actual_length = 0;
> +
> +	if (timeout <= 0 || timeout > USB_MAX_SYNCHRONOUS_TIMEOUT) {

You are rejecting values that used to be acceptable. That is unavoidable,
but somebody was surely foolish enough to pass 0 from user space in every
case and got away with it until now.

It would seem to me that you need to at least convert 0 to
USB_MAX_SYNCHRONOUS_TIMEOUT, if not outright capping the
timeout instead of erroring out.

	Regards
		Oliver

Re: [PATCH 2/2] USB: usbtmc: Don't accept very long timeouts

[Oliver Neukum]

Hi,

I am terribly sorry to be only critical today.

On 09.02.26 21:00, Alan Stern wrote:
> The usbtmc driver accepts timeout values specified by the user in an
> ioctl command, and uses these timeouts for usb_bulk_msg() calls.
> Since that function will reject timeouts that are too long, change the
> driver to reject ioctls setting the timeout to a value above the limit.
> 
> If it turns out some users do need very long timeouts for these
> messages, we will have to remove the limit and rewrite the calls to
> usb_bulk_msg() to be interruptible.
> 
> Reported-by: syzbot+25ba18e2c5040447585d@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/linux-usb/8e1c7ac5-e076-44b0-84b8-1b34b20f0ae1@suse.com/T/#t
> Tested-by: syzbot+25ba18e2c5040447585d@syzkaller.appspotmail.com
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Fixes: 048c6d88a021 ("usb: usbtmc: Add ioctls to set/get usb timeout")
> CC: stable@vger.kernel.org
> 
> ---
> 
>   drivers/usb/class/usbtmc.c |    9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> Index: usb-devel/drivers/usb/class/usbtmc.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/class/usbtmc.c
> +++ usb-devel/drivers/usb/class/usbtmc.c
> @@ -34,6 +34,8 @@
>   #define USBTMC_MIN_TIMEOUT	100
>   /* Default USB timeout (in milliseconds) */
>   #define USBTMC_TIMEOUT		5000
> +/* Maximum USB timeout (in milliseconds) */
> +#define USBTMC_MAX_TIMEOUT	USB_MAX_SYNCHRONOUS_TIMEOUT
>   
>   /* Max number of urbs used in write transfers */
>   #define MAX_URBS_IN_FLIGHT	16
> @@ -2014,10 +2016,11 @@ static int usbtmc_ioctl_set_timeout(stru

You are doing this for the generic timeout.

>   	if (get_user(timeout, (__u32 __user *)arg))
>   		return -EFAULT;
>   
> -	/* Note that timeout = 0 means
> -	 * MAX_SCHEDULE_TIMEOUT in usb_control_msg
> +	/*
> +	 * Impose a maximum limit to timeouts because the wait in
> +	 * usb_bulk_msg is uninterruptible.
>   	 */
> -	if (timeout < USBTMC_MIN_TIMEOUT)
> +	if (timeout < USBTMC_MIN_TIMEOUT || timeout > USBTMC_MAX_TIMEOUT)
>   		return -EINVAL;

Limiting the range.

>   	file_data->timeout = timeout;

This is, however, used in multiple places. Among them
usbtmc_generic_read(), which depends on a device actually
producing data. That uses wait_event_interruptible_timeout()

I am afraid you will have to check in the specific ioctl
or recode the wait in it.

	Regards
		Oliver

  

Re: [PATCH 2/2] USB: usbtmc: Don't accept very long timeouts

[Alan Stern]

On Tue, Feb 10, 2026 at 10:03:09AM +0100, Oliver Neukum wrote:
> Hi,
> 
> I am terribly sorry to be only critical today.

That's okay; I can take it.  :-)

> > +	/*
> > +	 * Impose a maximum limit to timeouts because the wait in
> > +	 * usb_bulk_msg is uninterruptible.
> >   	 */
> > -	if (timeout < USBTMC_MIN_TIMEOUT)
> > +	if (timeout < USBTMC_MIN_TIMEOUT || timeout > USBTMC_MAX_TIMEOUT)
> >   		return -EINVAL;
> 
> Limiting the range.
> 
> >   	file_data->timeout = timeout;
> 
> This is, however, used in multiple places. Among them
> usbtmc_generic_read(), which depends on a device actually
> producing data. That uses wait_event_interruptible_timeout()
> 
> I am afraid you will have to check in the specific ioctl
> or recode the wait in it.

How about accepting the value from the user, but limiting the timeout to 
USBTMC_MAX_TIMEOUT in the usb_bulk_msg() calls without changing the 
wait_event_interruptible_timeout() calls?

It probably would help to know something about how this class and driver 
were meant to work...

Alan Stern

Re: [PATCH 2/2] USB: usbtmc: Don't accept very long timeouts

[Oliver Neukum]

Hi,

I am glad that you can take it.

On 11.02.26 04:25, Alan Stern wrote:
  
> How about accepting the value from the user, but limiting the timeout to
> USBTMC_MAX_TIMEOUT in the usb_bulk_msg() calls without changing the
> wait_event_interruptible_timeout() calls?

That would presumably work. Yet I am not happy with it.

> It probably would help to know something about how this class and driver
> were meant to work...

Indeed. But we have to work with what we have.

This pushes me to take a step back, figuratively speaking, and
try to look at the situation in general. And I must say that
at this point, you seem to me to no longer be coding to produce
the optimal result, but so that you can keep using
usb_bulk_msg() with regards to TMC.

The issue exists in general so the first patch in your series
is necessary in any case, even if it may need improvements,
but in case of the second patch, I think you should go for
the full solution. That is, use an URB and wait for it to complete
in interruptible sleep without usage of a helper.
We know this will work.

	Regards
		Oliver

Re: [PATCH 2/2] USB: usbtmc: Don't accept very long timeouts

[Alan Stern]

On Wed, Feb 11, 2026 at 11:28:38AM +0100, Oliver Neukum wrote:
> On 11.02.26 04:25, Alan Stern wrote:
> > How about accepting the value from the user, but limiting the timeout to
> > USBTMC_MAX_TIMEOUT in the usb_bulk_msg() calls without changing the
> > wait_event_interruptible_timeout() calls?
> 
> That would presumably work. Yet I am not happy with it.
> 
> > It probably would help to know something about how this class and driver
> > were meant to work...
> 
> Indeed. But we have to work with what we have.
> 
> This pushes me to take a step back, figuratively speaking, and
> try to look at the situation in general. And I must say that
> at this point, you seem to me to no longer be coding to produce
> the optimal result, but so that you can keep using
> usb_bulk_msg() with regards to TMC.
> 
> The issue exists in general so the first patch in your series
> is necessary in any case, even if it may need improvements,
> but in case of the second patch, I think you should go for
> the full solution. That is, use an URB and wait for it to complete
> in interruptible sleep without usage of a helper.
> We know this will work.

So there are two issues to consider.

First, the change to usbcore.  Rather than giving an error if the 
requested timeout is too large, I suggest decreasing it to 
USB_MAX_UNINTERRUPTIBLE_TIMEOUT and continuing.  Does that seem 
reasonable for the synchronous usbfs ioctls?  (Note that the 
USBDEVFS_SUBMITURB ioctl is not subject to this problem, since it is 
asynchronous.)

Second, what to do for usbtmc.  One approach is to do nothing and rely 
on the change to usbcore.  The simplest alternative seems to be 
something you suggested earlier: Add a usb_bulk_msg_interruptible() API 
to the core and make usbtmc use it instead of usb_bulk_msg().  Of 
course, this also has the potential problem you mentioned: The transfer 
could be interrupted by a random unblocked signal.

Which leads me to ask: Which consideration do you think is more 
important for usbtmc: Having a timeout that is possibly too short, or 
being subject to interruptions by unimportant signals?  I can't think of 
any way to avoid both.

(Actually, I can: Use an interruptible wait, but restart from where it 
left off whenever it is interrupted.  This seems rather ridiculous.  For 
one thing, the user would not be able to cut the call short, which 
defeats the main reason for making something interruptible.  For 
another, it could consume a signal without the user's knowledge.)

My opinion is that if usbtmc would really be worried about limiting 
timeouts to 60 seconds for its usb_bulk_msg() calls then it shouldn't 
have used usb_bulk_msg() in the first place.  That API was never 
intended for any transfer that might delay more than 10 seconds or so.  
For this reason I favor the first approach: Do nothing.

Alan Stern

Re: [PATCH 2/2] USB: usbtmc: Don't accept very long timeouts

[Oliver Neukum]

On 11.02.26 16:31, Alan Stern wrote:

> First, the change to usbcore.  Rather than giving an error if the
> requested timeout is too large, I suggest decreasing it to
> USB_MAX_UNINTERRUPTIBLE_TIMEOUT and continuing.  Does that seem
> reasonable for the synchronous usbfs ioctls?  (Note that the
> USBDEVFS_SUBMITURB ioctl is not subject to this problem, since it is
> asynchronous.)

Yes, this seems entirely reasonable.
   
> Which leads me to ask: Which consideration do you think is more
> important for usbtmc: Having a timeout that is possibly too short, or
> being subject to interruptions by unimportant signals?  I can't think of
> any way to avoid both.

May I suggest a third alternative?
Implement usb_bulk_msg_killable()
  > My opinion is that if usbtmc would really be worried about limiting
> timeouts to 60 seconds for its usb_bulk_msg() calls then it shouldn't
> have used usb_bulk_msg() in the first place.  That API was never
> intended for any transfer that might delay more than 10 seconds or so.
> For this reason I favor the first approach: Do nothing.

Well, what conclusion does that support? Not using usb_bulk_msg()?

	Regards
		Oliver

Re: [PATCH 2/2] USB: usbtmc: Don't accept very long timeouts

[Alan Stern]

On Wed, Feb 11, 2026 at 04:59:40PM +0100, Oliver Neukum wrote:
> 
> 
> On 11.02.26 16:31, Alan Stern wrote:
> 
> > First, the change to usbcore.  Rather than giving an error if the
> > requested timeout is too large, I suggest decreasing it to
> > USB_MAX_UNINTERRUPTIBLE_TIMEOUT and continuing.  Does that seem
> > reasonable for the synchronous usbfs ioctls?  (Note that the
> > USBDEVFS_SUBMITURB ioctl is not subject to this problem, since it is
> > asynchronous.)
> 
> Yes, this seems entirely reasonable.

Okay, good, I'll write that.

> > Which leads me to ask: Which consideration do you think is more
> > important for usbtmc: Having a timeout that is possibly too short, or
> > being subject to interruptions by unimportant signals?  I can't think of
> > any way to avoid both.
> 
> May I suggest a third alternative?
> Implement usb_bulk_msg_killable()

Under what circumstances would the transfer be killed?  And how would 
the user be able to do this?  Would you temporarily block all signals 
except for a few like SIGINT, SIGTERM, SIGQUIT, and SIGKILL?  How would 
you choose which ones, exactly?

>  > My opinion is that if usbtmc would really be worried about limiting
> > timeouts to 60 seconds for its usb_bulk_msg() calls then it shouldn't
> > have used usb_bulk_msg() in the first place.  That API was never
> > intended for any transfer that might delay more than 10 seconds or so.
> > For this reason I favor the first approach: Do nothing.
> 
> Well, what conclusion does that support? Not using usb_bulk_msg()?

It all depends on what you want to accomplish.  In this case we don't 
really know what those calls in the usbtmc driver are meant to do.  At 
least, I don't.

Alan Stern

Re: [PATCH 2/2] USB: usbtmc: Don't accept very long timeouts

[Oliver Neukum]

On 11.02.26 17:18, Alan Stern wrote:

> Under what circumstances would the transfer be killed?  And how would
> the user be able to do this?  Would you temporarily block all signals
> except for a few like SIGINT, SIGTERM, SIGQUIT, and SIGKILL?  How would
> you choose which ones, exactly?

In abstract that is a good question, but it is for user space to solve.
TASK_KILLABLE does the job for us. In fact I'd suggest that you use
wait_event_killable() which should delegate the issue fully.
  
> It all depends on what you want to accomplish.  In this case we don't
> really know what those calls in the usbtmc driver are meant to do.  At
> least, I don't.

I share that problem. In that case, as little as we can get away with
and still solve the issue. "Little" being defined with as little
impact.

	Regards
		Oliver

Re: [PATCH 2/2] USB: usbtmc: Don't accept very long timeouts

[Alan Stern]

On Wed, Feb 11, 2026 at 06:59:17PM +0100, Oliver Neukum wrote:
> On 11.02.26 17:18, Alan Stern wrote:
> 
> > Under what circumstances would the transfer be killed?  And how would
> > the user be able to do this?  Would you temporarily block all signals
> > except for a few like SIGINT, SIGTERM, SIGQUIT, and SIGKILL?  How would
> > you choose which ones, exactly?
> 
> In abstract that is a good question, but it is for user space to solve.
> TASK_KILLABLE does the job for us. In fact I'd suggest that you use
> wait_event_killable() which should delegate the issue fully.
> > It all depends on what you want to accomplish.  In this case we don't
> > really know what those calls in the usbtmc driver are meant to do.  At
> > least, I don't.
> 
> I share that problem. In that case, as little as we can get away with
> and still solve the issue. "Little" being defined with as little
> impact.

In fact, is there any reason not to make usb_start_wait_urb() use 
wait_for_completion_killable_timeout() always?  Presumably a lot of the 
usages occur in the context of kernel threads that don't receive 
signals, and that ones that occur in the context of user threads 
arguably _should_ be killable.

If that was changed then usbtmc wouldn't need any revisions at all.

Alan Stern

Re: [PATCH 2/2] USB: usbtmc: Don't accept very long timeouts

[Oliver Neukum]

On 12.02.26 03:34, Alan Stern wrote:

> In fact, is there any reason not to make usb_start_wait_urb() use
> wait_for_completion_killable_timeout() always?  Presumably a lot of the

Yes, there is such a reason. In fact a number of related but slightly
distinct reasons. They all boil down to all drivers being potentially
being in the block IO path due to resetting and resuming devices as
operations shared between all drivers but running in the context of the
thread that happens to trigger them.

Let us assume a device with two interfaces, for the sake of argument
a combined serial and storage device. Now user space opens the serial device.
The kernel (in the context of the opening task) now resumes both interfaces.
While the device is being resumed, the kernel decides to write out
pages associated with the storage interface. It will notice that the device
is being resumed and that the IO has to wait for that operation to finish
(and succeed, but that's a different question).

Now what does happen if the task doing the resuming dies due to a signal
while running a resume() method? Yes, a deadlock.
In short, resume(), reset_resume(), pre_reset() and post_reset()
cannot handle signals. Hence any waiting they do must be uninterruptible.

	Regards
		Oliver