Re: [BUG] usb: gadgetfs: KASAN null-ptr-deref and intermittent UAF in ep_aio_cancel()

[Minseo Kim]

Hi Alan,

Thank you for the patch.

> Can you repeat these tests with the patch below applied on top
> of the other two?  It should cause usb_gadget_unregister_driver() to
> wait until ep_aio_complete() returns (the actual wait loop is in
> dummy-hcd's dummy_udc_async_callbacks() routine).

Yes. With the new dummy_hcd change applied on top of the two GadgetFS
patches, usb_gadget_unregister_driver() waited in
dummy_udc_async_callbacks() until the held ep_aio_complete() callback
returned in both the normal transfer path through dummy_giveback() and
the direct giveback path in dummy_queue().

I used upstream v7.2-rc1 as the base. Because dummy_giveback() is not
present in v7.2-rc1, both matched test trees also included the changes
from upstream commit d5e5cd3654d2b5359a12ea6586120f05b28634ee
("usb: gadget: dummy_hcd: prevent fifo_req reuse during giveback").

In the normal transfer case, I held ep_aio_complete() immediately after
iocb->ki_complete(). Without the new dummy_hcd change,
dummy_udc_async_callbacks(false) saw callback_usage equal to 0, and the
ep0 close returned while the callback was still held. With the change,
it saw callback_usage equal to 1, and the ep0 close task remained blocked
in dummy_udc_async_callbacks(). The relevant part of the blocked close
task's stack was:

    dummy_udc_async_callbacks [dummy_hcd]
    gadget_unbind_driver
    device_remove
    device_release_driver_internal
    driver_detach
    bus_remove_driver
    driver_unregister
    usb_gadget_unregister_driver
    dev_release [gadgetfs]

After I released the callback gate, ep_aio_complete() returned,
callback_usage fell to 0, and the same close completed. I observed this
sequence in four separate runs with the new change.

To exercise the other accounting site, I separately tested the direct
giveback path for a 64-byte write in dummy_queue(). With the change, the
close task again waited in dummy_udc_async_callbacks() with callback_usage
equal to 1. In the matched control, dummy_udc_async_callbacks() returned
with callback_usage at 0, and gadgetfs_unbind() then remained in its
existing udc_usage wait until I released the callback gate.

I also repeated the case where the request was still on the endpoint
queue when usb_ep_disable() began. Neither usb_ep_disable() nor the ep0
close returned while the completion callback was held. After I released
the callback gate, io_getevents() returned one completion with
res=-ESHUTDOWN, and a subsequent check returned no additional completion
event.

With sincere appreciation and great respect,
Minseo Kim

2026년 9월 17일 (목) 오전 4:39, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>님이 작성:
>
> On Wed, Sep 16, 2026 at 11:00:00PM +0900, Minseo Kim wrote:
> > Hi Alan,
> >
> > > Hmmm.  Do you know where the unmount operation was getting stuck?  Was
> > > it the usb_gadget_unregister_driver() call inside dev_release()?  I just
> > > want to be sure about this.
> >
> > In these reruns, I found that the unmount task was blocked in
> > synchronize_rcu_expedited(), called from namespace_unlock(), rather than
> > in usb_gadget_unregister_driver().
> >
> > I reran the PWRITE callback tail test with both patches applied, using a
> > resident helper whose main thread invoked umount2() directly. A monitor
> > thread in the helper captured the blocked main thread's kernel stack. In
> > three runs from fresh boots, the relevant frames were:
> >
> >     synchronize_rcu_expedited
> >     namespace_unlock
> >     path_umount
> >     __x64_sys_umount
> >
> > In each of those three runs, the same umount2() call returned
> > successfully after I released the callback gate.
> >
> > USB gadget request completion callbacks run with interrupts disabled, and
> > interrupt-disabled regions act as implicit RCU read-side critical
> > sections. The observed wait is therefore consistent with the diagnostic
> > gate delaying completion of the expedited grace period.
> >
> > I also added diagnostic markers around dev_release() and
> > usb_gadget_unregister_driver(). The test harness signaled the resident
> > helper only after the reproducer had closed ep0. In five clean runs from
> > fresh boots with these markers, both usb_gadget_unregister_driver() and
> > dev_release() had returned before the resident helper invoked umount2(),
> > while ep_aio_complete() was still held immediately after
> > iocb->ki_complete().
>
> Okay, that's not good.  We can't expect to rely on an unintended side
> effect.  All the completion handlers should finish before
> usb_gadget_unregister_driver() returns; we need to enforce that.
>
> I believe this misbehavior is caused by an oversight in the dummy-hcd
> driver.  Can you repeat these tests with the patch below applied on top
> of the other two?  It should cause usb_gadget_unregister_driver() to
> wait until ep_aio_complete() returns (the actual wait loop is in
> dummy-hcd's dummy_udc_async_callbacks() routine).
>
> Alan Stern
>
>
> Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c
> +++ usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -343,9 +343,11 @@ static void dummy_giveback(struct dummy
>  {
>         bool fifo = req == &dum->fifo_req;
>
> +       ++dum->callback_usage;
>         spin_unlock(&dum->lock);
>         usb_gadget_giveback_request(_ep, &req->req);
>         spin_lock(&dum->lock);
> +       --dum->callback_usage;
>         if (fifo)
>                 dum->fifo_req_busy = 0;
>  }
> @@ -759,11 +761,13 @@ static int dummy_queue(struct usb_ep *_e
>                 req->req.complete = fifo_complete;
>
>                 list_add_tail(&req->queue, &ep->queue);
> +               ++dum->callback_usage;
>                 spin_unlock(&dum->lock);
>                 _req->actual = _req->length;
>                 _req->status = 0;
>                 usb_gadget_giveback_request(_ep, _req);
>                 spin_lock(&dum->lock);
> +               --dum->callback_usage;
>         }  else
>                 list_add_tail(&req->queue, &ep->queue);
>         spin_unlock_irqrestore(&dum->lock, flags);
>