Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER

[Lorenzo Stoakes <ljs@kernel.org>]

On Mon, Jul 06, 2026 at 09:32:47PM +0800, Xie Yuanbin wrote:
> On Fri, 26 Jun 2026 15:30:47 +0800, Qi Xi wrote:
> > @@ -181,7 +181,9 @@ __do_user_fault(unsigned long addr, unsigned int fsr, unsigned int sig,
> >  		pr_err("8<--- cut here ---\n");
> >  		pr_err("%s: unhandled page fault (%d) at 0x%08lx, code 0x%03x\n",
> >  		       tsk->comm, sig, addr, fsr);
> > +		mmap_read_lock(tsk->mm);
> >  		show_pte(KERN_ERR, tsk->mm, addr);
> > +		mmap_read_unlock(tsk->mm);
> >  		show_regs(regs);
> >  	}
> >  #endif
>
> I found that this fix does not completely solve the problem. For a user
> fault, the addr could also be a kernel address. For arm32/x86, the kernel
> address space and user address space share the same pgd page table,
> but the kernel address space's page table is not protected by
> current->mm->mmap_lock.
>
> I have written a use case to construct and verify this point. When A user
> program accesses a kernel address and triggers __do_user_fault(),
> show_pte() will directly print the kernel page table.
>
> So, I suggest that:
> ```c
> 	if (user_mode(regs)) {
> 		struct mm_struct *const pt_mm = addr >= TASK_SIZE ?
> 			&init_mm : current->mm;
>
> 		mmap_read_lock(pt_mm);
> 		show_pte(KERN_ALERT, pt_mm, addr);
> 		mmap_read_unlock(pt_mm);
> 	} else {
> 		// .. keep nothing change
> 		show_pte(KERN_ALERT, current->mm, addr);
> 	}
> ```
>
> I have read this article:
> Link: https://docs.kernel.org/mm/process_addrs.html
> `mmap_read_lock(&init_mm)` should be able to ensure that the kernel
> address's page tables can be traversed. But I'm not quite sure if

I added a section specifically about this -

https://docs.kernel.org/mm/process_addrs.html#traversing-non-vma-page-tables

But note:

"Since, aside from vmalloc and memory hot plug, kernel page tables are not torn
down all that often - this usually suffices, however any caller of this
functionality must ensure that any additionally required locks are acquired in
advance."

With the latter part being particularly important - you really need to be sure
you aren't going to be raced on page table teardown by anything.

However:

	* You're safe from vmalloc trying to install a huge page table (only way
	  it removes intermediate page tables) since !HAVE_ARCH_HUGE_VMAP.

	* And since arm32 !ARCH_ENABLE_MEMORY_HOTPLUG you're safe from that too
          :)

(Really I think you should be using walk_page_range_debug() here ultimately but
that's a future refactor).

BUT see below:

> `mmap_read_lock(&current->mm)` provides protection for user-space non-VMA
> addresses?

OK so this _does_ need addressing, and I covered it in the document:

	We also permit a truly unusual case is the traversal of non-VMA ranges
	in userland ranges, as provided for by walk_page_range_debug().

	We must take great care in this case, as the munmap() implementation
	detaches VMAs under an mmap write lock before tearing down page tables
	under a downgraded mmap read lock.

	This means such an operation could race with this, and thus an mmap
	write lock is required.

I.e. you need a write lock.

So in conclusion the patch should be:

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index e62cc4be5a..1f2a85e1fa 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -181,7 +181,9 @@ __do_user_fault(unsigned long addr, unsigned int fsr, unsigned int sig,
 		pr_err("8<--- cut here ---\n");
 		pr_err("%s: unhandled page fault (%d) at 0x%08lx, code 0x%03x\n",
 		       tsk->comm, sig, addr, fsr);
+		mmap_write_lock(tsk->mm);
 		show_pte(KERN_ERR, tsk->mm, addr);
+		mmap_write_unlock(tsk->mm);
 		show_regs(regs);
 	}
 #endif

>
> Also cc to mm maintainers:
> Cc: David Hildenbrand <david@kernel.org>
> Cc: Lorenzo Stoakes <ljs@kernel.org>
> Cc: Liam R. Howlett <liam@infradead.org>
> Cc: Vlastimil Babka <vbabka@kernel.org>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Suren Baghdasaryan <surenb@google.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Linus Walleij <linusw@kernel.org>

Thanks :)

Cheers, Lorenzo