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

[Alan Stern]

On Fri, Aug 28, 2026 at 03:30:10PM -0500, neck3922@xxxxxxxxx wrote:
> Hi Alan,
> 
> Thank you for the revised patch and for explaining the intended use of
> the saved endpoint.
> 
> I applied the patch as posted to upstream v7.2-rc1, commit
> dc59e4fea9d83f03bad6bddf3fa2e52491777482.
> 
> I first tested your revision unchanged. The IRQ-state fix and saved
> endpoint behaved as intended. The exact pre-queue cancellation matrix
> produced exactly one expected completion in every case, with no KASAN,
> Oops, or LOCKDEP output. The original ep_aio_cancel() null-ptr-deref
> and UAF signatures, the earlier ep_aio() submit-path regressions, and the
> teardown NULL-endpoint failure did not recur in these tests.

All good results.

> > I think it will still be necessary to flush the workqueue after
> > destroy_ep_files() runs.  The best way to check whether this is needed
> > would be to put a long delay right at the start of ep_unlink_worker()
> > and then run a test where during that delay, the test program closes its
> > open files, unmounts the directory, and unloads the gadgetfs module.
> 
> Using CONFIG_USB_GADGETFS=m on the posted revision, I inserted a
> diagnostic 15-second delay at the entry of ep_unlink_worker(). While the
> worker was delayed, the test process had no open /dev/gadget/* file
> descriptors, GadgetFS unmounted successfully, the gadgetfs module
> reference count reached zero, and rmmod succeeded before the delay ended.
> The delay was diagnostic only.
> 
> When the delayed worker resumed, the kernel warned and then panicked. The
> serial log showed an unresolved work-function address and identified
> gadgetfs as the last unloaded module:
> 
>   Modules linked in: dummy_hcd [last unloaded: gadgetfs(O)]
> 
> Thus, in this path, a running unlink_work item does not pin the gadgetfs
> module. Teardown must quiesce pending or running GadgetFS AIO work before
> module unload, or otherwise retain the module until that work completes.

Indeed.  Quiescing won't be easy to do; it will just have to wait until 
the work is complete.  In other words, flush the workqueue.

> While rerunning the directed cross-CPU test from my previous message, I
> found that the underlying lifetime race involving ep_unlink_worker()
> remained:
> 
>   BUG: KASAN: slab-use-after-free in ep_unlink_worker
>   Read of size 8
>   drivers/usb/gadget/legacy/inode.c:489
> 
> The faulting statement was:
> 
>   put_ep(priv->epdata);
> 
> After AIO_UNLINK_DONE was published, completion work could free priv
> before the unlink worker's final accesses through priv. Saving epdata and
> ep before usb_ep_dequeue() eliminated the failure in the directed test.

Ah, yes.  I should have realized that last time.  I just didn't think 
hard enough about how the ordering could interact with those tests at 
the end of ep_unlink_worker().  (Or maybe I did and then forgot to 
include the changes into the patch -- I can't remember.)  Anyway, my 
version of the patch has been updated accordingly.

> Based on our discussion and the results above, I prepared the cumulative
> patch below, incorporating your latest revision. It also fixes an
> existing native PREAD issue: the to_free predicate could skip copying an
> ITER_UBUF payload while still reporting success.

I'm not sure what you're referring to.  Are you saying that this test
in ep_read_iter():

		if (!iter_is_ubuf(&priv->to) && !priv->to_free) {

is wrong, for example, the && should be || ?  In fact, I don't 
understand the reason for the iter_is_ubuf() check at all.

Note that to_free isn't a predicate; rather it's a pointer to a copy of 
an iov_iter structure.

> During development and validation, I found two further ordering issues.
> In the existing completion ordering, request cleanup could overlap
> usb_ep_disable() during final endpoint release.

Yes, I know.  I understood that it was okay to call 
usb_ep_free_request() after usb_ep_disable().  Was that wrong?  Did you 
see it create any problems?

Note that usb_ep_disable() doesn't return until all the requests queued 
for that endpoint have completed.  So ep_aio_complete() will have run, 
but the work routines may still be pending.

>  Separately, an
> intermediate cumulative version allowed gadgetfs_unbind() to pass the
> completion-work flush before a callback had queued that work.

This is probably because you were flushing the workqueues at the wrong
time.

For the final submission, I think the workqueue management stuff should 
go into its own separate patch.  Straightening out the various AIO races 
is already complicated enough by itself.

>  The
> resulting patch addresses all three issues and applies directly to
> upstream v7.2-rc1.

I'll review the patch later.  For now, there's two things to mention.
First, when you create your patches, it would help to add the -p
option to the diff command.

Second, why did you change ep_aio_complete() to make it queue up
ep_user_copy_worker() even when nothing needed to be copied to
userspace?  It's a bad idea to run a workqueue routine if it isn't
necessary.

Alan Stern