Re: [PATCH v2 7/7] ARM: mm: Fix PXN process with LPAE feature

Kefeng Wang
2021-06-02T15:13:21Z

On 2021/6/2 18:52, Russell King (Oracle) wrote:
> Hi,
>
> On Wed, Jun 02, 2021 at 03:02:46PM +0800, Kefeng Wang wrote:
>> When user code execution with privilege mode, it will lead to
>> 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"
>>
>> Lets' fix it by adding the check in do_page_fault() and panic
>> when ARM_LPAE enabled.
>>
>> 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 | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
>> index 7cfa9a59d3ec..279bbeb33b48 100644
>> --- a/arch/arm/mm/fault.c
>> +++ b/arch/arm/mm/fault.c
>> @@ -257,8 +257,14 @@ 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 && addr < TASK_SIZE && !user_mode(regs))
>> + die_kernel_fault("execution of user memory",
>> + addr, fsr, regs);
>> +#endif
>> + }
> Do we need to do this test here?
>
> Also, is this really LPAE specific? We have similar protection on 32-bit
> ARM using domains to disable access to userspace except when the user
> accessors are being used, so I would expect kernel-mode execution to
> also cause a fault there.
IFSR format when using the Short-descriptor translation table format

Domain fault      01001            First level   01011     Second level

Permission fault 01101            First level   01111     Second level

IFSR format when using the Long-descriptor translation table format

0011LL Permission fault. LL bits indicate levelb.

After check the ARM spec, I think for the permission fault,  we should panic

with or without LPAE, will change to

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 7cfa9a59d3ec..dd97d9b19dec 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -257,8 +257,11 @@ 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;
+               if (!user_mode(regs))
+                       die_kernel_fault("execution of memory", addr,
fsr, regs);
+       }

perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr);

If no object, I will send all patches with updates to  patch system,
thanks.

>