Re: [PATCH 1/2] arm64: mm: drop VM_FAULT_BADMAP/VM_FAULT_BADACCESS

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

On 2024/4/10 20:39, Cristian Marussi wrote:
> On Wed, Apr 10, 2024 at 07:53:21PM +0800, Kefeng Wang wrote:
>>
>>
>> On 2024/4/10 19:24, Aishwarya TCV wrote:
>>>
> 
> Hi,
> 
>>>
>>> On 07/04/2024 09:12, Kefeng Wang wrote:
>>>> If bad map or access, directly set si_code to SEGV_MAPRR or SEGV_ACCERR,
>>>> also set fault to 0 and goto error handling, which make us to drop the
>>>> arch's special vm fault reason.
>>>>
>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>>> ---
>>>>    arch/arm64/mm/fault.c | 41 ++++++++++++++++++-----------------------
>>>>    1 file changed, 18 insertions(+), 23 deletions(-)
>>>
>>> Hi Kefeng,
>>>
>>> While running LTP test suite, I observed that few test cases are unable
>>> to kill exe when run against next-master(next-20240409) kernel with
>>> Arm64 on JUNO in our CI. I can send the full logs if required, but it
>>> doesn't say much.
>>
>> Sorry about it, as Catalin pointed, there is issue in this patch, and
>> I replied today, could you help to test with following changes, many thanks.
>>
>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>> index 61a2acae0dca..451ba7cbd5ad 100644
>> --- a/arch/arm64/mm/fault.c
>> +++ b/arch/arm64/mm/fault.c
>> @@ -604,6 +604,7 @@ static int __kprobes do_page_fault(unsigned long far,
>> unsigned long esr,
>>          }
>>
>>          if (!(vma->vm_flags & vm_flags)) {
>> +               mmap_read_unlock(mm);
>>                  fault = 0;
>>                  si_code = SEGV_ACCERR;
>>                  goto bad_area;
>> @@ -632,6 +633,7 @@ static int __kprobes do_page_fault(unsigned long far,
>> unsigned long esr,
>>          if (likely(!(fault & VM_FAULT_ERROR)))
>>                  return 0;
>>
>> +       si_code = SEGV_MAPERR;
>>   bad_area:
>>          /*
>>           * If we are in kernel mode at this point, we have no context to
>>
> 
> On my JUNO setup, the above patch solves the 'un-killable tests' issues and
> all the previously failing tests passes. (I have NOT run the full LTP
> test-suite, though, only replayed the tests that were failing before as
> mentioned by Aishwarya)

Thanks, I will resend v2, but let's wait for Catalin to see if there is 
any other opinion.
> 
> Thanks,
> Cristian