Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
From: Qi Xi
Date: Thu Jul 09 2026 - 22:34:43 EST
On 07/07/2026 23:34, Russell King wrote:
On Tue, Jul 07, 2026 at 02:20:19PM +0100, Lorenzo Stoakes wrote:
On Tue, Jul 07, 2026 at 09:14:09PM +0800, Xie Yuanbin wrote:Unconditionally taking the lock could lead to a deadlock. Consider
On Tue, 7 Jul 2026 12:57:45 +0100, Russell King wrote:Well the whole reason you're faulting here might be because a userland process
No. This information is useful debug for kernel oops.For kernel oops, I think it should be `!user_mode(regs)`, Qi Xi's reply:
On Tue, 7 Jul 2026 19:48:12 +0800, Qi Xi wrote:
For do_DataAbort() fallback:changes nothing to kernel oops. It only skip show_pte() for user-mode
if (user_mode(regs)) {
if (addr < TASK_SIZE) {
mmap_read_lock(current->mm);
show_pte(KERN_ALERT, current->mm, addr);
mmap_read_unlock(current->mm);
}
} else {
show_pte(KERN_ALERT, current->mm, addr);
}
faults, and the fault addr is a kernel address, which means a user
program is trying to access a kernel address.
I think it is reasonable to skip show_pte() in this case?
did that right? The page tables should tell you (presumably on ARM32 :)
And I hate to repeat myself, maybe you didn't read the whole thread but... just
use mmap_write_lock(), this isn't necessary?
What is this trying to achieve?
You're not in a hotpath, why are you bothering to conditionally take/not take
the lock?
the case where the mmap lock is held, and we get an unrecognised
abort from the kernel.
If we try to take the mmap lock again, we'll deadlock, which will
result in very little debug information being output - and the
system locks up. The only thing that would save such a case would
be if the user had decided to use a hardware watchdog, or is
physically present to press the reset button.
We are preparing a v4 and would like to confirm the approach for
the do_DataAbort() fallback path (__do_user_fault() is similar).
As Lorenzo noted, an mmap write lock is required here because
munmap() downgrades the write lock to a read lock before tearing
down page tables.
- show_pte(KERN_ALERT, current->mm, addr);
+ if (user_mode(regs)) {
+ if (addr < TASK_SIZE) {
+ mmap_write_lock(current->mm);
+ show_pte(KERN_ALERT, current->mm, addr);
+ mmap_write_unlock(current->mm);
+ }
+ } else {
+ show_pte(KERN_ALERT, current->mm, addr);
+ }
The lock is taken only for user_mode(regs) + addr < TASK_SIZE, so
kernel aborts that may already hold the mmap lock are left unchanged.
For user-mode faults on kernel addresses (addr >= TASK_SIZE), as
Yuanbin noted, it is reasonable to skip show_pte().
Please let us know if you see any issues with this approach, or if you
would suggest a different way to handle it.