[PATCH v4] ARM: mm: acquire mmap write lock around show_pte() for user faults

[PATCH v4] ARM: mm: acquire mmap write lock around show_pte() for user faults

[Xie Yuanbin]

When CONFIG_DEBUG_USER=y, and cmdline "user_debug=31" is set,
a user fault may trigger show_pte() without any lock.
If another thread in the same process concurrently calls munmap(),
the page table pages may be freed while show_pte() is still traversing
them, causing a use-after-free in show_pte().

If CONFIG_ARM_LPAE=y, this may cause a kernel panic if the pages table
of PMD are freed when show_pte() is running.

Acquire mmap_write_lock() around show_pte() for user faults to fix the
contention.

For user faults, additionally restrict that show_pte() is called only
when the addr is a user-space address (addr < TASK_SIZE). This is because
the lock of tsk->mm only protects the virtual memory of user address space,
furthermore, dumping the page tables of a kernel-space address for user
faults is unnecessary and may have security implications.

Keep everything unchanged for kernel faults, because the kernel is
already in the "oops" state, acquiring a lock may risk a deadlock.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 6d021b724481 ("ARM: dump pgd, pmd and pte states on unhandled data abort faults")
Depends-on: <20260629123349.134224-1-xieyuanbin1@huawei.com>
Depends-on: 59e4f3b45b96 ("ARM: ensure interrupts are enabled in __do_user_fault()")

Cc: Russell King <linux@armlinux.org.uk>
Suggested-by: Lorenzo Stoakes <ljs@kernel.org>
Co-developed-by: Qi Xi <xiqi2@huawei.com>
Signed-off-by: Qi Xi <xiqi2@huawei.com>
Signed-off-by: Xie Yuanbin <xieyuanbin1@huawei.com>
---
For __do_user_fault(), it must be a user fault, so the
judgement of user_mode(regs) can be omitted.

For do_DataAbort(), it may be a user fault or a kernel fault. If it is
a kernel fault, it must be a kernel oops, the kernel will die soon.
Also, a user program may be constructed to trigger
do_DataAbort()->show_pte(), as described in this mail:
Link: https://lore.kernel.org/20260625100031.25088-1-xieyuanbin1@huawei.com

For user faults, it is safe to acquire mmap_write_lock(), because
we never return to user with mmap lock held or pagefault disabled.

Note that, due to the lack of ARMv4T for testing, this patch does not
fix the issue in baddataabort()->show_pte().

V3->V4:
  - Keep nothing change for kernel faults.
  - Change mmap read lock to write lock.
  - Restrict that show_pte() is called only when the addr
    is a user-space address (addr < TASK_SIZE).

 arch/arm/mm/fault.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index c68677503532..0a09d4ff7718 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -174,21 +174,25 @@ __do_user_fault(unsigned long addr, unsigned int fsr, unsigned int sig,
 	struct task_struct *tsk = current;
 
 	local_irq_enable();
 
 #ifdef CONFIG_DEBUG_USER
 	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
 	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {
 		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);
-		show_pte(KERN_ERR, tsk->mm, addr);
+		if (likely(addr < TASK_SIZE)) {
+			mmap_write_lock(tsk->mm);
+			show_pte(KERN_ERR, tsk->mm, addr);
+			mmap_write_unlock(tsk->mm);
+		}
 		show_regs(regs);
 	}
 #endif
 #ifndef CONFIG_KUSER_HELPERS
 	if ((sig == SIGSEGV) && ((addr & PAGE_MASK) == 0xffff0000))
 		printk_ratelimited(KERN_DEBUG
 				   "%s: CONFIG_KUSER_HELPERS disabled at 0x%08lx\n",
 				   tsk->comm, addr);
 #endif
 
@@ -632,21 +636,29 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 
 	if (!inf->fn(addr, fsr & ~FSR_LNX_PF, regs))
 		return;
 
 	if (likely(user_mode(regs)))
 		local_irq_enable();
 
 	pr_alert("8<--- cut here ---\n");
 	pr_alert("Unhandled fault: %s (0x%03x) at 0x%08lx\n",
 		inf->name, fsr, addr);
