Re: [PATCH AUTOSEL 6.18-5.10] USB: gadget: inode: fix mm lifetime handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: gadget: inode: mmdrop() called in atomic context in ep_aio_complete()
--

commit 1034aa9a2c06c8a1a74aa881eb54ff67ca5c0fee
Author: Gabriel Prostitis <prostitisgabriel@gmail.com>

USB: gadget: inode: fix mm lifetime handling

This commit fixes a use-after-free in gadgetfs async I/O handling by pinning
the submitting task's mm_struct with mmgrab() in ep_aio() and releasing it
with mmdrop() upon request completion. It also uses mmget_not_zero() in
ep_user_copy_worker() before switching to the address space.

Link: https://patch.msgid.link/20260601-mm-uaf-fix-v2-2-3c942a707bce@gmail.com

> diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
> index 13c3da49348c5..7d201c065ae3b 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c

[ ... ]

> @@ -503,6 +509,7 @@ static void ep_aio_complete(struct usb_ep *ep, struct usb_request *req)
>  	 * complete the aio request immediately.
>  	 */
>  	if (priv->to_free == NULL || unlikely(req->actual == 0)) {
> +		mmdrop(priv->mm);

[Severity: High]
Is it safe to call mmdrop(priv->mm) directly in ep_aio_complete()?

ep_aio() pins current->mm with mmgrab(priv->mm) for all async requests. If
the submitting task exits before the USB request completes (for example,
with io_uring or a forked process where do_exit() -> exit_mm() -> mmput()
runs before completion), priv->mm can hold the final reference on
mm->mm_count.

When the request completes and takes the immediate completion branch
(priv->to_free == NULL || unlikely(req->actual == 0)) — which occurs for
async writes, single-buffer ITER_UBUF reads where dup_iter() returns NULL,
zero-length reads, errors, disconnects, or cancellations via
ep_aio_cancel() -> usb_ep_dequeue() — ep_aio_complete() runs as the
usb_request->complete callback with interrupts disabled and while holding
spin_lock(&epdata->dev->lock):

drivers/usb/gadget/legacy/inode.c:ep_aio_complete() {
	spin_lock(&epdata->dev->lock);
	priv->req = NULL;
	priv->epdata = NULL;

	if (priv->to_free == NULL || unlikely(req->actual == 0)) {
		mmdrop(priv->mm);
}

If mmdrop(priv->mm) drops the last reference, it invokes __mmdrop() from
atomic, interrupts-disabled context:

kernel/fork.c:__mmdrop() {
	...
	cleanup_lazy_tlbs(mm);

	WARN_ON_ONCE(mm == current->active_mm);
	mm_free_pgd(mm);
	...
}

This appears to cause a few issues:

1. On architectures with CONFIG_MMU_LAZY_TLB_SHOOTDOWN=y,
__mmdrop() -> cleanup_lazy_tlbs() -> on_each_cpu_mask() ->
smp_call_function_many_cond() waits synchronously for cross-CPU IPIs,
triggering lockdep_assert_irqs_enabled() and WARN_ON_ONCE(!in_task()) in
kernel/smp.c and potentially deadlocking with interrupts disabled.

2. On x86, __mmdrop() -> mm_free_pgd() -> pgd_free() -> pgd_dtor() acquires
spin_lock(&pgd_lock) without disabling interrupts:

arch/x86/mm/pgtable.c:pgd_dtor() {
	spin_lock(&pgd_lock);
	pgd_list_del(pgd);
	spin_unlock(&pgd_lock);
}

As noted in free_signal_struct() in kernel/fork.c, __mmdrop() is not safe to
call from interrupt or softirq context on x86 because taking pgd_lock can
deadlock if the interrupted context already holds pgd_lock.

3. On CONFIG_PREEMPT_RT=y, __mmdrop() acquires sleeping spinlock_t locks
while interrupts are disabled and a spinlock is held.

Should ep_aio_complete() use mmdrop_async(priv->mm) here (or avoid grabbing
priv->mm when priv->to_free == NULL, since the worker is only scheduled when
priv->to_free != NULL)?

>  		kfree(req->buf);
>  		kfree(priv->to_free);
>  		kfree(priv);

[ ... ]

> @@ -543,6 +550,7 @@ static ssize_t ep_aio(struct kiocb *iocb,
>  	priv->epdata = epdata;
>  	priv->actual = 0;
>  	priv->mm = current->mm; /* mm teardown waits for iocbs in exit_aio() */
> +	mmgrab(priv->mm);

[ ... ]

> @@ -572,6 +580,7 @@ static ssize_t ep_aio(struct kiocb *iocb,
>  
>  fail:
>  	spin_unlock_irq(&epdata->dev->lock);
> +	mmdrop(priv->mm);
>  	kfree(priv->to_free);
>  	kfree(priv);
>  	put_ep(epdata);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-360-sashal@kernel.org?part=1