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

[Minseo Kim]

Hi Alan,

> I wonder which part of the code was holding udc_usage above 0?

It was the udc_usage increment in ep_aio() during request submission.
The increment occurs before dev->lock is released to call
kiocb_set_cancel_fn() and usb_ep_queue(), and the corresponding decrement
occurs after usb_ep_queue() returns:

    ++epdata->dev->udc_usage;
    spin_unlock_irq(&epdata->dev->lock);

    kiocb_set_cancel_fn(iocb, ep_aio_cancel);
    value = usb_ep_queue(ep, req, GFP_KERNEL);

    spin_lock_irq(&epdata->dev->lock);
    --epdata->dev->udc_usage;

In the matched control, a request with a length of 64 bytes took the
direct giveback path, where dummy_queue() calls
usb_gadget_giveback_request() before returning to usb_ep_queue().
usb_gadget_giveback_request() synchronously invokes the request's
completion callback, which was ep_aio_complete() in this test. Because I
held ep_aio_complete() at the diagnostic gate immediately after
iocb->ki_complete() returned, the nested giveback call could not return.
Consequently, neither dummy_queue() nor usb_ep_queue() had returned, and
ep_aio() had not reached the decrement. gadgetfs_unbind() therefore saw
udc_usage at 1 and waited.

In the normal transfer test, by contrast, usb_ep_queue() returned and
ep_aio() decremented udc_usage before the later completion callback ran.
I confirmed this ordering in three runs without enabling a callback
gate.

I verified the sequence for direct giveback with markers after the
increment, after usb_ep_queue() returned but before the decrement, after
the decrement, and immediately before and after the unbind wait. In each
of three runs, markers with the same dev pointer showed ep_aio()
incrementing udc_usage to 1 and gadgetfs_unbind() entering the udc_usage
wait with the count still at 1. After I released the diagnostic gate,
usb_ep_queue() returned, ep_aio() decremented the count to 0, and
gadgetfs_unbind() left the wait.

> I will submit the patches, with your Signed-off-by:
> added to this one also, if that's okay with you.

Yes, that is okay with me. Please add it as:

Minseo Kim <neck3922@xxxxxxxxx>

Thank you for taking this forward.

With sincere appreciation and great respect,
Minseo Kim

2026년 9월 18일 (금) 오전 12:38, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>님이 작성:
>
> On Thu, Sep 17, 2026 at 11:02:13PM +0900, Minseo Kim wrote:
> > 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").
>
> Okay.  I recently rebased my kernel tree to v7.3-rc3, so a few
> discrepancies are to be expected.
>
> > 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]
>
> That is just as it should be.
>
> dummy-hcd emulates a UDC driver.  With a real driver, givebacks would be
> triggered by a device interrupt (signalling completion of a request) and
> the synchronize_irq() call in gadget_unbind_driver() would wait until
> outstanding calls to the IRQ handler (and thus the completion handler)
> had completed.  But dummy-hcd doesn't have real hardware, so instead of
> device interrupts it relies on timer interrupts and its
> dummy_ucd_async_callbacks() routine is supposed to emulate
> synchronize_irq().  Thus it should wait until all outstanding completion
> handler calls have completed.
>
> > 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 wonder which part of the code was holding udc_usage above 0?
>
> > 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.
>
> It all sounds good.  I will submit the patches, with your Signed-off-by:
> added to this one also, if that's okay with you.
>
> Alan Stern