* [PATCH] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback
@ 2026-07-14 6:48 Jinchao Wang
2026-07-14 21:13 ` Alan Stern
2026-07-16 10:42 ` [PATCH v2] " Jinchao Wang
0 siblings, 2 replies; 7+ messages in thread
From: Jinchao Wang @ 2026-07-14 6:48 UTC (permalink / raw)
To: gregkh, linux-usb
Cc: stern, bigeasy, eeodqql09, kees, surban, linux-kernel,
syzkaller-bugs, stable, wangjinchao600
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@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>
---
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 */
+/*
+ * 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;
+ 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)) {
@@ -339,9 +362,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);
}
}
@@ -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;
@@ -785,9 +807,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 +1543,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 +1928,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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback
2026-07-14 6:48 [PATCH] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback Jinchao Wang
@ 2026-07-14 21:13 ` Alan Stern
2026-07-15 5:34 ` Jinchao Wang
2026-07-16 10:42 ` [PATCH v2] " Jinchao Wang
1 sibling, 1 reply; 7+ messages in thread
From: Alan Stern @ 2026-07-14 21:13 UTC (permalink / raw)
To: Jinchao Wang
Cc: gregkh, linux-usb, bigeasy, eeodqql09, kees, surban,
linux-kernel, syzkaller-bugs, stable
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@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>
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback
2026-07-14 21:13 ` Alan Stern
@ 2026-07-15 5:34 ` Jinchao Wang
2026-07-17 2:40 ` Alan Stern
0 siblings, 1 reply; 7+ messages in thread
From: Jinchao Wang @ 2026-07-15 5:34 UTC (permalink / raw)
To: Alan Stern
Cc: gregkh, linux-usb, bigeasy, eeodqql09, kees, surban,
linux-kernel, syzkaller-bugs, stable
On 7/14/2026 5:13 PM, Alan Stern wrote:
> 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@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>
>
> Wow! I'm impressed. How did you figure this out?
With a hardware watchpoint: I armed one on the victim field
(req->complete, at arg2+56 of usb_gadget_giveback_request) only while
usb_gadget_giveback_request() was running, and it caught the writing
memcpy with a full stack - usb_ep_queue <- raw_process_ep_io <-
raw_ioctl - on the same request that crashed an instant later.
The watchpoint setup came from a small tool I am working on; I posted
it as an RFC in case it is useful to others:
https://lore.kernel.org/all/20260714182243.10687-1-wangjinchao600@gmail.com/
>
>> ---
>> 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.
Right, will fix in v2.
>
>> +/*
>> + * 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.
Indeed better - the flag then covers the whole lifetime of the shared
request instead of just the giveback window. Will do in v2, with the
list_empty() test removed.
> Otherwise this seems fine.
Thanks for the review, v2 shortly.
Thanks,
Jinchao
>
> Alan Stern
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback
2026-07-14 6:48 [PATCH] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback Jinchao Wang
2026-07-14 21:13 ` Alan Stern
@ 2026-07-16 10:42 ` Jinchao Wang
2026-07-16 13:50 ` Alan Stern
1 sibling, 1 reply; 7+ messages in thread
From: Jinchao Wang @ 2026-07-16 10:42 UTC (permalink / raw)
To: gregkh, stern
Cc: linux-usb, linux-kernel, eeodqql09, surban, bigeasy, kees,
Jinchao Wang, syzbot+faf3a6cf579fc65591ca, stable
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback
2026-07-16 10:42 ` [PATCH v2] " Jinchao Wang
@ 2026-07-16 13:50 ` Alan Stern
0 siblings, 0 replies; 7+ messages in thread
From: Alan Stern @ 2026-07-16 13:50 UTC (permalink / raw)
To: Jinchao Wang
Cc: gregkh, linux-usb, linux-kernel, eeodqql09, surban, bigeasy,
kees, syzbot+faf3a6cf579fc65591ca, stable
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback
2026-07-15 5:34 ` Jinchao Wang
@ 2026-07-17 2:40 ` Alan Stern
2026-07-17 2:47 ` Jinchao Wang
0 siblings, 1 reply; 7+ messages in thread
From: Alan Stern @ 2026-07-17 2:40 UTC (permalink / raw)
To: Jinchao Wang
Cc: gregkh, linux-usb, bigeasy, eeodqql09, kees, surban,
linux-kernel, syzkaller-bugs, stable
On Wed, Jul 15, 2026 at 01:34:13AM -0400, Jinchao Wang wrote:
> > Wow! I'm impressed. How did you figure this out?
>
> With a hardware watchpoint: I armed one on the victim field
> (req->complete, at arg2+56 of usb_gadget_giveback_request) only while
> usb_gadget_giveback_request() was running, and it caught the writing
> memcpy with a full stack - usb_ep_queue <- raw_process_ep_io <-
> raw_ioctl - on the same request that crashed an instant later.
>
> The watchpoint setup came from a small tool I am working on; I posted
> it as an RFC in case it is useful to others:
>
> https://lore.kernel.org/all/20260714182243.10687-1-wangjinchao600@gmail.com/
That tool looks really nice. I'm glad to see hardware breakpoints
becoming easier to use, because I wrote a large part of the initial
implementation -- see for example commit 0067f1297241 ("hw-breakpoints:
x86 architecture implementation of Hardware Breakpoint interfaces").
Alan Stern
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback
2026-07-17 2:40 ` Alan Stern
@ 2026-07-17 2:47 ` Jinchao Wang
0 siblings, 0 replies; 7+ messages in thread
From: Jinchao Wang @ 2026-07-17 2:47 UTC (permalink / raw)
To: Alan Stern
Cc: gregkh, linux-usb, bigeasy, eeodqql09, kees, surban,
linux-kernel, syzkaller-bugs, stable
On 7/16/2026 10:40 PM, Alan Stern wrote:
> On Wed, Jul 15, 2026 at 01:34:13AM -0400, Jinchao Wang wrote:
>>> Wow! I'm impressed. How did you figure this out?
>>
>> With a hardware watchpoint: I armed one on the victim field
>> (req->complete, at arg2+56 of usb_gadget_giveback_request) only while
>> usb_gadget_giveback_request() was running, and it caught the writing
>> memcpy with a full stack - usb_ep_queue <- raw_process_ep_io <-
>> raw_ioctl - on the same request that crashed an instant later.
>>
>> The watchpoint setup came from a small tool I am working on; I posted
>> it as an RFC in case it is useful to others:
>>
>> https://lore.kernel.org/all/20260714182243.10687-1-wangjinchao600@gmail.com/
>
> That tool looks really nice. I'm glad to see hardware breakpoints
> becoming easier to use, because I wrote a large part of the initial
> implementation -- see for example commit 0067f1297241 ("hw-breakpoints:
> x86 architecture implementation of Hardware Breakpoint interfaces").
>
> Alan Stern
Thank you! KWatch builds directly on that work - the perf hw_breakpoint
layer does all the heavy lifting, and KWatch only adds a small
"reinstall" operation on top, plus the windowing logic around it.
I am preparing a v2 of the RFC based on the review so far; if you don't
mind, I will Cc you on it.
Thanks,
Jinchao
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-07-17 2:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-14 6:48 [PATCH] usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback Jinchao Wang
2026-07-14 21:13 ` Alan Stern
2026-07-15 5:34 ` Jinchao Wang
2026-07-17 2:40 ` Alan Stern
2026-07-17 2:47 ` Jinchao Wang
2026-07-16 10:42 ` [PATCH v2] " Jinchao Wang
2026-07-16 13:50 ` Alan Stern
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome