The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v3 0/2] ARM: mm: fix use-after-free in show_pte()
@ 2026-06-26  7:30 Qi Xi
  2026-06-26  7:30 ` [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER Qi Xi
  2026-06-26  7:30 ` [PATCH v3 2/2] ARM: mm: protect show_pte() in do_DataAbort() fallback path Qi Xi
  0 siblings, 2 replies; 21+ messages in thread
From: Qi Xi @ 2026-06-26  7:30 UTC (permalink / raw)
  To: Russell King, Andrew Morton
  Cc: linux-arm-kernel, linux-kernel, Yuanbin Xie, Nanyong Sun, Qi Xi

This series fixes a use-after-free in show_pte() on 32-bit ARM.

show_pte() is called from __do_user_fault() after do_page_fault() has
already released mmap_read_lock. If another thread concurrently calls
munmap(), the page table pages can be freed while show_pte() is still
walking them, causing a use-after-free.

Patch 1 fixes the main path (__do_user_fault) with mmap_read_lock().
Patch 2 protects the do_DataAbort() fallback path with
mmap_read_trylock(), which enters show_pte() only for rare FSR types
(fsr_info entries with fn=do_bad).

v3: Split into two patches.
v2: Also fix do_DataAbort() fallback path.

Qi Xi (2):
  ARM: mm: fix use-after-free in __do_user_fault() under
    CONFIG_DEBUG_USER
  ARM: mm: protect show_pte() in do_DataAbort() fallback path

 arch/arm/mm/fault.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

-- 
2.33.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-06-26  7:30 [PATCH v3 0/2] ARM: mm: fix use-after-free in show_pte() Qi Xi
@ 2026-06-26  7:30 ` Qi Xi
  2026-06-26  9:44   ` Russell King
  2026-07-06 13:32   ` Xie Yuanbin
  2026-06-26  7:30 ` [PATCH v3 2/2] ARM: mm: protect show_pte() in do_DataAbort() fallback path Qi Xi
  1 sibling, 2 replies; 21+ messages in thread
From: Qi Xi @ 2026-06-26  7:30 UTC (permalink / raw)
  To: Russell King, Andrew Morton
  Cc: linux-arm-kernel, linux-kernel, Yuanbin Xie, Nanyong Sun, Qi Xi

When CONFIG_DEBUG_USER is enabled with user_debug=31 on 32-bit ARM,
a user page fault triggers show_pte() via __do_user_fault() after
do_page_fault() has already released mmap_read_lock. If another
thread concurrently calls munmap(), the page table pages can be
freed while show_pte() is still reading them, causing a
use-after-free in show_pte().

The race can be reproduced on multi_v7_defconfig with:
    CONFIG_DEBUG_USER=y
    CONFIG_ARM_LPAE=y
    kernel command line: user_debug=31

A delay inserted in show_pte() for testing widens the race window and
makes the UAF reliably reproducible. On LPAE, the race works as
follows:

  CPU 0 (fault path)                       CPU 1 (munmap)
  munmap(page 0) -> clears PTE[0]
  PTE/PMD pages remain

  read page 0 -> page fault
    -> do_DataAbort()
      -> do_page_fault()
        -> lock_mm_and_find_vma() -> no VMA
           (mmap_read_lock released)
        -> __do_user_fault()
          -> show_pte(tsk->mm, addr)
            -> *pgd (valid)
            -> p4d/pud checks pass

            -> [delay]                     munmap(page 1)
                                             -> clears PTE[1]
                                             -> PTE/PMD pages freed
                                             -> PGD cleared

            -> pmd_offset(pud, addr)
              -> *pud=0 -> __va(0)
              -> dereference
              -> secondary data abort (kernel)

Fix by taking mmap_read_lock() around show_pte() in __do_user_fault().
__do_user_fault() is called from process context with interrupts
enabled, so the context can sleep and mmap_read_lock() is safe here.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Tested-by: Yuanbin Xie <xieyuanbin1@huawei.com>
Suggested-by: Yuanbin Xie <xieyuanbin1@huawei.com>
Signed-off-by: Qi Xi <xiqi2@huawei.com>
---
 arch/arm/mm/fault.c | 2 ++
 1 file changed, 2 insertions(+)

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_read_lock(tsk->mm);
 		show_pte(KERN_ERR, tsk->mm, addr);
+		mmap_read_unlock(tsk->mm);
 		show_regs(regs);
 	}
 #endif
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 2/2] ARM: mm: protect show_pte() in do_DataAbort() fallback path
  2026-06-26  7:30 [PATCH v3 0/2] ARM: mm: fix use-after-free in show_pte() Qi Xi
  2026-06-26  7:30 ` [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER Qi Xi
@ 2026-06-26  7:30 ` Qi Xi
  2026-06-26  9:45   ` Russell King
  2026-06-26 10:16   ` Xie Yuanbin
  1 sibling, 2 replies; 21+ messages in thread
From: Qi Xi @ 2026-06-26  7:30 UTC (permalink / raw)
  To: Russell King, Andrew Morton
  Cc: linux-arm-kernel, linux-kernel, Yuanbin Xie, Nanyong Sun, Qi Xi

The do_DataAbort() fallback path handles FSR types not serviced by
do_page_fault() (fsr_info entries with fn=do_bad). This path also
calls show_pte() without holding mmap_read_lock, exposing it to
the same use-after-free issue.

Since do_DataAbort() is an exception entry point that can be reached
from contexts where sleeping is not allowed, use mmap_read_trylock().
If the lock cannot be acquired, the page table dump is skipped.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Suggested-by: Yuanbin Xie <xieyuanbin1@huawei.com>
Signed-off-by: Qi Xi <xiqi2@huawei.com>
---
 arch/arm/mm/fault.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 1f2a85e1fa..0a8fc40afe 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -638,7 +638,10 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	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 (mmap_read_trylock(current->mm)) {
+		show_pte(KERN_ALERT, current->mm, addr);
+		mmap_read_unlock(current->mm);
+	}
 
 	arm_notify_die("", regs, inf->sig, inf->code, (void __user *)addr,
 		       fsr, 0);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-06-26  7:30 ` [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER Qi Xi
@ 2026-06-26  9:44   ` Russell King
  2026-06-27  1:39     ` Qi Xi
  2026-07-06 13:32   ` Xie Yuanbin
  1 sibling, 1 reply; 21+ messages in thread
From: Russell King @ 2026-06-26  9:44 UTC (permalink / raw)
  To: Qi Xi
  Cc: Andrew Morton, linux-arm-kernel, linux-kernel, Yuanbin Xie,
	Nanyong Sun

On Fri, Jun 26, 2026 at 03:30:47PM +0800, Qi Xi wrote:
> When CONFIG_DEBUG_USER is enabled with user_debug=31 on 32-bit ARM,
> a user page fault triggers show_pte() via __do_user_fault() after
> do_page_fault() has already released mmap_read_lock. If another
> thread concurrently calls munmap(), the page table pages can be
> freed while show_pte() is still reading them, causing a
> use-after-free in show_pte().
> 
> The race can be reproduced on multi_v7_defconfig with:
>     CONFIG_DEBUG_USER=y
>     CONFIG_ARM_LPAE=y
>     kernel command line: user_debug=31
> 
> A delay inserted in show_pte() for testing widens the race window and
> makes the UAF reliably reproducible. On LPAE, the race works as
> follows:
> 
>   CPU 0 (fault path)                       CPU 1 (munmap)
>   munmap(page 0) -> clears PTE[0]
>   PTE/PMD pages remain
> 
>   read page 0 -> page fault
>     -> do_DataAbort()
>       -> do_page_fault()
>         -> lock_mm_and_find_vma() -> no VMA
>            (mmap_read_lock released)
>         -> __do_user_fault()
>           -> show_pte(tsk->mm, addr)
>             -> *pgd (valid)
>             -> p4d/pud checks pass
> 
>             -> [delay]                     munmap(page 1)
>                                              -> clears PTE[1]
>                                              -> PTE/PMD pages freed
>                                              -> PGD cleared
> 
>             -> pmd_offset(pud, addr)
>               -> *pud=0 -> __va(0)
>               -> dereference
>               -> secondary data abort (kernel)
> 
> Fix by taking mmap_read_lock() around show_pte() in __do_user_fault().
> __do_user_fault() is called from process context with interrupts
> enabled, so the context can sleep and mmap_read_lock() is safe here.

This is a fault path which should only be called when something is
already wrong, the mm lock may already be held (e.g. a kernel
fault while already holding the mmap lock.) We can't take any locks
here.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/2] ARM: mm: protect show_pte() in do_DataAbort() fallback path
  2026-06-26  7:30 ` [PATCH v3 2/2] ARM: mm: protect show_pte() in do_DataAbort() fallback path Qi Xi
@ 2026-06-26  9:45   ` Russell King
  2026-06-26 10:16   ` Xie Yuanbin
  1 sibling, 0 replies; 21+ messages in thread
From: Russell King @ 2026-06-26  9:45 UTC (permalink / raw)
  To: Qi Xi
  Cc: Andrew Morton, linux-arm-kernel, linux-kernel, Yuanbin Xie,
	Nanyong Sun

On Fri, Jun 26, 2026 at 03:30:48PM +0800, Qi Xi wrote:
> The do_DataAbort() fallback path handles FSR types not serviced by
> do_page_fault() (fsr_info entries with fn=do_bad). This path also
> calls show_pte() without holding mmap_read_lock, exposing it to
> the same use-after-free issue.
> 
> Since do_DataAbort() is an exception entry point that can be reached
> from contexts where sleeping is not allowed, use mmap_read_trylock().
> If the lock cannot be acquired, the page table dump is skipped.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Suggested-by: Yuanbin Xie <xieyuanbin1@huawei.com>
> Signed-off-by: Qi Xi <xiqi2@huawei.com>

Same reason as patch 1. We can't take locks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/2] ARM: mm: protect show_pte() in do_DataAbort() fallback path
  2026-06-26  7:30 ` [PATCH v3 2/2] ARM: mm: protect show_pte() in do_DataAbort() fallback path Qi Xi
  2026-06-26  9:45   ` Russell King
@ 2026-06-26 10:16   ` Xie Yuanbin
  2026-06-26 12:37     ` Russell King
  1 sibling, 1 reply; 21+ messages in thread
From: Xie Yuanbin @ 2026-06-26 10:16 UTC (permalink / raw)
  To: xiqi2, linux
  Cc: akpm, linux-arm-kernel, linux-kernel, sunnanyong, xieyuanbin1

On Fri, 26 Jun 2026 15:30:48 +0800, Qi Xi wrote:
> @@ -638,7 +638,10 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> 	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 (mmap_read_trylock(current->mm)) {
> +		show_pte(KERN_ALERT, current->mm, addr);
> +		mmap_read_unlock(current->mm);
> +	}

For kernel faults, `current->mm` maybe NULL, and
`mmap_read_trylock(current->mm)` may cause a panic.
Also, interrupts may be disabled here.

I suggest that waiting for this patch to be merged first:
https://lore.kernel.org/20260625122612.43501-1-xieyuanbin1@huawei.com
which make sure that interrupts are enabled here.
Then, something like:
```c
	if (user_mode(regs))
		mmap_read_lock(current->mm);
	show_pte(KERN_ALERT, current->mm, addr);
	if (user_mode(regs))
		mmap_read_unlock(current->mm);
