Re: [BUG] usb: gadgetfs: KASAN null-ptr-deref and intermittent UAF in ep_aio_cancel()
Minseo Kim <[email protected]> Sun, 2 Aug 2026 15:08:11 +0900
| Newsgroups | org.kernel.vger.linux-usb,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAFmvuTXvwuUrfbvsAQ_VvC9jH5iS62J+Ei23TD0NfQ_igFRJ_A@mail.gmail.com> |
Hi Alan,
Thank you for the new patch. I applied it as posted to upstream v7.2-rc1,
commit dc59e4fea9d83f03bad6bddf3fa2e52491777482.
Across the timing ranges I tested, I did not reproduce the originally
reported ep_aio_cancel() stale-priv failure on the kernel with the new
patch applied. In a targeted diagnostic comparison, the original
ep_aio_cancel() UAF was reproduced on the unpatched kernel. Using the
same kernel configuration, reproducer, and arguments, the diagnostic
runs on the patched kernel produced no KASAN or Oops. I also did not
observe the original null-ptr-deref or UAF signatures in non-diagnostic
runs of the patched kernel.
However, I observed a different slab-use-after-free in ep_aio() on the
patched kernel:
BUG: KASAN: slab-use-after-free in ep_aio+0x573/0x660
Read of size 4
drivers/usb/gadget/legacy/inode.c:647
This UAF was repeatedly reproduced on the patched kernel. It was not
observed in control runs on the unpatched kernel using the same kernel
configuration, reproducer, workload, and timing settings.
The ordering appears to be:
1. ep_aio() registers cancellation and successfully queues the USB request.
2. ep_aio() releases epdata->dev->lock before acquiring aio_lock.
3. ep_aio_complete() can run the immediate-completion branch, clear
iocb->private, call iocb->ki_complete(), set priv->req_state to
AIO_GIVEN_BACK, observe that cancel_state is not AIO_UNLINKING, and
reach the path that frees priv.
4. ep_aio() then acquires aio_lock and reads priv->req_state at line 647.
Thus aio_lock serializes the post-queue state check and update, but it
does not keep priv alive until ep_aio() has finished that transition.
This is not the original ep_aio_cancel() stale-pointer window. The
observed UAF and the code ordering are consistent with ep_aio()
dereferencing priv after the completion path has freed it.
This suggests that the submitting path may need an additional lifetime
handoff. Either priv must remain alive until ep_aio() has finished its
post-queue work, or ep_aio() must no longer access priv after the point at
which the completion path may free it.
In diagnostic runs, I also tested cancellation while the request was in
AIO_SUBMITTING. In the queue-success ordering, I observed one completion
event with res=-ECONNRESET; in the forced queue-failure ordering, one
event with res=-EINVAL. Neither case produced KASAN or an Oops.
I also rechecked the potential unbind race you mentioned earlier. Using
pointer-correlated tracing on the patched build, I observed an
ep_aio_cancel() operation overlapping the gadgetfs_unbind() path while
that path called usb_ep_disable() on the request's endpoint. The
ep_aio_cancel() entry and ep_aio_complete() records referred to the same
iocb, priv, and usb_request, and the corresponding usb_ep_dequeue()
record referred to that same usb_request and endpoint.
ep_user_copy_worker() also ran for the same priv while gadgetfs_unbind()
was still in progress.
Corresponding runs with the probes disabled, using the same delay settings
as the traced runs, produced no KASAN or Oops.
Supporting files:
C reproducer for the ep_aio() post-queue UAF on the patched kernel:
https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_v3_validation_20260802/repro_v3_ep_aio_postqueue_uaf.c
Build:
gcc -O2 -Wall -Wextra -pthread -o repro_v3_ep_aio_postqueue_uaf \
repro_v3_ep_aio_postqueue_uaf.c
Symbolized KASAN report:
https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_v3_validation_20260802/symbolized_report_v3_ep_aio_postqueue_uaf_unified.txt
Kernel config used for the post-queue UAF reproducer and unpatched
control runs:
https://raw.githubusercontent.com/neck392/linux-kernel-bug-reports/main/gadgetfs_candidate_v3_validation_20260802/kernel.config.v7.2-rc1-candidate-v3-unified
I hope these results are useful.
Best regards,
Minseo Kim
2026년 8월 1일 (토) 오전 3:59, Alan Stern <[email protected]>님이 작성:
>
> On Fri, Jul 31, 2026 at 12:55:03AM +0900, Minseo Kim wrote:
> > For a fix, I think the important invariant is exactly-once completion and
> > teardown: each GadgetFS private object and USB request must be released
> > exactly once, the iocb must be completed exactly once, and no path may
> > dereference any of these objects after its lifetime has ended.
>
> Okay, the new patch below guarantees this. Or at least, I believe it
> does. :-)
>
> > One additional point from the v2 results may be relevant to the approach
> > you outlined: delaying kiocb_set_cancel_fn() until after successful
> > submission. Moving it after a successful usb_ep_queue(), by itself, may
> > not be sufficient: once the request has been queued successfully, its
> > completion can run before cancellation is registered. The revised-patch
> > UAF demonstrated this completion-before-registration ordering. The
> > registration and completion paths therefore appear to need an additional
> > ordering or lifetime mechanism.
>
> The new patch registers the cancel function before submission, so this
> will be okay.
>
> > There is also a locking constraint: io_cancel() and free_ioctx_users() can
> > invoke the cancel callback while holding ctx->ctx_lock. A synchronous
> > dequeue giveback must therefore not cause aio_complete_rw() to reacquire
> > that lock while the iocb is still linked and the original caller still
> > holds ctx->ctx_lock.
>
> In the new patch, nothing more complicated than kfree() happens while
> the new aio_lock is held, no new locking cycles will be created. Of
> course, your testing may reveal a pre-existing cycle.
>
> On the other hand, we have no control over whether dequeue givebacks are
> synchronous. If necessary we could complete the aio in a different
> thread, but that would be wasteful if it isn't needed.
>
> Thanks for your help, and let me know how the patch below works out.
>
> 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
> @@ -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,19 @@ static long ep_ioctl(struct file *fd, un
>
> /* ASYNCHRONOUS ENDPOINT I/O OPERATIONS (bulk/intr/iso) */
>
> +enum aio_req_state {
> + AIO_SUBMITTING,
> + AIO_RUNNING,
> + AIO_COMPLETED,
> + AIO_GIVEN_BACK,
> +};
> +
> +enum aio_cancel_state {
> + AIO_NOT_CANCELLED,
> + AIO_UNLINKING,
> + AIO_UNLINK_DONE,
> +};
> +
> struct kiocb_priv {
> struct usb_request *req;
> struct ep_data *epdata;
> @@ -443,24 +458,53 @@ struct kiocb_priv {
> struct iov_iter to;
> const void *to_free;
> unsigned actual;
> + enum aio_req_state req_state;
> + enum aio_cancel_state cancel_state;
> };
>
> static int ep_aio_cancel(struct kiocb *iocb)
> {
> - struct kiocb_priv *priv = iocb->private;
> + struct kiocb_priv *priv;
> struct ep_data *epdata;
> + struct usb_ep *ep;
> int value;
> + enum aio_req_state req_state;
>
> - local_irq_disable();
> - 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();
> + spin_lock_irq(&aio_lock);
> + priv = iocb->private;
> + if (!priv || priv->cancel_state != AIO_NOT_CANCELLED) {
> + spin_unlock_irq(&aio_lock);
> + return -EINVAL; /* Already completed or cancelled */
> + }
> + if (priv->req_state == AIO_SUBMITTING) {
> + priv->cancel_state = AIO_UNLINK_DONE;
> + spin_unlock_irq(&aio_lock);
> + return 0; /* ep_aio() will call us again if needed */
> + }
>
> + epdata = priv->epdata;
> + ep = epdata->ep;
> + priv->cancel_state = AIO_UNLINKING;
> + spin_unlock_irq(&aio_lock);
> +
> + value = usb_ep_dequeue(ep, priv->req);
> +
> + spin_lock_irq(&aio_lock);
> + req_state = priv->req_state;
> + priv->cancel_state = AIO_UNLINK_DONE;
> + spin_unlock_irq(&aio_lock);
> +
> + /*
> + * priv->req and epdata are freed after unlinking and completion are
> + * both done.
> + * priv itself is freed after unlinking and giveback are both done.
> + */
> + if (req_state >= AIO_COMPLETED) {
> + usb_ep_free_request(ep, priv->req);
> + put_ep(epdata);
> + if (req_state == AIO_GIVEN_BACK)
> + kfree(priv);
> + }
> return value;
> }
>
> @@ -482,7 +526,12 @@ static void ep_user_copy_worker(struct w
>
> kfree(priv->buf);
> kfree(priv->to_free);
> - kfree(priv);
> +
> + spin_lock_irq(&aio_lock);
> + priv->req_state = AIO_GIVEN_BACK;
> + if (priv->cancel_state != AIO_UNLINKING)
> + kfree(priv);
> + spin_unlock_irq(&aio_lock);
> }
>
> static void ep_aio_complete(struct usb_ep *ep, struct usb_request *req)
> @@ -490,11 +539,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;
> + enum aio_req_state new_req_state;
> + enum aio_cancel_state cancel_state;
>
> - /* lock against disconnect (and ideally, cancel) */
> + /* lock against unbind */
> spin_lock(&epdata->dev->lock);
> - priv->req = NULL;
> - priv->epdata = NULL;
> +
> + /* Prevent future cancellation */
> + spin_lock(&aio_lock);
> + iocb->private = NULL;
> + spin_unlock(&aio_lock);
>
> /* 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,10 +557,9 @@ 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);
> + new_req_state = AIO_GIVEN_BACK;
> } else {
> /* ep_copy_to_user() won't report both; we hide some faults */
> if (unlikely(0 != req->status))
> @@ -516,12 +569,23 @@ static void ep_aio_complete(struct usb_e
> priv->buf = req->buf;
> priv->actual = req->actual;
> INIT_WORK(&priv->work, ep_user_copy_worker);
> - schedule_work(&priv->work);
> + new_req_state = AIO_COMPLETED;
> }
> -
> - usb_ep_free_request(ep, req);
> spin_unlock(&epdata->dev->lock);
> - put_ep(epdata);
> +
> + spin_lock(&aio_lock);
> + priv->req_state = new_req_state;
> + cancel_state = priv->cancel_state;
> + spin_unlock(&aio_lock);
> +
> + if (new_req_state == AIO_COMPLETED)
> + schedule_work(&priv->work);
> + if (cancel_state != AIO_UNLINKING) {
> + usb_ep_free_request(ep, req);
> + put_ep(epdata);
> + if (new_req_state == AIO_GIVEN_BACK)
> + kfree(priv);
> + }
> }
>
> static ssize_t ep_aio(struct kiocb *iocb,
> @@ -532,11 +596,12 @@ static ssize_t ep_aio(struct kiocb *iocb
> {
> struct usb_request *req;
> ssize_t value;
> + struct usb_ep *ep;
> + bool need_unlink = false;
>
> iocb->private = priv;
> priv->iocb = iocb;
>
> - kiocb_set_cancel_fn(iocb, ep_aio_cancel);
> get_ep(epdata);
> priv->epdata = epdata;
> priv->actual = 0;
> @@ -547,10 +612,11 @@ static ssize_t ep_aio(struct kiocb *iocb
> */
> spin_lock_irq(&epdata->dev->lock);
> value = -ENODEV;
> - if (unlikely(epdata->ep == NULL))
> + ep = epdata->ep;
> + if (unlikely(ep == NULL))
> goto fail;
>
> - req = usb_ep_alloc_request(epdata->ep, GFP_ATOMIC);
> + req = usb_ep_alloc_request(ep, GFP_ATOMIC);
> value = -ENOMEM;
> if (unlikely(!req))
> goto fail;
> @@ -560,12 +626,37 @@ static ssize_t ep_aio(struct kiocb *iocb
> req->length = len;
> req->complete = ep_aio_complete;
> req->context = iocb;
> - value = usb_ep_queue(epdata->ep, req, GFP_ATOMIC);
> +
> + priv->req_state = AIO_SUBMITTING;
> + priv->cancel_state = AIO_NOT_CANCELLED;
> + kiocb_set_cancel_fn(iocb, ep_aio_cancel);
> +
> + value = usb_ep_queue(ep, req, GFP_ATOMIC);
> if (unlikely(0 != value)) {
> - usb_ep_free_request(epdata->ep, req);
> + spin_lock(&aio_lock);
> + iocb->private = NULL;
> + spin_unlock(&aio_lock);
> +
> + usb_ep_free_request(ep, req);
> goto fail;
> }
> spin_unlock_irq(&epdata->dev->lock);
> +
> + spin_lock_irq(&aio_lock);
> + if (priv->req_state == AIO_SUBMITTING) {
> + priv->req_state = AIO_RUNNING;
> +
> + /* Cancelled before or just after submission? */
> + if (priv->cancel_state == AIO_UNLINK_DONE) {
> + priv->cancel_state = AIO_NOT_CANCELLED;
> + need_unlink = true;
> + }
> + } /* Otherwise already completed */
> + spin_unlock_irq(&aio_lock);
> +
> + if (need_unlink) /* Redo cancel that was too early */
> + ep_aio_cancel(iocb);
> +
> return -EIOCBQUEUED;
>
> fail:
>