1. Patchew
  2. linux
  3. View series

[PATCH v2] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback

Jinchao Wang posted 1 patch 1 week, 2 days ago 
drivers/usb/gadget/udc/dummy_hcd.c | 40 ++++++++++++++++++++----------
1 file changed, 27 insertions(+), 13 deletions(-)
[PATCH v2] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback
Posted by Jinchao Wang 1 week, 2 days ago 
dummy_hcd embeds a single shared usb_request (dum->fifo_req) that the
"emulated single-request FIFO" fast-path in dummy_queue() reuses for
small IN transfers: it copies the caller's request into it
(req->req = *_req) and queues it, treating list_empty(&fifo_req.queue)
as "the slot is free".

The completion side (dummy_timer/transfer/nuke/dummy_dequeue) follows
the standard pattern: list_del_init(&req->queue) unlinks the request,
then the lock is dropped and usb_gadget_giveback_request() invokes
req->complete().  But list_del_init() makes fifo_req.queue look empty
*before* the completion callback returns, so a concurrent dummy_queue()
on another CPU sees the slot as free, reuses fifo_req and runs
req->req = *_req -- overwriting req->complete while dummy_timer is
mid-calling it.  The indirect call then jumps to a clobbered pointer,
causing a general protection fault / page fault in dummy_timer
(syzkaller extid faf3a6cf579fc65591ca).  The clobbering write is an
in-bounds memcpy on a live shared object, so KASAN cannot flag it.

Add a fifo_req_busy bit covering the shared request's whole lifetime:
set it in dummy_queue() when the FIFO fast-path takes fifo_req (making
it the fast-path guard, replacing the list_empty(&fifo_req.queue)
test), and clear it after the completion callback has returned, via a
dummy_giveback() helper used at all four gadget-request giveback
sites.  The shared slot can no longer be reused until its completion
callback has finished.

