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

From: Qi Xi

Date: Tue Jul 07 2026 - 08:50:21 EST

On 07/07/2026 19:57, Russell King wrote:

On Tue, Jul 07, 2026 at 07:48:12PM +0800, Qi Xi wrote:

On 06/07/2026 21:32, 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
`mmap_read_lock(&current->mm)` provides protection for user-space non-VMA
addresses?

You're right. And I think the fix is to simply skip show_pte() for kernel
addresses.

No. This information is useful debug for kernel oops.

For addr >= TASK_SIZE, concurrent munmap cannot free the kernel page tables.
So there is no uaf risk, and show_pte() is still called without the lock as before.

Does the following change look acceptable?
if (user_mode(regs) && addr < TASK_SIZE) {
mmap_read_lock(current->mm);
show_pte(KERN_ALERT, current->mm, addr);
mmap_read_unlock(current->mm);
} else {
// .. keep nothing change
show_pte(KERN_ALERT, current->mm, addr);
}