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

[Minseo Kim]

Hi Alan,

Thank you for the second patch.

> In your earlier testing, didn't you find that with the unpatched driver,
> the driver module's usage count remained elevated as long as the
> ep_user_copy_worker() was pending on the workqueue? And therefore it
> was impossible to unload the module while a work item was still queued
> or running?

To check that point directly, I tested the unpatched upstream copy-work
path using diagnostic gates at two points in ep_user_copy_worker(). The
module usage count remained elevated when I held the worker at entry,
before iocb->ki_complete(). When I held the worker after that call, the
usage count reached zero and the module was unloaded before the worker
finished; the kernel then warned and panicked. This suggests that
AIO may not retain the file reference throughout the worker tail after
ki_complete().

> Could you test it and verify that it prevents the problem you observed
> with only the first patch installed?

Yes. I applied your first patch and then your second patch to upstream
v7.2-rc1, commit dc59e4fea9d83f03bad6bddf3fa2e52491777482.

With the first patch alone, I reproduced the earlier module-unload
failure with ep_unlink_worker() held at entry. With both patches applied
using the same reproducer and ep_unlink_worker() entry gate,
gadgetfs_unbind() reached the flush point and the ep0 close remained
blocked, while unmount and rmmod attempts were rejected. After I released
the worker, the close completed and subsequent unmount and rmmod attempts
succeeded. Holding ep_user_copy_worker() at entry with both patches
applied produced the same result.

I also tested the boundary where gadgetfs_unbind() reached the flush
point before the completion callback queued copy_work. After the
callback queued the work, I held the worker tail immediately after
iocb->ki_complete(). In this ordering, rmmod did not complete while the
worker was held; after I released the worker, rmmod completed cleanly.

I reran the exact pre-queue cancellation matrix, the original
null-ptr-deref reproducer, the directed cross-CPU UAF reproducer,
deferred giveback, forced dequeue failure, the earlier NULL-endpoint
case, payload checks, and CPU hotplug. With both patches applied, all
produced the expected results without a KASAN report, Oops, or LOCKDEP
warning. Strict KCSAN did not report a race involving GadgetFS or its AIO
work functions.

In my x86-64 QEMU and dummy_hcd tests, the second patch prevented the
module-unload problem that remained with the first patch alone. I did not
find a new failure attributable to the second patch in these tests.

With sincere appreciation and great respect,
Minseo Kim

2026년 9월 9일 (수) 오전 4:03, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>님이 작성:
>
> On Mon, Sep 07, 2026 at 10:00:00PM +0900, Minseo Kim wrote:
> > Hi Alan,
> >
> > > Is it okay to add your Tested-by: tag?
> >
> > Yes, please add:
> >
> > Tested-by: Minseo Kim <neck3922@xxxxxxxxx>
>
> Will do.
>
> > Thank you for the time and care you have put into this fix. I have learned
> > a great deal while working through this issue with you.
>
> You're welcome.
>
> In your earlier testing, didn't you find that with the unpatched driver,
> the driver module's usage count remained elevated as long as the
> ep_user_copy_worker() was pending on the workqueue?  And therefore it
> was impossible to unload the module while a work item was still queued
> or running?
>
> This patch introduces the possibility of that happening.  Therefore I
> would like to have a second patch, which flushes the workqueue and
> prevents the kernel from trying to run code in an unloaded module, ready
> to submit along with the first one.
>
> My version of this second patch (meant to apply on top of the first
> patch) is below.  It is closely based on the version you wrote, the main
> difference being that it uses a single workqueue for both work routines.
> Could you test it and verify that it prevents the problem you observed
> with only the first patch installed?
>
> Thank,
>
> Alan Stern
>
>
>
> Index: usb-devel/drivers/usb/gadget/legacy/inode.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/gadget/legacy/inode.c
> +++ usb-devel/drivers/usb/gadget/legacy/inode.c
> @@ -237,6 +237,7 @@ static const char *CHIP;
>  static DEFINE_MUTEX(sb_mutex);         /* Serialize superblock operations */
>
>  static DEFINE_SPINLOCK(aio_lock);      /* Protect aio cancellation info */
> +static struct workqueue_struct *gadgetfs_wq;
>
>  /*----------------------------------------------------------------------*/
>
> @@ -514,9 +515,7 @@ static int ep_aio_cancel(struct kiocb *i
>                 spin_unlock_irqrestore(&aio_lock, flags);
>                 return 0;       /* ep_aio() will call us again if needed */
>         }
> -
>         priv->cancel_state = AIO_UNLINKING;
> -       spin_unlock_irqrestore(&aio_lock, flags);
>
>         /*
>          * We are called with the aio core holding iocb's context lock.
> @@ -527,7 +526,9 @@ static int ep_aio_cancel(struct kiocb *i
>          * For this reason, do the dequeue operation in a work routine.
>          */
>         INIT_WORK(&priv->unlink_work, ep_unlink_worker);
> -       schedule_work(&priv->unlink_work);
> +       queue_work(gadgetfs_wq, &priv->unlink_work);
> +
> +       spin_unlock_irqrestore(&aio_lock, flags);
>         return 0;
>  }
>
> @@ -597,7 +598,7 @@ static void ep_aio_complete(struct usb_e
>
>         if (new_req_state == AIO_COMPLETED) {
>                 INIT_WORK(&priv->copy_work, ep_user_copy_worker);
> -               schedule_work(&priv->copy_work);
> +               queue_work(gadgetfs_wq, &priv->copy_work);
>         }
>         if (cancel_state != AIO_UNLINKING) {
>                 usb_ep_free_request(ep, req);
> @@ -1773,6 +1774,8 @@ gadgetfs_unbind (struct usb_gadget *gadg
>         spin_unlock_irq (&dev->lock);
>
>         destroy_ep_files (dev);
> +       flush_workqueue(gadgetfs_wq);
> +
>         gadget->ep0->driver_data = NULL;
>         set_gadget_data (gadget, NULL);
>
> @@ -2245,10 +2248,17 @@ static int __init gadgetfs_init (void)
>  {
>         int status;
>
> +       gadgetfs_wq = alloc_workqueue("gadgetfs", WQ_PERCPU, 0);
> +       if (!gadgetfs_wq)
> +               return -ENOMEM;
> +
>         status = register_filesystem (&gadgetfs_type);
> -       if (status == 0)
> -               pr_info ("%s: %s, version " DRIVER_VERSION "\n",
> -                       shortname, driver_desc);
> +       if (status) {
> +               destroy_workqueue(gadgetfs_wq);
> +               return status;
> +       }
> +
> +       pr_info("%s: %s, version " DRIVER_VERSION "\n", shortname, driver_desc);
>         return status;
>  }
>  module_init (gadgetfs_init);
> @@ -2257,6 +2267,7 @@ static void __exit gadgetfs_cleanup (voi
>  {
>         pr_debug ("unregister %s\n", shortname);
>         unregister_filesystem (&gadgetfs_type);
> +       destroy_workqueue(gadgetfs_wq);
>  }
>  module_exit (gadgetfs_cleanup);
>
>