From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Xie Yuanbin <xieyuanbin1@huawei.com>
Cc: linux@armlinux.org.uk, akpm@linux-foundation.org,
xiqi2@huawei.com, david@kernel.org, linusw@kernel.org,
liam@infradead.org, vbabka@kernel.org, rppt@kernel.org,
surenb@google.com, mhocko@suse.com, will@kernel.org,
linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org, liaohua4@huawei.com,
lilinjie8@huawei.com, sunnanyong@huawei.com
Subject: Re: [PATCH v4] ARM: mm: acquire mmap write lock around show_pte() for user faults
Date: Thu, 16 Jul 2026 14:47:24 +0100 [thread overview]
Message-ID: <aljg0sddPohJC5ij@lucifer> (raw)
In-Reply-To: <20260716014022.2823-1-xieyuanbin1@huawei.com>
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
prev parent reply other threads:[~2026-07-16 13:47 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-16 1:40 [PATCH v4] ARM: mm: acquire mmap write lock around show_pte() for user faults Xie Yuanbin
2026-07-16 13:47 ` Lorenzo Stoakes (ARM) [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aljg0sddPohJC5ij@lucifer \
--to=ljs@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=david@kernel.org \
--cc=liam@infradead.org \
--cc=liaohua4@huawei.com \
--cc=lilinjie8@huawei.com \
--cc=linusw@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux@armlinux.org.uk \
--cc=mhocko@suse.com \
--cc=rppt@kernel.org \
--cc=sunnanyong@huawei.com \
--cc=surenb@google.com \
--cc=vbabka@kernel.org \
--cc=will@kernel.org \
--cc=xieyuanbin1@huawei.com \
--cc=xiqi2@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.