-	show_pte(KERN_ALERT, current->mm, addr);
+	if (likely(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);
+	}
 
 	arm_notify_die("", regs, inf->sig, inf->code, (void __user *)addr,
 		       fsr, 0);
 }
 
 void __init
 hook_ifault_code(int nr, int (*fn)(unsigned long, unsigned int, struct pt_regs *),
 		 int sig, int code, const char *name)
 {
 	if (nr < 0 || nr >= ARRAY_SIZE(ifsr_info))
-- 
2.53.0

Re: [PATCH v4] ARM: mm: acquire mmap write lock around show_pte() for user faults

[Lorenzo Stoakes (ARM)]

On Thu, Jul 16, 2026 at 09:40:22AM +0800, Xie Yuanbin wrote:
> When CONFIG_DEBUG_USER=y, and cmdline "user_debug=31" is set,
> a user fault may trigger show_pte() without any lock.
> If another thread in the same process concurrently calls munmap(),
> the page table pages may be freed while show_pte() is still traversing
> them, causing a use-after-free in show_pte().
>
> If CONFIG_ARM_LPAE=y, this may cause a kernel panic if the pages table
> of PMD are freed when show_pte() is running.
>
> Acquire mmap_write_lock() around show_pte() for user faults to fix the
> contention.
>
> For user faults, additionally restrict that show_pte() is called only
> when the addr is a user-space address (addr < TASK_SIZE). This is because
> the lock of tsk->mm only protects the virtual memory of user address space,
> furthermore, dumping the page tables of a kernel-space address for user
> faults is unnecessary and may have security implications.
>
> Keep everything unchanged for kernel faults, because the kernel is
> already in the "oops" state, acquiring a lock may risk a deadlock.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Fixes: 6d021b724481 ("ARM: dump pgd, pmd and pte states on unhandled data abort faults")
> Depends-on: <20260629123349.134224-1-xieyuanbin1@huawei.com>
> Depends-on: 59e4f3b45b96 ("ARM: ensure interrupts are enabled in __do_user_fault()")
>
> Cc: Russell King <linux@armlinux.org.uk>
> Suggested-by: Lorenzo Stoakes <ljs@kernel.org>
> Co-developed-by: Qi Xi <xiqi2@huawei.com>
> Signed-off-by: Qi Xi <xiqi2@huawei.com>
> Signed-off-by: Xie Yuanbin <xieyuanbin1@huawei.com>

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
> For __do_user_fault(), it must be a user fault, so the
> judgement of user_mode(regs) can be omitted.
>
> For do_DataAbort(), it may be a user fault or a kernel fault. If it is
> a kernel fault, it must be a kernel oops, the kernel will die soon.
> Also, a user program may be constructed to trigger
> do_DataAbort()->show_pte(), as described in this mail:
> Link: https://lore.kernel.org/20260625100031.25088-1-xieyuanbin1@huawei.com
>
> For user faults, it is safe to acquire mmap_write_lock(), because
> we never return to user with mmap lock held or pagefault disabled.
>
> Note that, due to the lack of ARMv4T for testing, this patch does not
> fix the issue in baddataabort()->show_pte().
>
> V3->V4:
>   - Keep nothing change for kernel faults.
>   - Change mmap read lock to write lock.
>   - Restrict that show_pte() is called only when the addr
>     is a user-space address (addr < TASK_SIZE).
>
>  arch/arm/mm/fault.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index c68677503532..0a09d4ff7718 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -174,21 +174,25 @@ __do_user_fault(unsigned long addr, unsigned int fsr, unsigned int sig,
>  	struct task_struct *tsk = current;
>
>  	local_irq_enable();
>
>  #ifdef CONFIG_DEBUG_USER
>  	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
>  	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {
>  		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);
> -		show_pte(KERN_ERR, tsk->mm, addr);
> +		if (likely(addr < TASK_SIZE)) {
> +			mmap_write_lock(tsk->mm);
> +			show_pte(KERN_ERR, tsk->mm, addr);
> +			mmap_write_unlock(tsk->mm);
> +		}
>  		show_regs(regs);
>  	}
>  #endif
>  #ifndef CONFIG_KUSER_HELPERS
>  	if ((sig == SIGSEGV) && ((addr & PAGE_MASK) == 0xffff0000))
>  		printk_ratelimited(KERN_DEBUG
>  				   "%s: CONFIG_KUSER_HELPERS disabled at 0x%08lx\n",
>  				   tsk->comm, addr);
>  #endif
>
> @@ -632,21 +636,29 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>
>  	if (!inf->fn(addr, fsr & ~FSR_LNX_PF, regs))
>  		return;
>
>  	if (likely(user_mode(regs)))
>  		local_irq_enable();
>
>  	pr_alert("8<--- cut here ---\n");
>  	pr_alert("Unhandled fault: %s (0x%03x) at 0x%08lx\n",
>  		inf->name, fsr, addr);
> -	show_pte(KERN_ALERT, current->mm, addr);
> +	if (likely(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);
> +	}
>
>  	arm_notify_die("", regs, inf->sig, inf->code, (void __user *)addr,
>  		       fsr, 0);
>  }
>
>  void __init
>  hook_ifault_code(int nr, int (*fn)(unsigned long, unsigned int, struct pt_regs *),
>  		 int sig, int code, const char *name)
>  {
>  	if (nr < 0 || nr >= ARRAY_SIZE(ifsr_info))
> --
> 2.53.0
>

Cheers, Lorenzo

Re: [PATCH v4] ARM: mm: acquire mmap write lock around show_pte() for user faults

[Linus Walleij]

On Thu, Jul 16, 2026 at 3:42 AM Xie Yuanbin <xieyuanbin1@huawei.com> wrote:

> When CONFIG_DEBUG_USER=y, and cmdline "user_debug=31" is set,
> a user fault may trigger show_pte() without any lock.
> If another thread in the same process concurrently calls munmap(),
> the page table pages may be freed while show_pte() is still traversing
> them, causing a use-after-free in show_pte().
>
> If CONFIG_ARM_LPAE=y, this may cause a kernel panic if the pages table
> of PMD are freed when show_pte() is running.
>
> Acquire mmap_write_lock() around show_pte() for user faults to fix the
> contention.
>
> For user faults, additionally restrict that show_pte() is called only
> when the addr is a user-space address (addr < TASK_SIZE). This is because
> the lock of tsk->mm only protects the virtual memory of user address space,
> furthermore, dumping the page tables of a kernel-space address for user
> faults is unnecessary and may have security implications.
>
> Keep everything unchanged for kernel faults, because the kernel is
> already in the "oops" state, acquiring a lock may risk a deadlock.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Fixes: 6d021b724481 ("ARM: dump pgd, pmd and pte states on unhandled data abort faults")
> Depends-on: <20260629123349.134224-1-xieyuanbin1@huawei.com>
> Depends-on: 59e4f3b45b96 ("ARM: ensure interrupts are enabled in __do_user_fault()")

This looks a bit odd. I don't get the first fix tag, drop it.
I think drop both Depends-on as well.

> Cc: Russell King <linux@armlinux.org.uk>
> Suggested-by: Lorenzo Stoakes <ljs@kernel.org>
> Co-developed-by: Qi Xi <xiqi2@huawei.com>
> Signed-off-by: Qi Xi <xiqi2@huawei.com>
> Signed-off-by: Xie Yuanbin <xieyuanbin1@huawei.com>

Looks correct to me otherwise.

Reviewed-by: Linus Walleij <linusw@kernel.org>

Put it in Russell's patch tracker after dropping the weird
tags.

Yours,
Linus Walleij

Re: [PATCH v4] ARM: mm: acquire mmap write lock around show_pte() for user faults

[Xie Yuanbin]

On Sat, 25 Jul 2026 00:05:58 +0200, Linus Walleij wrote:
> On Thu, 16 Jul 2026 09:40:22 +0800, Xie Yuanbin <xieyuanbin1@huawei.com> wrote:
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Fixes: 6d021b724481 ("ARM: dump pgd, pmd and pte states on unhandled data abort faults")
>> Depends-on: <20260629123349.134224-1-xieyuanbin1@huawei.com>
>> Depends-on: 59e4f3b45b96 ("ARM: ensure interrupts are enabled in __do_user_fault()")
>
> This looks a bit odd. I don't get the first fix tag, drop it.

The first fix tag can be open as:
Link: https://github.com/torvalds/linux/commit/1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
Link: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
It seems that a 12-character hash is no longer sufficient to index it. :)

May I increase the number of hash characters or just drop it?

> I think drop both Depends-on as well.

These tags are for the maintainers of the stable/LTS branches. Since
the bug is very old (we discovered it in linux-5.10), they are likely to
be backported to the LTS versions. However, the depends-on commits may
not be merged to the LTS versions.

If the depends-on commits are not merged, this patch may trigger a
might_sleep() warning when calling mmap_write_lock().

Do I still need to drop them?

> Put it in Russell's patch tracker after dropping the weird
> tags.

Just to confirm, I will be the one to submit this patch to this website:
Link: https://www.armlinux.org.uk/developer/patches
right?

Thanks very much!