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

[Qi Xi <xiqi2@huawei.com>]

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