* [PATCH 0/2] ARM: fix PXN process with LPAE feature
@ 2021-05-29 3:41 Kefeng Wang
2021-05-29 3:41 ` [PATCH 1/2] ARM: mm: Refactor __do_page_fault() Kefeng Wang
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Kefeng Wang @ 2021-05-29 3:41 UTC (permalink / raw)
To: Russell King, linux-arm-kernel
Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee,
Kefeng Wang
refactor and cleanup [__do|do]_page_fault, then fix the pxn process
when lpae feature enabled.
Kefeng Wang (2):
ARM: mm: Refactor __do_page_fault()
ARM: mm: Fix PXN process when ARM_LPAE enabled
arch/arm/mm/fault.c | 78 +++++++++++++++++----------------------------
1 file changed, 30 insertions(+), 48 deletions(-)
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] ARM: mm: Refactor __do_page_fault() 2021-05-29 3:41 [PATCH 0/2] ARM: fix PXN process with LPAE feature Kefeng Wang @ 2021-05-29 3:41 ` Kefeng Wang 2021-06-01 14:31 ` Russell King (Oracle) 2021-05-29 3:41 ` [PATCH 2/2] ARM: mm: Fix PXN process when ARM_LPAE enabled Kefeng Wang 2021-06-01 13:29 ` [PATCH 0/2] ARM: fix PXN process with LPAE feature Kefeng Wang 2 siblings, 1 reply; 9+ messages in thread From: Kefeng Wang @ 2021-05-29 3:41 UTC (permalink / raw) To: Russell King, linux-arm-kernel Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee, Kefeng Wang 1. cleanup access_error(), make vma flags set and check into __do_page_fault() and do_page_fault() directly. 2. drop fsr and task argument, instead, using vm_flags in __do_page_fault(). 3. cleans up the multiple goto statements in __do_page_fault(). 4. use current->mm directly in do_page_fault(). Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- arch/arm/mm/fault.c | 73 ++++++++++++++++----------------------------- 1 file changed, 25 insertions(+), 48 deletions(-) diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c index efa402025031..81cf3e6e2a3d 100644 --- a/arch/arm/mm/fault.c +++ b/arch/arm/mm/fault.c @@ -183,74 +183,45 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs) #define VM_FAULT_BADMAP 0x010000 #define VM_FAULT_BADACCESS 0x020000 -/* - * Check that the permissions on the VMA allow for the fault which occurred. - * If we encountered a write fault, we must have write permission, otherwise - * we allow any permission. - */ -static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma) -{ - unsigned int mask = VM_ACCESS_FLAGS; - - if ((fsr & FSR_WRITE) && !(fsr & FSR_CM)) - mask = VM_WRITE; - if (fsr & FSR_LNX_PF) - mask = VM_EXEC; - - return vma->vm_flags & mask ? false : true; -} - static vm_fault_t __kprobes -__do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr, - unsigned int flags, struct task_struct *tsk, - struct pt_regs *regs) +__do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int flags, + unsigned long vma_flags, struct pt_regs *regs) { - struct vm_area_struct *vma; - vm_fault_t fault; - - vma = find_vma(mm, addr); - fault = VM_FAULT_BADMAP; + struct vm_area_struct *vma = find_vma(mm, addr); if (unlikely(!vma)) - goto out; - if (unlikely(vma->vm_start > addr)) - goto check_stack; + return VM_FAULT_BADMAP; + + if (unlikely(vma->vm_start > addr)) { + if (!(vma->vm_flags & VM_GROWSDOWN)) + return VM_FAULT_BADMAP; + if (addr < FIRST_USER_ADDRESS) + return VM_FAULT_BADMAP; + if (expand_stack(vma, addr)) + return VM_FAULT_BADMAP; + } /* * Ok, we have a good vm_area for this * memory access, so we can handle it. */ -good_area: - if (access_error(fsr, vma)) { - fault = VM_FAULT_BADACCESS; - goto out; - } + if (!(vma->vm_flags & vma_flags)) + return VM_FAULT_BADACCESS; return handle_mm_fault(vma, addr & PAGE_MASK, flags, regs); - -check_stack: - /* Don't allow expansion below FIRST_USER_ADDRESS */ - if (vma->vm_flags & VM_GROWSDOWN && - addr >= FIRST_USER_ADDRESS && !expand_stack(vma, addr)) - goto good_area; -out: - return fault; } static int __kprobes do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs) { - struct task_struct *tsk; - struct mm_struct *mm; + struct mm_struct *mm = current->mm; int sig, code; vm_fault_t fault; unsigned int flags = FAULT_FLAG_DEFAULT; + unsigned long vm_flags = VM_ACCESS_FLAGS; if (kprobe_page_fault(regs, fsr)) return 0; - tsk = current; - mm = tsk->mm; - /* Enable interrupts if they were enabled in the parent context. */ if (interrupts_enabled(regs)) local_irq_enable(); @@ -264,8 +235,14 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs) if (user_mode(regs)) flags |= FAULT_FLAG_USER; - if ((fsr & FSR_WRITE) && !(fsr & FSR_CM)) + + if ((fsr & FSR_WRITE) && !(fsr & FSR_CM)) { flags |= FAULT_FLAG_WRITE; + vm_flags = VM_WRITE; + } + + if (fsr & FSR_LNX_PF) + vm_flags = VM_EXEC; perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr); @@ -293,7 +270,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs) #endif } - fault = __do_page_fault(mm, addr, fsr, flags, tsk, regs); + fault = __do_page_fault(mm, addr, flags, vm_flags, regs); /* If we need to retry but a fatal signal is pending, handle the * signal first. We do not need to release the mmap_lock because -- 2.26.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: mm: Refactor __do_page_fault() 2021-05-29 3:41 ` [PATCH 1/2] ARM: mm: Refactor __do_page_fault() Kefeng Wang @ 2021-06-01 14:31 ` Russell King (Oracle) 2021-06-02 6:42 ` Kefeng Wang 2023-06-02 9:49 ` Ard Biesheuvel 0 siblings, 2 replies; 9+ messages in thread From: Russell King (Oracle) @ 2021-06-01 14:31 UTC (permalink / raw) To: Kefeng Wang Cc: linux-arm-kernel, Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee On Sat, May 29, 2021 at 11:41:37AM +0800, Kefeng Wang wrote: > 1. cleanup access_error(), make vma flags set and check into > __do_page_fault() and do_page_fault() directly. > > 2. drop fsr and task argument, instead, using vm_flags in > __do_page_fault(). > > 3. cleans up the multiple goto statements in __do_page_fault(). > > 4. use current->mm directly in do_page_fault(). > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> This patch is a really good example of something that is very difficult to review and see that there are no unintended changes. Many people have complained about my patches, where I create a series of many patches where each patch does exactly _one_ simple transformation to the code. This is a good example _why_ I do that - a step by step single transformation approach is way easier to review. Sorry, but I'm not able to sensibly review this patch, and therefore I won't apply it. Please split it into smaller changes. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: mm: Refactor __do_page_fault() 2021-06-01 14:31 ` Russell King (Oracle) @ 2021-06-02 6:42 ` Kefeng Wang 2023-06-02 9:49 ` Ard Biesheuvel 1 sibling, 0 replies; 9+ messages in thread From: Kefeng Wang @ 2021-06-02 6:42 UTC (permalink / raw) To: Russell King (Oracle) Cc: linux-arm-kernel, Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee On 2021/6/1 22:31, Russell King (Oracle) wrote: > On Sat, May 29, 2021 at 11:41:37AM +0800, Kefeng Wang wrote: >> 1. cleanup access_error(), make vma flags set and check into >> __do_page_fault() and do_page_fault() directly. >> >> 2. drop fsr and task argument, instead, using vm_flags in >> __do_page_fault(). >> >> 3. cleans up the multiple goto statements in __do_page_fault(). >> >> 4. use current->mm directly in do_page_fault(). >> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > This patch is a really good example of something that is very difficult > to review and see that there are no unintended changes. > > Many people have complained about my patches, where I create a series of > many patches where each patch does exactly _one_ simple transformation to > the code. This is a good example _why_ I do that - a step by step single > transformation approach is way easier to review. > > Sorry, but I'm not able to sensibly review this patch, and therefore > I won't apply it. Please split it into smaller changes. Ok, will split it and send v2, thanks. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: mm: Refactor __do_page_fault() 2021-06-01 14:31 ` Russell King (Oracle) 2021-06-02 6:42 ` Kefeng Wang @ 2023-06-02 9:49 ` Ard Biesheuvel 2023-06-02 9:51 ` Ard Biesheuvel 1 sibling, 1 reply; 9+ messages in thread From: Ard Biesheuvel @ 2023-06-02 9:49 UTC (permalink / raw) To: Russell King (Oracle) Cc: Kefeng Wang, linux-arm-kernel, Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee On Tue, 1 Jun 2021 at 16:32, Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Sat, May 29, 2021 at 11:41:37AM +0800, Kefeng Wang wrote: > > 1. cleanup access_error(), make vma flags set and check into > > __do_page_fault() and do_page_fault() directly. > > > > 2. drop fsr and task argument, instead, using vm_flags in > > __do_page_fault(). > > > > 3. cleans up the multiple goto statements in __do_page_fault(). > > > > 4. use current->mm directly in do_page_fault(). > > > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > > This patch is a really good example of something that is very difficult > to review and see that there are no unintended changes. > > Many people have complained about my patches, where I create a series of > many patches where each patch does exactly _one_ simple transformation to > the code. This is a good example _why_ I do that - a step by step single > transformation approach is way easier to review. > > Sorry, but I'm not able to sensibly review this patch, and therefore > I won't apply it. Please split it into smaller changes. > Agreed. If your commit message contains an enumeration of things the patch does, it is a very strong hint that each of those things needs to be a separate patch if at all possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: mm: Refactor __do_page_fault() 2023-06-02 9:49 ` Ard Biesheuvel @ 2023-06-02 9:51 ` Ard Biesheuvel 2023-06-02 9:59 ` Kefeng Wang 0 siblings, 1 reply; 9+ messages in thread From: Ard Biesheuvel @ 2023-06-02 9:51 UTC (permalink / raw) To: Russell King (Oracle) Cc: Kefeng Wang, linux-arm-kernel, Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee On Fri, 2 Jun 2023 at 11:49, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 1 Jun 2021 at 16:32, Russell King (Oracle) > <linux@armlinux.org.uk> wrote: > > > > On Sat, May 29, 2021 at 11:41:37AM +0800, Kefeng Wang wrote: > > > 1. cleanup access_error(), make vma flags set and check into > > > __do_page_fault() and do_page_fault() directly. > > > > > > 2. drop fsr and task argument, instead, using vm_flags in > > > __do_page_fault(). > > > > > > 3. cleans up the multiple goto statements in __do_page_fault(). > > > > > > 4. use current->mm directly in do_page_fault(). > > > > > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > > > > This patch is a really good example of something that is very difficult > > to review and see that there are no unintended changes. > > > > Many people have complained about my patches, where I create a series of > > many patches where each patch does exactly _one_ simple transformation to > > the code. This is a good example _why_ I do that - a step by step single > > transformation approach is way easier to review. > > > > Sorry, but I'm not able to sensibly review this patch, and therefore > > I won't apply it. Please split it into smaller changes. > > > > Agreed. If your commit message contains an enumeration of things the > patch does, it is a very strong hint that each of those things needs > to be a separate patch if at all possible. Also, apologies for digging up this 2 year old thread :-) I did so unintentionally. (Somehow, it turned up as new/unread in my LAKML folder) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: mm: Refactor __do_page_fault() 2023-06-02 9:51 ` Ard Biesheuvel @ 2023-06-02 9:59 ` Kefeng Wang 0 siblings, 0 replies; 9+ messages in thread From: Kefeng Wang @ 2023-06-02 9:59 UTC (permalink / raw) To: Ard Biesheuvel, Russell King (Oracle) Cc: linux-arm-kernel, Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee On 2023/6/2 17:51, Ard Biesheuvel wrote: > On Fri, 2 Jun 2023 at 11:49, Ard Biesheuvel <ardb@kernel.org> wrote: >> >> On Tue, 1 Jun 2021 at 16:32, Russell King (Oracle) >> <linux@armlinux.org.uk> wrote: >>> >>> On Sat, May 29, 2021 at 11:41:37AM +0800, Kefeng Wang wrote: >>>> 1. cleanup access_error(), make vma flags set and check into >>>> __do_page_fault() and do_page_fault() directly. >>>> >>>> 2. drop fsr and task argument, instead, using vm_flags in >>>> __do_page_fault(). >>>> >>>> 3. cleans up the multiple goto statements in __do_page_fault(). >>>> >>>> 4. use current->mm directly in do_page_fault(). >>>> >>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> >>> >>> This patch is a really good example of something that is very difficult >>> to review and see that there are no unintended changes. >>> >>> Many people have complained about my patches, where I create a series of >>> many patches where each patch does exactly _one_ simple transformation to >>> the code. This is a good example _why_ I do that - a step by step single >>> transformation approach is way easier to review. >>> >>> Sorry, but I'm not able to sensibly review this patch, and therefore >>> I won't apply it. Please split it into smaller changes. >>> >> >> Agreed. If your commit message contains an enumeration of things the >> patch does, it is a very strong hint that each of those things needs >> to be a separate patch if at all possible. Yes, already split it and the new version is merged, > > Also, apologies for digging up this 2 year old thread :-) I did so > unintentionally. Never mind, thank for all kind of reviews :) > > (Somehow, it turned up as new/unread in my LAKML folder) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] ARM: mm: Fix PXN process when ARM_LPAE enabled 2021-05-29 3:41 [PATCH 0/2] ARM: fix PXN process with LPAE feature Kefeng Wang 2021-05-29 3:41 ` [PATCH 1/2] ARM: mm: Refactor __do_page_fault() Kefeng Wang @ 2021-05-29 3:41 ` Kefeng Wang 2021-06-01 13:29 ` [PATCH 0/2] ARM: fix PXN process with LPAE feature Kefeng Wang 2 siblings, 0 replies; 9+ messages in thread From: Kefeng Wang @ 2021-05-29 3:41 UTC (permalink / raw) To: Russell King, linux-arm-kernel Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee, Kefeng Wang If we support PXN feature, should panic when user code execution with privilege mode, but it is not true when LAPE enabled, let's fix it by adding the check in do_page_fault(). Fixes: 1d4d37159d01 ("ARM: 8235/1: Support for the PXN CPU feature on ARMv7") Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- arch/arm/mm/fault.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c index 81cf3e6e2a3d..a6ada914cdf5 100644 --- a/arch/arm/mm/fault.c +++ b/arch/arm/mm/fault.c @@ -241,8 +241,13 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs) vm_flags = VM_WRITE; } - if (fsr & FSR_LNX_PF) + if (fsr & FSR_LNX_PF) { vm_flags = VM_EXEC; +#ifdef CONFIG_ARM_LPAE + if (addr < TASK_SIZE && !user_mode(regs)) + __do_kernel_fault(mm, addr, fsr, regs); +#endif + } perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr); -- 2.26.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] ARM: fix PXN process with LPAE feature 2021-05-29 3:41 [PATCH 0/2] ARM: fix PXN process with LPAE feature Kefeng Wang 2021-05-29 3:41 ` [PATCH 1/2] ARM: mm: Refactor __do_page_fault() Kefeng Wang 2021-05-29 3:41 ` [PATCH 2/2] ARM: mm: Fix PXN process when ARM_LPAE enabled Kefeng Wang @ 2021-06-01 13:29 ` Kefeng Wang 2 siblings, 0 replies; 9+ messages in thread From: Kefeng Wang @ 2021-06-01 13:29 UTC (permalink / raw) To: Russell King, linux-arm-kernel Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee On 2021/5/29 11:41, Kefeng Wang wrote: > refactor and cleanup [__do|do]_page_fault, then fix the pxn process > when lpae feature enabled. Additional information: when user code execution with privilege mode, it will lead infinite loop in the page fault handler if ARM_LPAE enabled, the issue could be reproduced with "echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT" also I could add a die_kernel_fault() (like arm64 does) to report different message in different scenes. Any comments, thanks. > > Kefeng Wang (2): > ARM: mm: Refactor __do_page_fault() > ARM: mm: Fix PXN process when ARM_LPAE enabled > > arch/arm/mm/fault.c | 78 +++++++++++++++++---------------------------- > 1 file changed, 30 insertions(+), 48 deletions(-) > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-06-02 10:00 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-05-29 3:41 [PATCH 0/2] ARM: fix PXN process with LPAE feature Kefeng Wang 2021-05-29 3:41 ` [PATCH 1/2] ARM: mm: Refactor __do_page_fault() Kefeng Wang 2021-06-01 14:31 ` Russell King (Oracle) 2021-06-02 6:42 ` Kefeng Wang 2023-06-02 9:49 ` Ard Biesheuvel 2023-06-02 9:51 ` Ard Biesheuvel 2023-06-02 9:59 ` Kefeng Wang 2021-05-29 3:41 ` [PATCH 2/2] ARM: mm: Fix PXN process when ARM_LPAE enabled Kefeng Wang 2021-06-01 13:29 ` [PATCH 0/2] ARM: fix PXN process with LPAE feature Kefeng Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).