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

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

[Xie Yuanbin]

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>

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

[Qi Xi]

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>

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

[Russell King]

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!

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

[Lorenzo Stoakes]

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

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

[Qi Xi]

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

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

[Lorenzo Stoakes]

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

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

[Xie Yuanbin]

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?

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

[Lorenzo Stoakes]

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

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

[Xie Yuanbin]

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.

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

[Xie Yuanbin]

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

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

[Russell King]

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!

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

[Qi Xi]

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.

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

[Lorenzo Stoakes]

On Tue, Jul 07, 2026 at 09:35:27PM +0800, Xie Yuanbin wrote:
> 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.

You're welcome! :)

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

hotplug lock, mmap write lock.

>
> Thanks again.

Cheers, Lorenzo

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

[Lorenzo Stoakes]

On Fri, Jul 10, 2026 at 10:32:29AM +0800, Qi Xi wrote:
>
> 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.

Yeah right, ugh.

We don't currently have an mmap_write_trylock(), that was removed in commit
cf95e337cb63 ("mm: delete mmap_write_trylock() and vma_try_start_write()")
but I think it's legit to bring _only_ the mmap_write_trylock() back (not
vma_try_start_write()), AND updated to account for how VMA locking works
currently (we have to update a seqcount on mmap write lock acquisition).

To make life easy - I've attached a patch that you can add as part of a
series (please keep attribution etc. to me so I can be blamed/villified if
this is insane :)

That way we avoid the deadlock and also avoid concurrent downgraded
munmap() oopsing in show_pte().

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

In general yeah but with mmap_write_trylock() after you add the attached
patch to a series (making sure to cc- the right people etc.)... but also,
wouldn't you just generally want this in show_pte()?

Seems to me best way is to put the existing show_pte() into a __show_pte()
and then do show_pte() like (untested top of my head thing):

void show_pte(const char *lvl, struct mm_struct *mm, unsigned long addr,
	      bool is_user)
{
	/* Write lock needed to account for concurrent downgraded munmap(). */
	if (is_user && !mmap_write_trylock(mm)) {
		printk("%s[%08lx] unable to acquire lock for PTE output\n",
			lvl, addr);
		return;
	}
	__show_pte(lvl, mm, addr);
	if (is_user)
		mmap_write_unlock(mm);
}

Cheers, Lorenzo

----8<----
From 2d0906c9ca962361fcdf2a033deb2a37252c486a Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <ljs@kernel.org>
Date: Sat, 11 Jul 2026 08:07:27 +0100
Subject: [PATCH] mm: add mmap_write_trylock()

Commit cf95e337cb63 ("mm: delete mmap_write_trylock() and
vma_try_start_write()") removed mmap_write_trylock() as there were no
users.

Re-add this helper and update to have it update the mm's write lock
sequence count number as per the current VMA lock implementation.

The use cases for this function are very narrow - in the vast majority of
cases an inability to acquire the write lock is terminal so you want either
an unconditional or killable variant of the mmap write lock.

However this is being added so a subsequent patch can safely dump PTE
information when an unhandled fault arises in arm32.

Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
---
 include/linux/mmap_lock.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
index 04b8f61ece5d..fe14d4babae9 100644
--- a/include/linux/mmap_lock.h
+++ b/include/linux/mmap_lock.h
@@ -546,6 +546,18 @@ static inline void mmap_write_lock_nested(struct mm_struct *mm, int subclass)
 	__mmap_lock_trace_acquire_returned(mm, true, true);
 }

+static inline bool __must_check mmap_write_trylock(struct mm_struct *mm)
+{
+	bool ret;
+
+	__mmap_lock_trace_start_locking(mm, true);
+	ret = down_write_trylock(&mm->mmap_lock) != 0;
+	if (ret)
+		mm_lock_seqcount_begin(mm);
+	__mmap_lock_trace_acquire_returned(mm, true, ret);
+	return ret;
+}
+
 static inline int __must_check mmap_write_lock_killable(struct mm_struct *mm)
 {
 	int ret;
--
2.55.0

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

[Xie Yuanbin]

On Sat, 11 Jul 2026 08:27:23 +0100, Lorenzo Stoakes wrote:
> In general yeah but with mmap_write_trylock() after you add the attached
> patch to a series (making sure to cc- the right people etc.)... but also,
> wouldn't you just generally want this in show_pte()?
>
> Seems to me best way is to put the existing show_pte() into a __show_pte()
> and then do show_pte() like (untested top of my head thing):
>
> void show_pte(const char *lvl, struct mm_struct *mm, unsigned long addr,
> 	      bool is_user)
> {
> 	/* Write lock needed to account for concurrent downgraded munmap(). */
> 	if (is_user && !mmap_write_trylock(mm)) {
> 		printk("%s[%08lx] unable to acquire lock for PTE output\n",
> 			lvl, addr);
> 		return;
> 	}
> 	__show_pte(lvl, mm, addr);
> 	if (is_user)
> 		mmap_write_unlock(mm);
> }

For user faults, I think we don't need to worry about deadlock issues.

As Russell King described, deadlock problems can only occur in kernel faults.
On Tue, 7 Jul 2026 16:34:21 +0100, Russell King wrote:
> 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.

As we never return to user mode with mmap lock held, so for user faults,
it is safe to acquire mmap lock. This is just the same with the syscall
entry of `pkey_alloc`, we immediately acquire the mmap write lock:
Link: https://elixir.bootlin.com/linux/v7.2-rc2/source/mm/mprotect.c#L1011

For the kernel, the user faults entry and the system call entry are not
fundamentally different; they are both kernel entry points.

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

[Lorenzo Stoakes]

On Sat, Jul 11, 2026 at 03:56:51PM +0800, Xie Yuanbin wrote:
> On Sat, 11 Jul 2026 08:27:23 +0100, Lorenzo Stoakes wrote:
> > In general yeah but with mmap_write_trylock() after you add the attached
> > patch to a series (making sure to cc- the right people etc.)... but also,
> > wouldn't you just generally want this in show_pte()?
> >
> > Seems to me best way is to put the existing show_pte() into a __show_pte()
> > and then do show_pte() like (untested top of my head thing):
> >
> > void show_pte(const char *lvl, struct mm_struct *mm, unsigned long addr,
> > 	      bool is_user)
> > {
> > 	/* Write lock needed to account for concurrent downgraded munmap(). */
> > 	if (is_user && !mmap_write_trylock(mm)) {
> > 		printk("%s[%08lx] unable to acquire lock for PTE output\n",
> > 			lvl, addr);
> > 		return;
> > 	}
> > 	__show_pte(lvl, mm, addr);
> > 	if (is_user)
> > 		mmap_write_unlock(mm);
> > }
>
> For user faults, I think we don't need to worry about deadlock issues.
>
> As Russell King described, deadlock problems can only occur in kernel faults.
> On Tue, 7 Jul 2026 16:34:21 +0100, Russell King wrote:
> > 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.
>
> As we never return to user mode with mmap lock held, so for user faults,
> it is safe to acquire mmap lock. This is just the same with the syscall
> entry of `pkey_alloc`, we immediately acquire the mmap write lock:
> Link: https://elixir.bootlin.com/linux/v7.2-rc2/source/mm/mprotect.c#L1011
>
> For the kernel, the user faults entry and the system call entry are not
> fundamentally different; they are both kernel entry points.

OK given arm32 can't do any of the stuff that makes kernel faults an issue, and
if it's certain you can't deadlock on mmap write lock for VMA or non-VMA ranges
on user fault, then no need for the mmap write trylock, and just take the mmap
write lock unconditionally for user faults, and no lock for kernel faults I
guess?

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

[Xie Yuanbin]

On Sat, 11 Jul 2026 09:05:03 +0100, Lorenzo Stoakes wrote:
> OK given arm32 can't do any of the stuff that makes kernel faults an issue, and
> if it's certain you can't deadlock on mmap write lock for VMA or non-VMA ranges
> on user fault, then no need for the mmap write trylock, and just take the mmap
> write lock unconditionally for user faults, and no lock for kernel faults I
> guess?

Yes, that's exactly what I think. We are waiting for ARM's maintainer Russell King
to confirm, and then we will send out the new patch. :)