Reported-by: syzbot+faf3a6cf579fc65591ca@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=faf3a6cf579fc65591ca
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
Changes in v2 (per Alan Stern's review of v1 [1]):

- Move dummy_giveback() above the "/* called with spinlock held */"
  comment so the comment is again immediately before nuke().
- Set fifo_req_busy in dummy_queue() when the FIFO fast-path takes the
  shared request (covering its whole in-use lifetime) instead of in
  dummy_giveback(), and drop the now-redundant
  list_empty(&fifo_req.queue) test from the fast-path guard.

[1] https://lore.kernel.org/all/20260714064829.172098-1-wangjinchao600@gmail.com/

 drivers/usb/gadget/udc/dummy_hcd.c | 40 ++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index f47903461ed5..c0e40fa6dde5 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -278,6 +278,7 @@ struct dummy {
 	unsigned			ints_enabled:1;
 	unsigned			udc_suspended:1;
 	unsigned			pullup:1;
+	unsigned			fifo_req_busy:1;
 
 	/*
 	 * HOST side support
@@ -329,6 +330,26 @@ static inline struct dummy *gadget_dev_to_dummy(struct device *dev)
 
 /* DEVICE/GADGET SIDE UTILITY ROUTINES */
 
+/*
+ * Give back a gadget request with dum->lock dropped around the callback.
+ * If @req is the shared fifo_req, clear fifo_req_busy afterward: the flag
+ * was set in dummy_queue() when the shared request was taken and must stay
+ * set until its completion callback has returned; list_del_init() alone
+ * makes the request look idle while the callback is still running.
+ * Caller holds dum->lock and has already done list_del_init() + status.
+ */
+static void dummy_giveback(struct dummy *dum, struct usb_ep *_ep,
+			   struct dummy_request *req)
+{
+	bool fifo = req == &dum->fifo_req;
+
+	spin_unlock(&dum->lock);
+	usb_gadget_giveback_request(_ep, &req->req);
+	spin_lock(&dum->lock);
+	if (fifo)
+		dum->fifo_req_busy = 0;
+}
+
 /* called with spinlock held */
 static void nuke(struct dummy *dum, struct dummy_ep *ep)
 {
@@ -339,9 +360,7 @@ static void nuke(struct dummy *dum, struct dummy_ep *ep)
 		list_del_init(&req->queue);
 		req->req.status = -ESHUTDOWN;
 
-		spin_unlock(&dum->lock);
-		usb_gadget_giveback_request(&ep->ep, &req->req);
-		spin_lock(&dum->lock);
+		dummy_giveback(dum, &ep->ep, req);
 	}
 }
 
@@ -728,10 +747,11 @@ static int dummy_queue(struct usb_ep *_ep, struct usb_request *_req,
 
 	/* implement an emulated single-request FIFO */
 	if (ep->desc && (ep->desc->bEndpointAddress & USB_DIR_IN) &&
-			list_empty(&dum->fifo_req.queue) &&
+			!dum->fifo_req_busy &&
 			list_empty(&ep->queue) &&
 			_req->length <= FIFO_SIZE) {
 		req = &dum->fifo_req;
+		dum->fifo_req_busy = 1;
 		req->req = *_req;
 		req->req.buf = dum->fifo_buf;
 		memcpy(dum->fifo_buf, _req->buf, _req->length);
@@ -785,9 +805,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 		dev_dbg(udc_dev(dum),
 				"dequeued req %p from %s, len %d buf %p\n",
 				req, _ep->name, _req->length, _req->buf);
-		spin_unlock(&dum->lock);
-		usb_gadget_giveback_request(_ep, _req);
-		spin_lock(&dum->lock);
+		dummy_giveback(dum, _ep, req);
 	}
 	spin_unlock_irqrestore(&dum->lock, flags);
 	return retval;
@@ -1523,9 +1541,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb *urb,
 		if (req->req.status != -EINPROGRESS) {
 			list_del_init(&req->queue);
 
-			spin_unlock(&dum->lock);
-			usb_gadget_giveback_request(&ep->ep, &req->req);
-			spin_lock(&dum->lock);
+			dummy_giveback(dum, &ep->ep, req);
 
 			/* requests might have been unlinked... */
 			rescan = 1;
@@ -1910,9 +1926,7 @@ static enum hrtimer_restart dummy_timer(struct hrtimer *t)
 				dev_dbg(udc_dev(dum), "stale req = %p\n",
 						req);
 
-				spin_unlock(&dum->lock);
-				usb_gadget_giveback_request(&ep->ep, &req->req);
-				spin_lock(&dum->lock);
+				dummy_giveback(dum, &ep->ep, req);
 				ep->already_seen = 0;
 				goto restart;
 			}
-- 
2.53.0
Re: [PATCH v2] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback
Posted by Alan Stern 1 week, 2 days ago 
On Thu, Jul 16, 2026 at 06:42:17AM -0400, Jinchao Wang wrote:
> dummy_hcd embeds a single shared usb_request (dum->fifo_req) that the
> "emulated single-request FIFO" fast-path in dummy_queue() reuses for
> small IN transfers: it copies the caller's request into it
> (req->req = *_req) and queues it, treating list_empty(&fifo_req.queue)
> as "the slot is free".
> 
> The completion side (dummy_timer/transfer/nuke/dummy_dequeue) follows
> the standard pattern: list_del_init(&req->queue) unlinks the request,
> then the lock is dropped and usb_gadget_giveback_request() invokes
> req->complete().  But list_del_init() makes fifo_req.queue look empty
> *before* the completion callback returns, so a concurrent dummy_queue()
> on another CPU sees the slot as free, reuses fifo_req and runs
> req->req = *_req -- overwriting req->complete while dummy_timer is
> mid-calling it.  The indirect call then jumps to a clobbered pointer,
> causing a general protection fault / page fault in dummy_timer
> (syzkaller extid faf3a6cf579fc65591ca).  The clobbering write is an
> in-bounds memcpy on a live shared object, so KASAN cannot flag it.
> 
> Add a fifo_req_busy bit covering the shared request's whole lifetime:
> set it in dummy_queue() when the FIFO fast-path takes fifo_req (making
> it the fast-path guard, replacing the list_empty(&fifo_req.queue)
> test), and clear it after the completion callback has returned, via a
> dummy_giveback() helper used at all four gadget-request giveback
> sites.  The shared slot can no longer be reused until its completion
> callback has finished.
> 
> Reported-by: syzbot+faf3a6cf579fc65591ca@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=faf3a6cf579fc65591ca
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
> Changes in v2 (per Alan Stern's review of v1 [1]):
> 
> - Move dummy_giveback() above the "/* called with spinlock held */"
>   comment so the comment is again immediately before nuke().
> - Set fifo_req_busy in dummy_queue() when the FIFO fast-path takes the
>   shared request (covering its whole in-use lifetime) instead of in
>   dummy_giveback(), and drop the now-redundant
>   list_empty(&fifo_req.queue) test from the fast-path guard.
> 
> [1] https://lore.kernel.org/all/20260714064829.172098-1-wangjinchao600@gmail.com/

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

>  drivers/usb/gadget/udc/dummy_hcd.c | 40 ++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index f47903461ed5..c0e40fa6dde5 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -278,6 +278,7 @@ struct dummy {
>  	unsigned			ints_enabled:1;
>  	unsigned			udc_suspended:1;
>  	unsigned			pullup:1;
> +	unsigned			fifo_req_busy:1;
>  
>  	/*
>  	 * HOST side support
> @@ -329,6 +330,26 @@ static inline struct dummy *gadget_dev_to_dummy(struct device *dev)
>  
>  /* DEVICE/GADGET SIDE UTILITY ROUTINES */
>  
> +/*
> + * Give back a gadget request with dum->lock dropped around the callback.
> + * If @req is the shared fifo_req, clear fifo_req_busy afterward: the flag
> + * was set in dummy_queue() when the shared request was taken and must stay
> + * set until its completion callback has returned; list_del_init() alone
> + * makes the request look idle while the callback is still running.
> + * Caller holds dum->lock and has already done list_del_init() + status.
> + */
> +static void dummy_giveback(struct dummy *dum, struct usb_ep *_ep,
> +			   struct dummy_request *req)
> +{
> +	bool fifo = req == &dum->fifo_req;
> +
> +	spin_unlock(&dum->lock);
> +	usb_gadget_giveback_request(_ep, &req->req);
> +	spin_lock(&dum->lock);
> +	if (fifo)
> +		dum->fifo_req_busy = 0;
> +}
> +
>  /* called with spinlock held */
>  static void nuke(struct dummy *dum, struct dummy_ep *ep)
>  {
> @@ -339,9 +360,7 @@ static void nuke(struct dummy *dum, struct dummy_ep *ep)
>  		list_del_init(&req->queue);
>  		req->req.status = -ESHUTDOWN;
>  
> -		spin_unlock(&dum->lock);
> -		usb_gadget_giveback_request(&ep->ep, &req->req);
> -		spin_lock(&dum->lock);
> +		dummy_giveback(dum, &ep->ep, req);
>  	}
>  }
>  
> @@ -728,10 +747,11 @@ static int dummy_queue(struct usb_ep *_ep, struct usb_request *_req,
>  
>  	/* implement an emulated single-request FIFO */
>  	if (ep->desc && (ep->desc->bEndpointAddress & USB_DIR_IN) &&
> -			list_empty(&dum->fifo_req.queue) &&
> +			!dum->fifo_req_busy &&
>  			list_empty(&ep->queue) &&
>  			_req->length <= FIFO_SIZE) {
>  		req = &dum->fifo_req;
> +		dum->fifo_req_busy = 1;
>  		req->req = *_req;
>  		req->req.buf = dum->fifo_buf;
>  		memcpy(dum->fifo_buf, _req->buf, _req->length);
> @@ -785,9 +805,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>  		dev_dbg(udc_dev(dum),
>  				"dequeued req %p from %s, len %d buf %p\n",
>  				req, _ep->name, _req->length, _req->buf);
> -		spin_unlock(&dum->lock);
> -		usb_gadget_giveback_request(_ep, _req);
> -		spin_lock(&dum->lock);
> +		dummy_giveback(dum, _ep, req);
>  	}
>  	spin_unlock_irqrestore(&dum->lock, flags);
>  	return retval;
> @@ -1523,9 +1541,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb *urb,
>  		if (req->req.status != -EINPROGRESS) {
>  			list_del_init(&req->queue);
>  
> -			spin_unlock(&dum->lock);
> -			usb_gadget_giveback_request(&ep->ep, &req->req);
> -			spin_lock(&dum->lock);
> +			dummy_giveback(dum, &ep->ep, req);
>  
>  			/* requests might have been unlinked... */
>  			rescan = 1;
> @@ -1910,9 +1926,7 @@ static enum hrtimer_restart dummy_timer(struct hrtimer *t)
>  				dev_dbg(udc_dev(dum), "stale req = %p\n",
>  						req);
>  
> -				spin_unlock(&dum->lock);
> -				usb_gadget_giveback_request(&ep->ep, &req->req);
> -				spin_lock(&dum->lock);
> +				dummy_giveback(dum, &ep->ep, req);
>  				ep->already_seen = 0;
>  				goto restart;
>  			}
> -- 
> 2.53.0
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.