[PATCH v2] usb: gadget: dummy_hcd: fix premature URB completion when ZLP follows partial transfer

Sebastian Urban posted 1 patch 3 weeks, 1 day ago
drivers/usb/gadget/udc/dummy_hcd.c | 6 ++++++
1 file changed, 6 insertions(+)
[PATCH v2] usb: gadget: dummy_hcd: fix premature URB completion when ZLP follows partial transfer
Posted by Sebastian Urban 3 weeks, 1 day ago
When a gadget request is only partially transferred in transfer()
because the per-frame bandwidth budget is exhausted, the loop advances
to the next queued request. If that next request is a zero-length
packet (ZLP), len evaluates to zero and the code takes the
unlikely(len == 0) path, which sets is_short = 1. This bypasses the
bandwidth guard ("limit < ep->ep.maxpacket && limit < len") that
lives in the else branch and would otherwise break out of the loop for
non-zero requests. The is_short path then completes the URB before all
data from the first request has been transferred.

Reproducer (bulk IN, high speed):

  Device side (FunctionFS with Linux AIO):
    1. Queue a 65024-byte write via io_submit (127 * 512, i.e. a
       multiple of the HS bulk max packet size).
    2. Immediately queue a zero-length write (ZLP) via io_submit.

  Host side:
    3. Submit a 65536-byte bulk IN URB.

  Expected: URB completes with actual_length = 65024.
  Actual:   URB completes with actual_length = 53248, losing 11776
            bytes that leak into subsequent URBs.

At high speed the per-frame budget is 53248 bytes (512 * 13 * 8).
The 65024-byte request exhausts this budget after 53248 bytes, leaving
the request incomplete (req->req.actual < req->req.length). Neither
the request nor the URB is finished, and rescan is 0, so the loop
advances to the ZLP. For the ZLP, dev_len = 0, so len = min(12288, 0)
= 0, taking the unlikely(len == 0) path and setting is_short = 1.
The is_short handler then sets *status = 0, completing the URB with
only 53248 of the expected 65024 bytes.

Fix this by breaking out of the loop when the current request has
remaining data (req->req.actual < req->req.length). The request
resumes on the next timer tick, preserving correct data ordering.

Signed-off-by: Sebastian Urban <surban@surban.net>
---
Changes in v2:
  - Rewrote commit message to describe the specific ZLP-after-partial-transfer
    scenario rather than overstating the general case (Alan Stern).
  - Added reproducer and step-by-step code path walkthrough.

 drivers/usb/gadget/udc/dummy_hcd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 1cefca660..0eead4a85 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1534,6 +1534,12 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb *urb,
 		/* rescan to continue with any other queued i/o */
 		if (rescan)
 			goto top;
+
+		/* request not fully transferred; stop iterating to
+		 * preserve data ordering across queued requests.
+		 */
+		if (req->req.actual < req->req.length)
+			break;
 	}
 	return sent;
 }
--
2.53.0
Re: [PATCH v2] usb: gadget: dummy_hcd: fix premature URB completion when ZLP follows partial transfer
Posted by Alan Stern 3 weeks, 1 day ago
On Sun, Mar 15, 2026 at 04:10:45PM +0100, Sebastian Urban wrote:
> When a gadget request is only partially transferred in transfer()
> because the per-frame bandwidth budget is exhausted, the loop advances
> to the next queued request. If that next request is a zero-length
> packet (ZLP), len evaluates to zero and the code takes the
> unlikely(len == 0) path, which sets is_short = 1. This bypasses the
> bandwidth guard ("limit < ep->ep.maxpacket && limit < len") that
> lives in the else branch and would otherwise break out of the loop for
> non-zero requests. The is_short path then completes the URB before all
> data from the first request has been transferred.
> 
> Reproducer (bulk IN, high speed):
> 
>   Device side (FunctionFS with Linux AIO):
>     1. Queue a 65024-byte write via io_submit (127 * 512, i.e. a
>        multiple of the HS bulk max packet size).
>     2. Immediately queue a zero-length write (ZLP) via io_submit.
> 
>   Host side:
>     3. Submit a 65536-byte bulk IN URB.
> 
>   Expected: URB completes with actual_length = 65024.
>   Actual:   URB completes with actual_length = 53248, losing 11776
>             bytes that leak into subsequent URBs.
> 
> At high speed the per-frame budget is 53248 bytes (512 * 13 * 8).
> The 65024-byte request exhausts this budget after 53248 bytes, leaving
> the request incomplete (req->req.actual < req->req.length). Neither
> the request nor the URB is finished, and rescan is 0, so the loop
> advances to the ZLP. For the ZLP, dev_len = 0, so len = min(12288, 0)
> = 0, taking the unlikely(len == 0) path and setting is_short = 1.
> The is_short handler then sets *status = 0, completing the URB with
> only 53248 of the expected 65024 bytes.
> 
> Fix this by breaking out of the loop when the current request has
> remaining data (req->req.actual < req->req.length). The request
> resumes on the next timer tick, preserving correct data ordering.
> 
> Signed-off-by: Sebastian Urban <surban@surban.net>
> ---

Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

> Changes in v2:
>   - Rewrote commit message to describe the specific ZLP-after-partial-transfer
>     scenario rather than overstating the general case (Alan Stern).
>   - Added reproducer and step-by-step code path walkthrough.
> 
>  drivers/usb/gadget/udc/dummy_hcd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index 1cefca660..0eead4a85 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1534,6 +1534,12 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb *urb,
>  		/* rescan to continue with any other queued i/o */
>  		if (rescan)
>  			goto top;
> +
> +		/* request not fully transferred; stop iterating to
> +		 * preserve data ordering across queued requests.
> +		 */
> +		if (req->req.actual < req->req.length)
> +			break;
>  	}
>  	return sent;
>  }
> --
> 2.53.0