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

[Kefeng Wang <wangkefeng.wang@huawei.com>]

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.

>