```

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/2] ARM: mm: protect show_pte() in do_DataAbort() fallback path
  2026-06-26 10:16   ` Xie Yuanbin
@ 2026-06-26 12:37     ` Russell King
  2026-06-27  1:22       ` Xie Yuanbin
  0 siblings, 1 reply; 21+ messages in thread
From: Russell King @ 2026-06-26 12:37 UTC (permalink / raw)
  To: Xie Yuanbin; +Cc: xiqi2, akpm, linux-arm-kernel, linux-kernel, sunnanyong

On Fri, Jun 26, 2026 at 06:16:15PM +0800, Xie Yuanbin wrote:
> On Fri, 26 Jun 2026 15:30:48 +0800, Qi Xi wrote:
> > @@ -638,7 +638,10 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> > 	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 (mmap_read_trylock(current->mm)) {
> > +		show_pte(KERN_ALERT, current->mm, addr);
> > +		mmap_read_unlock(current->mm);
> > +	}
> 
> For kernel faults, `current->mm` maybe NULL, and
> `mmap_read_trylock(current->mm)` may cause a panic.
> Also, interrupts may be disabled here.
> 
> I suggest that waiting for this patch to be merged first:
> https://lore.kernel.org/20260625122612.43501-1-xieyuanbin1@huawei.com
> which make sure that interrupts are enabled here.

No, it doesn't ensure that.

show_pte() is also called from die_kernel_fault() and
__do_kernel_fault(), which can be from a path that has interrupts
disabled. See the comments in do_kernel_address_page_fault().

We are not "fixing" show_pte(), which is a diagnostic function when
things go wrong in the kernel, and is there to *try* to give us
information to diagnose what happened. It is *not* a function that
is used routinely in the kernel.

The fact it is racy with other CPUs may be a problem, but it isn't
a big problem because when it's called, the system is practically
dead anyway.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/2] ARM: mm: protect show_pte() in do_DataAbort() fallback path
  2026-06-26 12:37     ` Russell King
@ 2026-06-27  1:22       ` Xie Yuanbin
  0 siblings, 0 replies; 21+ messages in thread
From: Xie Yuanbin @ 2026-06-27  1:22 UTC (permalink / raw)
  To: linux; +Cc: akpm, linux-arm-kernel, linux-kernel, sunnanyong, xieyuanbin1,
	xiqi2

On Fri, 26 Jun 2026 13:37:00 +0100, Russell King wrote:
> On Fri, Jun 26, 2026 at 06:16:15PM +0800, Xie Yuanbin wrote:
>> I suggest that waiting for this patch to be merged first:
>> https://lore.kernel.org/20260625122612.43501-1-xieyuanbin1@huawei.com
>> which make sure that interrupts are enabled here.
>
> No, it doesn't ensure that.

Oh, I'm sorry my description wasn't clear. It should make sure that
for user unhandled faults, interrupts are enabled here, without
kernel unhandled faults.

> We are not "fixing" show_pte(), which is a diagnostic function when
> things go wrong in the kernel, and is there to *try* to give us
> information to diagnose what happened. It is *not* a function that
> is used routinely in the kernel.

For kernel faults, I fully agree with it. However, for
user faults, I think "fixing" is necessary.
We might be able to construct a user-mode program in some way to
repeatedly trigger unhandled faults -> show_pte(), such as executing bkpt
instruction, or something else, and then the kernel panic, this may be a
security vulnerability.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-06-26  9:44   ` Russell King
@ 2026-06-27  1:39     ` Qi Xi
  0 siblings, 0 replies; 21+ messages in thread
From: Qi Xi @ 2026-06-27  1:39 UTC (permalink / raw)
  To: Russell King
  Cc: Andrew Morton, linux-arm-kernel, linux-kernel, Yuanbin Xie,
	Nanyong Sun

Hi Russell,

Thank you for the review. I understand the general concern about
taking locks in fault paths, but I would like to clarify the specific
case here.

__do_user_fault() with CONFIG_DEBUG_USER is not a kernel-dying path.
After show_pte() prints debug info, the kernel calls
force_sig_fault(SIGSEGV) and returns to user space. The system
continues running normally. Without this fix, a concurrent munmap can
cause show_pte() to trigger a secondary kernel fault, turning a
harmless SIGSEGV into a kernel panic.

Regarding your concern about the mm lock being already held: I have
verified that all three callers of __do_user_fault() (do_page_fault
-> bad_area, do_bad_area user path, and do_kernel_address_page_fault
user path) release mmap_read_lock or never hold it before entering
__do_user_fault(). So the lock is not held here.

It is also worth noting that we did NOT modify the paths where the
kernel is already dying (die_kernel_fault, __do_kernel_fault). Those
paths remain unchanged and continue to call show_pte() without any
lock, just as they always have.

On 26/06/2026 17:44, Russell King wrote:
> On Fri, Jun 26, 2026 at 03:30:47PM +0800, Qi Xi wrote:
>> When CONFIG_DEBUG_USER is enabled with user_debug=31 on 32-bit ARM,
>> a user page fault triggers show_pte() via __do_user_fault() after
>> do_page_fault() has already released mmap_read_lock. If another
>> thread concurrently calls munmap(), the page table pages can be
>> freed while show_pte() is still reading them, causing a
>> use-after-free in show_pte().
>>
>> The race can be reproduced on multi_v7_defconfig with:
>>      CONFIG_DEBUG_USER=y
>>      CONFIG_ARM_LPAE=y
>>      kernel command line: user_debug=31
>>
>> A delay inserted in show_pte() for testing widens the race window and
>> makes the UAF reliably reproducible. On LPAE, the race works as
>> follows:
>>
>>    CPU 0 (fault path)                       CPU 1 (munmap)
>>    munmap(page 0) -> clears PTE[0]
>>    PTE/PMD pages remain
>>
>>    read page 0 -> page fault
>>      -> do_DataAbort()
>>        -> do_page_fault()
>>          -> lock_mm_and_find_vma() -> no VMA
>>             (mmap_read_lock released)
>>          -> __do_user_fault()
>>            -> show_pte(tsk->mm, addr)
>>              -> *pgd (valid)
>>              -> p4d/pud checks pass
>>
>>              -> [delay]                     munmap(page 1)
>>                                               -> clears PTE[1]
>>                                               -> PTE/PMD pages freed
>>                                               -> PGD cleared
>>
>>              -> pmd_offset(pud, addr)
>>                -> *pud=0 -> __va(0)
>>                -> dereference
>>                -> secondary data abort (kernel)
>>
>> Fix by taking mmap_read_lock() around show_pte() in __do_user_fault().
>> __do_user_fault() is called from process context with interrupts
>> enabled, so the context can sleep and mmap_read_lock() is safe here.
> This is a fault path which should only be called when something is
> already wrong, the mm lock may already be held (e.g. a kernel
> fault while already holding the mmap lock.) We can't take any locks
> here.
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-06-26  7:30 ` [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER Qi Xi
  2026-06-26  9:44   ` Russell King
@ 2026-07-06 13:32   ` Xie Yuanbin
  2026-07-07 11:48     ` Qi Xi
  2026-07-07 12:46     ` Lorenzo Stoakes
  1 sibling, 2 replies; 21+ messages in thread
From: Xie Yuanbin @ 2026-07-06 13:32 UTC (permalink / raw)
  To: xiqi2, akpm, linux, david, ljs, liam, vbabka, rppt, surenb,
	mhocko, linusw
  Cc: linux-arm-kernel, linux-kernel, sunnanyong, xieyuanbin1, linux-mm,
	lilinjie8, liaohua4

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?

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>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-06 13:32   ` Xie Yuanbin
@ 2026-07-07 11:48     ` Qi Xi
  2026-07-07 11:57       ` Russell King
  2026-07-07 12:46     ` Lorenzo Stoakes
  1 sibling, 1 reply; 21+ messages in thread
From: Qi Xi @ 2026-07-07 11:48 UTC (permalink / raw)
  To: Xie Yuanbin, akpm, linux, david, ljs, liam, vbabka, rppt, surenb,
	mhocko, linusw
  Cc: linux-arm-kernel, linux-kernel, sunnanyong, linux-mm, lilinjie8,
	liaohua4



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.
For do_DataAbort() fallback:

	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);
	}

For a user-mode fault on a kernel address, printing kernel page tables via
show_pte() does not help. And The fault address is already printed above.
So it's safe and reasonable to skip it.

Same pattern applies to __do_user_fault().

> 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>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-07 11:48     ` Qi Xi
@ 2026-07-07 11:57       ` Russell King
  2026-07-07 12:47         ` Qi Xi
                           ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Russell King @ 2026-07-07 11:57 UTC (permalink / raw)
  To: Qi Xi
  Cc: Xie Yuanbin, akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko,
	linusw, linux-arm-kernel, linux-kernel, sunnanyong, linux-mm,
	lilinjie8, liaohua4

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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-06 13:32   ` Xie Yuanbin
  2026-07-07 11:48     ` Qi Xi
@ 2026-07-07 12:46     ` Lorenzo Stoakes
  2026-07-07 13:35       ` Xie Yuanbin
  1 sibling, 1 reply; 21+ messages in thread
From: Lorenzo Stoakes @ 2026-07-07 12:46 UTC (permalink / raw)
  To: Xie Yuanbin
  Cc: xiqi2, akpm, linux, david, liam, vbabka, rppt, surenb, mhocko,
	linusw, linux-arm-kernel, linux-kernel, sunnanyong, linux-mm,
	lilinjie8, liaohua4

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-07 11:57       ` Russell King
@ 2026-07-07 12:47         ` Qi Xi
  2026-07-07 12:47         ` Lorenzo Stoakes
  2026-07-07 13:14         ` Xie Yuanbin
  2 siblings, 0 replies; 21+ messages in thread
From: Qi Xi @ 2026-07-07 12:47 UTC (permalink / raw)
  To: Russell King
  Cc: Xie Yuanbin, akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko,
	linusw, linux-arm-kernel, linux-kernel, sunnanyong, linux-mm,
	lilinjie8, liaohua4


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);
}

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-07 11:57       ` Russell King
  2026-07-07 12:47         ` Qi Xi
@ 2026-07-07 12:47         ` Lorenzo Stoakes
  2026-07-07 13:14         ` Xie Yuanbin
  2 siblings, 0 replies; 21+ messages in thread
From: Lorenzo Stoakes @ 2026-07-07 12:47 UTC (permalink / raw)
  To: Russell King
  Cc: Qi Xi, Xie Yuanbin, akpm, david, liam, vbabka, rppt, surenb,
	mhocko, linusw, linux-arm-kernel, linux-kernel, sunnanyong,
	linux-mm, lilinjie8, liaohua4

On Tue, Jul 07, 2026 at 12:57:45PM +0100, Russell King wrote:
> On Tue, Jul 07, 2026 at 07:48:12PM +0800, Qi Xi wrote:
> > 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.

Yup see my reply, just upgrade it to an mmap write lock and you should be
safe to keep this.

>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Cheers, Lorenzo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-07 11:57       ` Russell King
  2026-07-07 12:47         ` Qi Xi
  2026-07-07 12:47         ` Lorenzo Stoakes
@ 2026-07-07 13:14         ` Xie Yuanbin
  2026-07-07 13:20           ` Lorenzo Stoakes
  2 siblings, 1 reply; 21+ messages in thread
From: Xie Yuanbin @ 2026-07-07 13:14 UTC (permalink / raw)
  To: linux
  Cc: akpm, david, liam, liaohua4, lilinjie8, linusw, linux-arm-kernel,
	linux-kernel, linux-mm, ljs, mhocko, rppt, sunnanyong, surenb,
	vbabka, xieyuanbin1, xiqi2

On Tue, 7 Jul 2026 12:57:45 +0100, Russell King wrote:
> 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:
>
>	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);
>	}

changes nothing to kernel oops. It only skip show_pte() for user-mode
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?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-07 13:14         ` Xie Yuanbin
@ 2026-07-07 13:20           ` Lorenzo Stoakes
  2026-07-07 14:04             ` Xie Yuanbin
  2026-07-07 15:34             ` Russell King
  0 siblings, 2 replies; 21+ messages in thread
From: Lorenzo Stoakes @ 2026-07-07 13:20 UTC (permalink / raw)
  To: Xie Yuanbin
  Cc: linux, akpm, david, liam, liaohua4, lilinjie8, linusw,
	linux-arm-kernel, linux-kernel, linux-mm, mhocko, rppt,
	sunnanyong, surenb, vbabka, xiqi2

On Tue, Jul 07, 2026 at 09:14:09PM +0800, Xie Yuanbin wrote:
> On Tue, 7 Jul 2026 12:57:45 +0100, Russell King wrote:
> > 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:
> >
> >	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);
> >	}
>
> changes nothing to kernel oops. It only skip show_pte() for user-mode
> 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?

Well the whole reason you're faulting here might be because a userland process
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?

Thanks, Lorenzo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-07 12:46     ` Lorenzo Stoakes
@ 2026-07-07 13:35       ` Xie Yuanbin
  0 siblings, 0 replies; 21+ messages in thread
From: Xie Yuanbin @ 2026-07-07 13:35 UTC (permalink / raw)
  To: ljs
  Cc: akpm, david, liam, liaohua4, lilinjie8, linusw, linux-arm-kernel,
	linux-kernel, linux-mm, linux, mhocko, rppt, sunnanyong, surenb,
	vbabka, xieyuanbin1, xiqi2

On Tue, 7 Jul 2026 13:46:19 +0100, Lorenzo Stoakes wrote:
> On Mon, Jul 06, 2026 at 09:32:47PM +0800, Xie Yuanbin wrote:
>> 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.

Thank you very much for your reply. Now I fully understand: to traverse
the page tables of non-VMA addr in user address space, the mmap write
lock is required.

But I still want like to ask a question:
> 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
>          :)

If (just hypothetically), the ARM32 architecture supports huge pages
and memory hotplug, what kind of lock do I need to safely traverse the
page tables of non-VMA addr in kernel space?

Thanks again.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-07 13:20           ` Lorenzo Stoakes
@ 2026-07-07 14:04             ` Xie Yuanbin
  2026-07-07 15:34             ` Russell King
  1 sibling, 0 replies; 21+ messages in thread
From: Xie Yuanbin @ 2026-07-07 14:04 UTC (permalink / raw)
  To: ljs, akpm
  Cc: david, liam, liaohua4, lilinjie8, linusw, linux-arm-kernel,
	linux-kernel, linux-mm, linux, mhocko, rppt, sunnanyong, surenb,
	vbabka, xieyuanbin1, xiqi2

On Tue, 7 Jul 2026 14:20:19 +0100, Lorenzo Stoakes wrote:
> Well the whole reason you're faulting here might be because a userland process
> 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?

I think it is not the same thing as the previous discussion regarding
which locks are needed for `show_pte()`.

Now, for this bug, we have two places that need to be fixed:
1. __do_user_fault()->show_pte();
2. do_DataAbort()->show_pte();

For the first one, it must be a user fault, the judgment of
user_mode(regs) can be omitted.

For the second one, it may be a user fault or a kernel fault. If it is
a kernel fault, it also means that this is a kernel oops.

For kernel oops, according to Russell's opinion, we do not need to fix
this bug, which means we do not need to acquire any locks, because the
kernel may die soon. However, show_pte() still needs to be retained
because it is useful for debugging the kernel.

For user faults, we need to fix it:
1. For user space addr (addr < TASK_SIZE), we need to acquire
   current->mm's write lock, before show_pte(), I understand it now.
2. For kernel space addr (addr >= TASK_SIZE), maybe we can omit
   show_pte()? After all, user-mode programs can access any kernel address
   to trigger user faults, but we dump the page tables of kernel
   addresses, would that be inappropriate,
   (e.g., posing potential security risks)?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-07 13:20           ` Lorenzo Stoakes
  2026-07-07 14:04             ` Xie Yuanbin
@ 2026-07-07 15:34             ` Russell King
  2026-07-10  2:32               ` Qi Xi
  1 sibling, 1 reply; 21+ messages in thread
From: Russell King @ 2026-07-07 15:34 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Xie Yuanbin, akpm, david, liam, liaohua4, lilinjie8, linusw,
	linux-arm-kernel, linux-kernel, linux-mm, mhocko, rppt,
	sunnanyong, surenb, vbabka, xiqi2

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:
> > On Tue, 7 Jul 2026 12:57:45 +0100, Russell King wrote:
> > > 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:
> > >
> > >	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);
> > >	}
> >
> > changes nothing to kernel oops. It only skip show_pte() for user-mode
> > 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?
> 
> Well the whole reason you're faulting here might be because a userland process
> 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?

Unconditionally taking the lock could lead to a deadlock. Consider
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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER
  2026-07-07 15:34             ` Russell King
@ 2026-07-10  2:32               ` Qi Xi
  0 siblings, 0 replies; 21+ messages in thread
From: Qi Xi @ 2026-07-10  2:32 UTC (permalink / raw)
  To: Russell King, Lorenzo Stoakes
  Cc: Xie Yuanbin, akpm, david, liam, liaohua4, lilinjie8, linusw,
	linux-arm-kernel, linux-kernel, linux-mm, mhocko, rppt,
	sunnanyong, surenb, vbabka, xiqi2, Kefeng Wang


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:
>>> On Tue, 7 Jul 2026 12:57:45 +0100, Russell King wrote:
>>>> 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:
>>>>
>>>> 	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);
>>>> 	}
>>> changes nothing to kernel oops. It only skip show_pte() for user-mode
>>> 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?
>> Well the whole reason you're faulting here might be because a userland process
>> 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?
> Unconditionally taking the lock could lead to a deadlock. Consider
> 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.


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-07-10  2:32 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-26  7:30 [PATCH v3 0/2] ARM: mm: fix use-after-free in show_pte() Qi Xi
2026-06-26  7:30 ` [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER Qi Xi
2026-06-26  9:44   ` Russell King
2026-06-27  1:39     ` Qi Xi
2026-07-06 13:32   ` Xie Yuanbin
2026-07-07 11:48     ` Qi Xi
2026-07-07 11:57       ` Russell King
2026-07-07 12:47         ` Qi Xi
2026-07-07 12:47         ` Lorenzo Stoakes
2026-07-07 13:14         ` Xie Yuanbin
2026-07-07 13:20           ` Lorenzo Stoakes
2026-07-07 14:04             ` Xie Yuanbin
2026-07-07 15:34             ` Russell King
2026-07-10  2:32               ` Qi Xi
2026-07-07 12:46     ` Lorenzo Stoakes
2026-07-07 13:35       ` Xie Yuanbin
2026-06-26  7:30 ` [PATCH v3 2/2] ARM: mm: protect show_pte() in do_DataAbort() fallback path Qi Xi
2026-06-26  9:45   ` Russell King
2026-06-26 10:16   ` Xie Yuanbin
2026-06-26 12:37     ` Russell King
2026-06-27  1:22       ` Xie Yuanbin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox