* Re: [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER [not found] <20260626073048.3595106-2-xiqi2@huawei.com> @ 2026-07-06 13:32 ` Xie Yuanbin 2026-07-07 11:48 ` Qi Xi 2026-07-07 12:46 ` Lorenzo Stoakes 0 siblings, 2 replies; 12+ 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(¤t->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] 12+ 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 ` [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER 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; 12+ 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(¤t->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] 12+ 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; 12+ 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(¤t->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] 12+ 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; 12+ 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(¤t->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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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 ` [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER 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; 12+ 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(¤t->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] 12+ 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; 12+ 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(¤t->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] 12+ messages in thread
end of thread, other threads:[~2026-07-10 2:32 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260626073048.3595106-2-xiqi2@huawei.com>
2026-07-06 13:32 ` [PATCH v3 1/2] ARM: mm: fix use-after-free in __do_user_fault() under CONFIG_DEBUG_USER 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox