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

[Minseo Kim]

Hi Alan,

Thank you for the revised patch and for your kind feedback on my
analysis. I applied the patch to upstream v7.2-rc1, commit
dc59e4fea9d83f03bad6bddf3fa2e52491777482.

Regarding cancellation before ep_aio() returns, the reproducer I used
for that test has separate submit and cancel threads. After
kiocb_set_cancel_fn() links the corresponding aio_kiocb into
ctx->active_reqs and releases ctx->ctx_lock, the cancel thread can find
it and call ep_aio_cancel() before the submit thread returns from
ep_aio(). The synchronous reference retained by the submission path is
not dropped by io_submit_one() until after ep_aio() returns, so the
aio_kiocb remains alive during this interval.

With the revised patch, I did not observe the earlier setup-time
null-ptr-deref or the original ep_aio_cancel() null-ptr-deref and UAF
signatures.

However, with kiocb_set_cancel_fn() moved after usb_ep_queue(), I
observed a different UAF:

  BUG: KASAN: slab-use-after-free in kiocb_set_cancel_fn
  Write of size 8 in list_add_tail() at fs/aio.c:658

After the request is queued by usb_ep_queue(), its completion callback
can run before the iocb is added to active_reqs. In that ordering,
aio_complete_rw() sees an empty ki_list, skips aio_remove_iocb(), and
drops the asynchronous reference. ep_aio() subsequently links the
completed iocb into active_reqs. When io_submit_one() drops the
synchronous reference, the aio_kiocb can be freed while still linked. A
later active-list operation can then access the stale entry; the first
KASAN-detected invalid access occurred in list_add_tail() during a
subsequent kiocb_set_cancel_fn() call.

This UAF was repeatedly observed with the revised patch and was not
observed in matched runs on the unpatched kernel using the same
reproducer, workload, and timing settings.

Although the direct aio_lock-to-ctx_lock nesting was removed from the
completion path, LOCKDEP still reported the following transitive cycle:

  &ctx->ctx_lock -> aio_lock -> &dev->lock#2 -> &ctx->ctx_lock

In the immediate-completion branch, ep_aio_complete() acquires
dev->lock while holding aio_lock and later calls iocb->ki_complete()
while dev->lock remains held. If the iocb is still linked in
ctx->active_reqs, aio_complete_rw() calls aio_remove_iocb(), which
acquires ctx->ctx_lock. This preserves the
aio_lock -> dev->lock -> ctx_lock dependency, while io_cancel() supplies
the ctx_lock -> aio_lock edge.

I also exercised the unbind window you mentioned. Using
pointer-correlated tracing, I observed usb_ep_dequeue() from
ep_aio_cancel() for a given usb_request overlapping usb_ep_disable() in
the gadgetfs_unbind() path on that request's endpoint. In those
lineages, ep_aio_complete() and ep_user_copy_worker() also ran for the
same priv and usb_request. Neither those overlaps nor probe-disabled
runs covering the same timing ranges produced KASAN or an Oops. This
does not rule out an unbind-related failure at other timings; I did not
reproduce the suspected UAF in the timing ranges I tested.

Supporting files:

  C reproducer for the post-queue registration UAF:
    https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_patch_v2_validation_20260715/repro_v2_postqueue_uaf.c

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

  Symbolized KASAN report for the post-queue registration UAF:
    https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_patch_v2_validation_20260715/symbolized_report_v2_postqueue_uaf.txt

  Kernel config for the KASAN reproducer:
    https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_patch_v2_validation_20260715/kernel.config.v7.2-rc1-candidate-v2-kasan-inline-dwarf5

  LOCKDEP report for the remaining transitive cycle:
    https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_patch_v2_validation_20260715/lockdep_v2_exact_cycle.txt

I hope this information is useful.

Best regards,
Minseo Kim

