block error issue with root hubs

block error issue with root hubs

[Oliver Neukum]

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

Hi,

looking at UAS error handling it seems to me that there is
a small likelihood of deadlocking when we wait on other tasks
processing PM requests on the same device. Do you think
the attached patch is enough or do we need to pass the flag down
into the HCDs?

	Regards
		Oliver

[-- Attachment #2: 0001-usb-core-hcd-fix-possible-deadlock-in-rh-control-tra.patch --]
[-- Type: text/x-patch, Size: 2696 bytes --]

From 767e9af371bf63413f1f7c0b2eca15bd52cdc1bb Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Tue, 28 Apr 2026 15:38:17 +0200
Subject: [PATCH] usb: core: hcd: fix possible deadlock in rh control transfers

From within the SCSI error handler memory allocations must not
trigger IO. Handling errors in UAS and the storage driver may
involve resetting a device. The thread doing the reset itself
relies on VM magic. However, that is insufficient, as resetting
a device involves resuming it. Resumption as well as resetting
involves conrol transfers to the parent of the device to be reset.
That may be a root hub. Hence usbcore must heed the flags passed
to usb_submit_urb() processing control transfers to root hubs.

The problem exist since the storage driver has been merged.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/core/hcd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 89221f1ce769..d95000c7b328 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -448,7 +448,7 @@ rh_string(int id, struct usb_hcd const *hcd, u8 *data, unsigned len)
 
 
 /* Root hub control transfers execute synchronously */
-static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
+static int rh_call_control(struct usb_hcd *hcd, struct urb *urb, gfp_t mf)
 {
 	struct usb_ctrlrequest *cmd;
 	u16		typeReq, wValue, wIndex, wLength;
@@ -483,8 +483,8 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
 	 * tbuf should be at least as big as the
 	 * USB hub descriptor.
 	 */
-	tbuf_size =  max_t(u16, sizeof(struct usb_hub_descriptor), wLength);
-	tbuf = kzalloc(tbuf_size, GFP_KERNEL);
+	tbuf_size = max_t(u16, sizeof(struct usb_hub_descriptor), wLength);
+	tbuf = kzalloc(tbuf_size, mf);
 	if (!tbuf) {
 		status = -ENOMEM;
 		goto err_alloc;
@@ -809,12 +809,12 @@ static int rh_queue_status (struct usb_hcd *hcd, struct urb *urb)
 	return retval;
 }
 
-static int rh_urb_enqueue (struct usb_hcd *hcd, struct urb *urb)
+static int rh_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mf)
 {
 	if (usb_endpoint_xfer_int(&urb->ep->desc))
 		return rh_queue_status (hcd, urb);
 	if (usb_endpoint_xfer_control(&urb->ep->desc))
-		return rh_call_control (hcd, urb);
+		return rh_call_control(hcd, urb, mf);
 	return -EINVAL;
 }
 
@@ -1535,7 +1535,7 @@ int usb_hcd_submit_urb (struct urb *urb, gfp_t mem_flags)
 	 */
 
 	if (is_root_hub(urb->dev)) {
-		status = rh_urb_enqueue(hcd, urb);
+		status = rh_urb_enqueue(hcd, urb, mem_flags);
 	} else {
 		status = map_urb_for_dma(hcd, urb, mem_flags);
 		if (likely(status == 0)) {
-- 
2.54.0

Re: block error issue with root hubs

[Alan Stern]

On Tue, Apr 28, 2026 at 09:02:22PM +0200, Oliver Neukum wrote:
> Hi,
> 
> looking at UAS error handling it seems to me that there is
> a small likelihood of deadlocking when we wait on other tasks
> processing PM requests on the same device. Do you think
> the attached patch is enough or do we need to pass the flag down
> into the HCDs?

This is good enough.  Practically everything the HCDs do is under a 
private spinlock, and they have to assume that their callbacks are 
invoked in atomic context anyway.

> 	Regards
> 		Oliver

> From 767e9af371bf63413f1f7c0b2eca15bd52cdc1bb Mon Sep 17 00:00:00 2001
> From: Oliver Neukum <oneukum@suse.com>
> Date: Tue, 28 Apr 2026 15:38:17 +0200
> Subject: [PATCH] usb: core: hcd: fix possible deadlock in rh control transfers
> 
> From within the SCSI error handler memory allocations must not
> trigger IO. Handling errors in UAS and the storage driver may
> involve resetting a device. The thread doing the reset itself
> relies on VM magic. However, that is insufficient, as resetting
> a device involves resuming it. Resumption as well as resetting
> involves conrol transfers to the parent of the device to be reset.
> That may be a root hub. Hence usbcore must heed the flags passed
> to usb_submit_urb() processing control transfers to root hubs.
> 
> The problem exist since the storage driver has been merged.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
>  drivers/usb/core/hcd.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 89221f1ce769..d95000c7b328 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -448,7 +448,7 @@ rh_string(int id, struct usb_hcd const *hcd, u8 *data, unsigned len)
>  
>  
>  /* Root hub control transfers execute synchronously */
> -static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
> +static int rh_call_control(struct usb_hcd *hcd, struct urb *urb, gfp_t mf)

However, I do suggest writing out the new variable name in full as 
mem_flags rather than the cryptic mf.  For consistency with the other 
usages in the source file if nothing else.

Alan Stern