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

Hi Alan,

Thank you. I am glad the testing has been useful.

> What was the cause of this warning? If it is sufficiently
> straightforward, maybe I can fix it as well.

The warning is caused by ep_aio_cancel() enabling local IRQs while its
caller still holds ctx->ctx_lock. io_cancel() acquires ctx->ctx_lock with
spin_lock_irq() and invokes the cancel callback before releasing that
lock. In the posted revision, ep_aio_cancel() uses spin_lock_irq() for
aio_lock and releases it with spin_unlock_irq(), which enables local IRQs
before the callback returns. LOCKDEP records the resulting SOFTIRQ-ON-W
usage; in the reported run, it later reports inconsistent
softirq-context use of the same lock in the free_ioctx_users() path.

When entered with local IRQs already disabled, the unpatched driver has
the same underlying behavior because ep_aio_cancel() unconditionally
calls local_irq_enable() before returning. Using the same reproducer,
arguments, and kernel configuration, I reproduced the warning on both the
posted revision and the unpatched kernel.

Would it make sense to change the aio_lock operations in
ep_aio_cancel() to spin_lock_irqsave() and spin_unlock_irqrestore(), using
the saved flags on every path that releases the lock? This would preserve
the incoming IRQ state both when the AIO core invokes the callback and
when ep_aio() replays an early cancellation.

I tested this change locally by rerunning, on fresh boots, the full
pre-queue cancellation matrix from my previous message and a longer
cancellation workload of 1000 rounds with 32 requests per round. Each
matrix case produced exactly one expected completion, and neither the
matrix nor the longer workload produced a KASAN report, Oops, or LOCKDEP
warning. If you prefer a different way to preserve the caller's IRQ
state, I would be happy to test that as well.

Separately, my understanding was that we had set the teardown issue aside
for a later discussion. I also tested that case and wanted to share the
result here in case it is useful.

On the posted revision, the reproducer repeatedly triggered:

  KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
  Workqueue: events ep_unlink_worker
  RIP: usb_ep_dequeue+0x2c/0x220
  drivers/usb/gadget/udc/core.c:333
  called from ep_unlink_worker+0x8d/0x1b0
  drivers/usb/gadget/legacy/inode.c:477

The endpoint argument to usb_ep_dequeue() was NULL. I also reproduced the
same teardown failure with the local IRQ-state change applied, so the two
issues appear independent.

The reproducer submits AIO FSYNC requests from a thread pinned to CPU 0
and verifies that at least 1024 remain outstanding. The cancel thread is
also pinned to CPU 0, so the work items handled by aio_fsync_work() and
ep_unlink_worker() are both queued through schedule_work() to the CPU 0
worker pool of the system per-CPU workqueue. In the faulting ordering,
after io_cancel() returned -1 with errno set to EINPROGRESS, the main
thread on CPU 1 closed ep0 before ep_unlink_worker() reached
usb_ep_dequeue().

Closing ep0 invoked dev_release(), which called
usb_gadget_unregister_driver(). The unregister path then invoked
gadgetfs_unbind(), where destroy_ep_files() cleared epdata->ep. When
ep_unlink_worker() later reached the dequeue call, it passed the now-NULL
epdata->ep to usb_ep_dequeue().

Quiescing or flushing the relevant unlink work only after
destroy_ep_files() would be too late for this ordering, because
epdata->ep had already been cleared before that synchronization began.
The teardown path therefore appears to need synchronization that prevents
ep_unlink_worker() from dereferencing the endpoint after invalidation,
whether by preventing new unlink_work from being queued and quiescing
pending or running work before invalidation, retaining the endpoint until
such work finishes, or using an equivalent state or lifetime mechanism.

Supporting files:

  LOCKDEP report for the posted revision:
    https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_patch_20260818_lockdep_teardown_followup_20260821/lockdep_ctx_lock_irq_state_warning.txt

  Matched unpatched LOCKDEP control report:
    https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_patch_20260818_lockdep_teardown_followup_20260821/lockdep_unpatched_ctx_lock_irq_state_control.txt

  C reproducer for the teardown null-ptr-deref:
    https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_patch_20260818_lockdep_teardown_followup_20260821/repro_unbind_unlink_ep_null.c

  Build:
    gcc -O2 -Wall -Wextra -pthread -o repro_unbind_unlink_ep_null \
      repro_unbind_unlink_ep_null.c

  Symbolized KASAN report:
    https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_patch_20260818_lockdep_teardown_followup_20260821/symbolized_report_unbind_unlink_ep_null.txt

  Kernel config used for these runs:
    https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_patch_20260818_lockdep_teardown_followup_20260821/kernel.config.kasan_inline_dwarf5_lockdep

I hope this clarifies the warning and provides useful information for
the separate teardown issue.

Best regards,
Minseo Kim

2026년 8월 20일 (목) 오후 11:08, Alan Stern <stern-nwvwT67g6+6dFdvTe/[email protected]>님이 작성:
>
> On Thu, Aug 20, 2026 at 06:40:19PM +0900, Minseo Kim wrote:
> > Hi Alan,
> >
> > Thank you for the revised patch and for your kind words about the testing.
> > I applied it as posted to upstream v7.2-rc1, commit
> > dc59e4fea9d83f03bad6bddf3fa2e52491777482.
> >
> > I did not reproduce the previously reported ep_unlink_worker() UAF with
> > this revision in the same directed cross-CPU diagnostic. I also reran the
> > original null-ptr-deref and UAF reproducers and the reproducers for the
> > earlier candidate-patch regressions, and did not observe their
> > corresponding KASAN signatures.
>
> Excellent!
>
> > > Nor any of the old lockdep violations, I trust.
> >
> > In the matched runs, I did not observe any of the previously reported
> > LOCKDEP violations or any new violation attributable to this revision.
> > The only LOCKDEP warning I observed was a ctx_lock IRQ-state warning that
> > was also reproduced in matched runs on the unpatched kernel.
>
> What was the cause of this warning?  If it is sufficiently
> straightforward, maybe I can fix it as well.
>
> > > What happens if the aio is cancelled exactly between ep_aio()'s calls
> > > to kiocb_set_cancel_fn() and usb_ep_queue()?
> >
> > I exercised this exact interval by pausing the submitting thread in a
> > return probe for kiocb_set_cancel_fn(), before control resumed in ep_aio()
> > and before usb_ep_queue() was called. I released the submit path either
> > when the return probe for ep_aio_cancel() ran or, separately, when the
> > return probe for __x64_sys_io_cancel() ran. Both release points produced
> > the same results described below.
> >
> > When I allowed the queue operation to succeed, io_cancel() returned
> > -EINPROGRESS in both the PWRITE and PREAD cases. ep_aio() then replayed
> > the cancellation after the queue succeeded, and exactly one completion
> > event reported res=-ECONNRESET.
> >
> > When I forced the queue operation to return -EINVAL, io_cancel() again
> > returned -EINPROGRESS in both cases, and exactly one completion event
> > reported res=-EINVAL.
> >
> > I also tested a 64-byte PWRITE for which dummy_hcd completed the request
> > inside its queue callback. io_cancel() returned -EINPROGRESS, and exactly
> > one completion event reported res=64.
>
> Good, that's exactly what the results should be.
>
> > None of these tested orderings produced an additional completion event,
> > a KASAN report, or an Oops. In these tested orderings, the AIO_SUBMITTING
> > handling produced exactly one completion in each case: an early
> > cancellation was replayed after a pending queue succeeded, a failed queue
> > produced one completion with its error, and an immediate completion did
> > not produce a second completion.
> >
> > I hope this answers the remaining question.
>
> Yes, it all sounds good.  This patch is just about ready for submission.
>
> Alan Stern