2026년 7월 14일 (화) 오후 12:23, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>님이 작성:
>
> On Tue, Jul 14, 2026 at 03:47:55AM +0900, Minseo Kim wrote:
> > Hi Alan,
> >
> > Thank you very much for taking the time to prepare this patch. I applied
> > the patch as posted in your message to upstream v7.2-rc1, commit
> > dc59e4fea9d83f03bad6bddf3fa2e52491777482.
> >
> > I repeatedly tested the proposed patch with the original null-ptr-deref
> > and UAF reproducers, as well as several reduced variants. During those
> > tests, I did not observe either of the original signatures.
>
> That's good progress.
>
> > While continuing the tests, I observed a new null-ptr-deref in
> > ep_aio_cancel() on the kernel with the proposed patch applied. It appears
> > to be associated with the initialization window in ep_aio(). Running the
> > reproducer without command-line arguments triggered:
> >
> >   KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
> >   RIP: ep_aio_cancel+0xd0/0x2d0
> >   drivers/usb/gadget/legacy/inode.c:473
> >
> > The corresponding source statement is:
> >
> >   ep = epdata->ep;
> >
> > where epdata is NULL. The faulting task has UID 1000.
> >
> > One detail that may be relevant is the initialization order in ep_aio().
> > iocb->private is assigned and kiocb_set_cancel_fn() registers the
> > cancellation callback before priv->epdata is initialized. Because priv is
> > zero-initialized and AIO_REQ_RUNNING is the zero-valued enumerator in the
> > proposed patch, the state check can treat priv as RUNNING before the
> > explicit state assignment. The observed crash is consistent with
> > cancellation reaching this initialization window, because priv->epdata
> > was NULL at the fault.
>
> In the patch below, I moved the kiocb_set_cancel_fn() call down to after
> all the other initialization.  That ought to take care of the problem.
> But I admit, I don't understand how the I/O can be cancelled before
> ep_aio() returns.
>
> > I also ran the reproducer on a LOCKDEP-enabled build. LOCKDEP reported the
> > following possible circular locking dependency involving the newly added
> > aio_lock:
> >
> >   &ctx->ctx_lock -> aio_lock -> &dev->lock#2 -> &ctx->ctx_lock
> >
> > The cycle appears to arise because io_cancel() calls ep_aio_cancel() while
> > holding ctx->ctx_lock, and ep_aio_cancel() then acquires aio_lock. In the
> > other direction, the patched completion path holds aio_lock and dev->lock
> > while calling iocb->ki_complete(). In this path, the completion callback
> > is aio_complete_rw(), which can acquire ctx->ctx_lock.
> >
> > The reported cycle involving aio_lock appears specific to the proposed
> > patch, since aio_lock is newly introduced there.
>
> In the new patch I removed the dev->lock accesses in ep_aio_cancel() and
> moved the iocb->ko_complete() call outside the scope of aio_lock in
> ep_aio_complete().  This should break the locking cycles.
>
> There's still a potential race: unbind against ep_aio_cancel or
> ep_user_copy_work.  That would be a difficult race to trigger; you'd
> have to unbind the gadgetfs driver just as an aio request was
> completing or being cancelled.
>
> > I wanted to share these observations in case they are useful. Please let
> > me know if I have misunderstood any part of the initialization or locking
> > sequence.
>
> No, I think you've done a great job of assimilating this complex
> information.  Let me know how the new patch works out.
>
> Alan Stern
>
>
> ---
>  drivers/usb/gadget/legacy/inode.c |   92 +++++++++++++++++++++++++++++---------
>  1 file changed, 72 insertions(+), 20 deletions(-)
>
> 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
> @@ -236,6 +236,8 @@ static void put_ep (struct ep_data *data
>  static const char *CHIP;
>  static DEFINE_MUTEX(sb_mutex);         /* Serialize superblock operations */
>
> +static DEFINE_SPINLOCK(aio_lock);      /* Protect aio cancellation info */
> +
>  /*----------------------------------------------------------------------*/
>
>  /* NOTE:  don't use dev_printk calls before binding to the gadget
> @@ -433,6 +435,13 @@ static long ep_ioctl(struct file *fd, un
>
>  /* ASYNCHRONOUS ENDPOINT I/O OPERATIONS (bulk/intr/iso) */
>
> +enum aio_req_state {
> +       AIO_REQ_RUNNING,
> +       AIO_REQ_CANCELLING,
> +       AIO_REQ_CANCELLED,
> +       AIO_REQ_COMPLETED,
> +};
> +
>  struct kiocb_priv {
>         struct usb_request      *req;
>         struct ep_data          *epdata;
> @@ -443,24 +452,48 @@ struct kiocb_priv {
>         struct iov_iter         to;
>         const void              *to_free;
>         unsigned                actual;
> +       enum aio_req_state      aio_state;
>  };
>
>  static int ep_aio_cancel(struct kiocb *iocb)
>  {
> -       struct kiocb_priv       *priv = iocb->private;
> +       struct kiocb_priv       *priv;
>         struct ep_data          *epdata;
> -       int                     value;
> +       struct usb_ep           *ep;
> +       struct dev_data         *dev;
> +       int                     value = -EINVAL;
> +
> +       spin_lock_irq(&aio_lock);
> +       priv = iocb->private;
> +       if (!priv || priv->aio_state != AIO_REQ_RUNNING)
> +               goto Done;      /* Already completed or cancelled */
>
> -       local_irq_disable();
> +       priv->aio_state = AIO_REQ_CANCELLING;
>         epdata = priv->epdata;
> -       // spin_lock(&epdata->dev->lock);
> -       if (likely(epdata && epdata->ep && priv->req))
> -               value = usb_ep_dequeue (epdata->ep, priv->req);
> -       else
> -               value = -EINVAL;
> -       // spin_unlock(&epdata->dev->lock);
> -       local_irq_enable();
> +       ep = epdata->ep;
> +       dev = epdata->dev;
> +       spin_unlock_irq(&aio_lock);
> +
> +       value = usb_ep_dequeue(ep, priv->req);
> +
> +       /*
> +        * priv->req is freed by the first stage of completion or the
> +        * second stage of cancellation, whichever is last.
> +        * priv itself is freed by the final stage of completion or
> +        * the second stage of cancellation, whichever is last.
> +        */
> +       spin_lock_irq(&aio_lock);
> +       if (priv->aio_state == AIO_REQ_CANCELLING) {
> +               /* Completion is not yet finished */
> +               priv->aio_state = AIO_REQ_CANCELLED;
> +       } else {
> +               /* Must be AIO_REQ_COMPLETED so completion is finished */
> +               usb_ep_free_request(ep, priv->req);
> +               kfree(priv);
> +       }
>
> + Done:
> +       spin_unlock_irq(&aio_lock);
>         return value;
>  }
>
> @@ -482,7 +515,13 @@ static void ep_user_copy_worker(struct w
>
>         kfree(priv->buf);
>         kfree(priv->to_free);
> -       kfree(priv);
> +
> +       spin_lock_irq(&aio_lock);
> +       if (priv->aio_state == AIO_REQ_CANCELLING)
> +               priv->aio_state = AIO_REQ_COMPLETED;
> +       else
> +               kfree(priv);    /* Not cancelled or cancellation is finished */
> +       spin_unlock_irq(&aio_lock);
>  }
>
>  static void ep_aio_complete(struct usb_ep *ep, struct usb_request *req)
> @@ -490,11 +529,16 @@ static void ep_aio_complete(struct usb_e
>         struct kiocb            *iocb = req->context;
>         struct kiocb_priv       *priv = iocb->private;
>         struct ep_data          *epdata = priv->epdata;
> +       bool                    cancelling;
> +       size_t                  ret;
> +
> +       /* lock against cancellation */
> +       spin_lock(&aio_lock);
> +       cancelling = (priv->aio_state == AIO_REQ_CANCELLING);
>
> -       /* lock against disconnect (and ideally, cancel) */
> +       /* lock against unbind */
>         spin_lock(&epdata->dev->lock);
> -       priv->req = NULL;
> -       priv->epdata = NULL;
> +       iocb->private = NULL;           /* Prevent future cancellation */
>
>         /* if this was a write or a read returning no data then we
>          * don't need to copy anything to userspace, so we can
> @@ -503,11 +547,17 @@ static void ep_aio_complete(struct usb_e
>         if (priv->to_free == NULL || unlikely(req->actual == 0)) {
>                 kfree(req->buf);
>                 kfree(priv->to_free);
> -               kfree(priv);
> -               iocb->private = NULL;
> -               iocb->ki_complete(iocb,
> -                               req->actual ? req->actual : (long)req->status);
> +               if (cancelling) /* Cancellation started, not yet finished */
> +                       priv->aio_state = AIO_REQ_COMPLETED;
> +               else            /* Not cancelled or cancellation is finished */
> +                       kfree(priv);
> +               ret = req->actual;
> +               if (ret == 0)
> +                       ret = req->status;
> +               spin_unlock(&aio_lock);         /* Cancel now may free req */
> +               iocb->ki_complete(iocb, ret);
>         } else {
> +               spin_unlock(&aio_lock);
>                 /* ep_copy_to_user() won't report both; we hide some faults */
>                 if (unlikely(0 != req->status))
>                         DBG(epdata->dev, "%s fault %d len %d\n",
> @@ -519,7 +569,8 @@ static void ep_aio_complete(struct usb_e
>                 schedule_work(&priv->work);
>         }
>
> -       usb_ep_free_request(ep, req);
> +       if (!cancelling)        /* Not cancelled or cancellation is finished */
> +               usb_ep_free_request(ep, req);
>         spin_unlock(&epdata->dev->lock);
>         put_ep(epdata);
>  }
> @@ -536,11 +587,11 @@ static ssize_t ep_aio(struct kiocb *iocb
>         iocb->private = priv;
>         priv->iocb = iocb;
>
> -       kiocb_set_cancel_fn(iocb, ep_aio_cancel);
>         get_ep(epdata);
>         priv->epdata = epdata;
>         priv->actual = 0;
>         priv->mm = current->mm; /* mm teardown waits for iocbs in exit_aio() */
> +       priv->aio_state = AIO_REQ_RUNNING;
>
>         /* each kiocb is coupled to one usb_request, but we can't
>          * allocate or submit those if the host disconnected.
> @@ -566,6 +617,7 @@ static ssize_t ep_aio(struct kiocb *iocb
>                 goto fail;
>         }
>         spin_unlock_irq(&epdata->dev->lock);
> +       kiocb_set_cancel_fn(iocb, ep_aio_cancel);
>         return -EIOCBQUEUED;
>
>  fail: