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

[김민서]

Hi Alan,

Thank you, that makes sense.

> Also, NULL checks don't fix races (cancel vs. complete).

Agreed. A NULL check alone would likely only address the repeated
NULL-deref symptom, not the stale non-NULL priv case.

> Serialization seems like the best
> solution (provided you can guarantee that it won't cause a deadlock).

That makes sense to me. Locking changes in this path could affect USB
request completion, AIO completion, and callback/deadlock interactions, so
I do not think it would be appropriate for me to send a locking patch
myself without first being confident about the deadlock implications.

> Maybe because at that point in the code there is no way to know whether
> epdata and epdata->dev are valid pointers?

Yes, I see that concern. Simply restoring the commented-out spin_lock() in
ep_aio_cancel() does not look obviously safe to me either: ep_aio_cancel()
would first have to rely on iocb->private (the priv pointer) in order to
reach epdata and epdata->dev, and holding epdata->dev->lock around
usb_ep_dequeue() could have problematic interactions with the completion
path, since ep_aio_complete() appears to use the same lock.

I can help validate candidate fixes under KASAN against the reproducer and
the intermittent UAF variant. I can also build a lockdep-enabled kernel and
run the reproducer against candidate fixes if that would be useful.

The clearest conclusion I can draw is still the lifetime invariant:
ep_aio_cancel() should not be able to use iocb->private as a
struct kiocb_priv pointer after the completion path has cleared
iocb->private or freed the underlying struct kiocb_priv. I would be happy
to test any candidate fix or approach you think would be worth trying.

Best regards,
Minseo Kim

2026년 7월 2일 (목) 오후 11:22, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>님이 작성:
>
> On Thu, Jul 02, 2026 at 05:08:17PM +0900, 김민서 wrote:
> > Hi Alan,
> >
> > Thank you for taking a look.
> >
> > I do not want to over-specify the exact fix, but my tentative view from
> > the reproducer runs and code inspection is that the key invariant is that
> > struct kiocb_priv should either remain valid while it can still be reached
> > by ep_aio_cancel() for the same iocb, or it should be made unreachable from
> > the cancel path in a synchronized way.
> >
> > In the immediate completion path I am looking at, ep_aio_complete()
> > appears to clear fields in priv and then free priv before iocb->private is
> > set to NULL and before iocb->ki_complete() runs. Meanwhile ep_aio_cancel()
> > reads iocb->private and immediately dereferences priv->epdata.
> >
> > That seems to explain both symptoms I observed: a NULL-deref if
> > iocb->private is already NULL, and a UAF if cancellation observes a stale
> > non-NULL priv pointer.
> >
> > I may be missing some ordering guarantee in the AIO core, but from the
> > reproducer this is the window that seems relevant. So I think a NULL check
> > in ep_aio_cancel() would likely handle the repeated NULL-deref symptom,
> > but may not address the stale non-NULL case. A safer direction might be to
> > ensure sufficient serialization or lifetime ordering around iocb->private /
> > struct kiocb_priv, so that either priv remains valid for the cancel
> > callback, or cancellation is already unreachable before priv is freed for
> > that iocb.
>
> Also, NULL checks don't fix races (cancel vs. complete).
>
> It's hard to make lifetime ordering work when you don't know whether the
> aio transfer will ever be cancelled.  Serialization seems like the best
> solution (provided you can guarantee that it won't cause a deadlock).
>
> However, I believe there is a valid reason why the spinlock calls in
> ep_aio_cancel() are commented out, although I don't remember what it is.
> Maybe because at that point in the code there is no way to know whether
> epdata and epdata->dev are valid pointers?
>
> Anyway, it would be good if you could figure out a solution and write a
> patch to implement it.  Perhaps a single global spinlock to protect all
> the aio pathways (or all those for a particular gadget) would work.
>
> Alan Stern