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

[Alan Stern]

On Tue, Jul 14, 2026 at 02:48:29PM +0800, 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, set across the lockless giveback window via a
> dummy_giveback() helper used at all four gadget-request giveback sites,
> and require !fifo_req_busy in the FIFO fast-path guard so the shared
> slot cannot be reused until its completion callback has returned.
> 
> Reported-by: syzbot+faf3a6cf579fc65591ca@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=faf3a6cf579fc65591ca
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jinchao Wang <wangjinchao600@xxxxxxxxx>

Wow!  I'm impressed.  How did you figure this out?

> ---
>  drivers/usb/gadget/udc/dummy_hcd.c | 40 +++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index f47903461ed5..fce3c3ba7a63 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
> @@ -330,6 +331,28 @@ static inline struct dummy *gadget_dev_to_dummy(struct device *dev)
>  /* DEVICE/GADGET SIDE UTILITY ROUTINES */
>  
>  /* called with spinlock held */

That comment line is supposed to come immediately before nuke().  Your 
new code got inserted below the comment instead of above it.

> +/*
> + * Give back a gadget request with dum->lock dropped around the callback.
> + * If @req is the shared fifo_req, mark it busy across the callback so
> + * dummy_queue()'s FIFO fast-path (keyed on list_empty(&fifo_req.queue))
> + * cannot reuse it mid-giveback: list_del_init() already made the queue look
> + * empty, but the request is in flight until the completion callback returns.
> + * 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;
> +
> +	if (fifo)
> +		dum->fifo_req_busy = 1;

Don't set the new flag here...

> +	spin_unlock(&dum->lock);
> +	usb_gadget_giveback_request(_ep, &req->req);
> +	spin_lock(&dum->lock);
> +	if (fifo)
> +		dum->fifo_req_busy = 0;
> +}
> +
>  static void nuke(struct dummy *dum, struct dummy_ep *ep)
>  {
>  	while (!list_empty(&ep->queue)) {

> @@ -729,6 +750,7 @@ 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;

Set it here instead, so the flag is set during the entire time that 
dum->fifo_req is in use.  As a bonus, you can then remove the 
list_empty(&dum->fifo_req.queue) test above.

Otherwise this seems fine.

Alan Stern