Re: [PATCH RFC] riscv/mm: Add handling for VM_FAULT_SIGSEGV in mm_fault_error()

[Zhe Qiao]

Hi everyone，

On 2024/7/30 20:43, Alexandre Ghiti wrote:
> Hi Zhe,
>
> On 30/07/2024 09:38, Zhe Qiao wrote:
>> Hello everyone, recently while learning about the Riscv architecture's
>> handling of page fault exceptions in the Linux kernel, I found that
>> there is no handling of VM_CAULT_SIGSEGV in mm_fault_erroneous (), but
>> rather a BUG() is executed directly. Therefore, I simultaneously analyzed
>> the processing procedures of arm64, powerpc, and sh architectures for
>> this area and found that all three architectures have processing for
>> VM_CAULT_SIGSEGV. Therefore, I added relevant processing methods for
>> VM_CAULT_SIGSEGV on the riscv architecture.
>>
>> As a beginner, I am not sure if this processing method is correct and
>> would like to hear the opinions of my seniors.
>
>
> FWIW, we correctly handle "normal" segfaults (ie no VMA or permission access faults).
>
> What we don't handle is handle_mm_fault() returning VM_FAULT_SIGSEGV, which can happen in different situations. For example, the BPF arena stuff implements a vma fault handler which can return VM_FAULT_SIGSEGV (see https://elixir.bootlin.com/linux/v6.10/source/kernel/bpf/arena.c#L273). In that case, we would BUG() instead of correctly killing the process.
>
> So I agree that we need this patch. Can you just rephrase the commit log? I would say something similar (without the link) to what I have just said above. And can you also add the following Fixes tag?
>
> Fixes: 07037db5d479 ("RISC-V: Paging and MMU")
>
> Thanks,
>
> Alex 


 Thinks for your reply. I have submitted a new patch. Connection address: https://lists.infradead.org/pipermail/linux-riscv/2024-July/058216.html


Thanks，


Zhe


>
>
>>
>> Signed-off-by: Zhe Qiao <qiaozhe@xxxxxxxxxxx>
>> ---
>>   arch/riscv/mm/fault.c | 17 +++++++++--------
>>   1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
>> index 5224f3733802..868163b904e2 100644
>> --- a/arch/riscv/mm/fault.c
>> +++ b/arch/riscv/mm/fault.c
>> @@ -61,26 +61,27 @@ static inline void no_context(struct pt_regs *regs, unsigned long addr)
>>     static inline void mm_fault_error(struct pt_regs *regs, unsigned long addr, vm_fault_t fault)
>>   {
>> +    if (!user_mode(regs)) {
>> +        no_context(regs, addr);
>> +        return;
>> +    }
>> +
>>       if (fault & VM_FAULT_OOM) {
>>           /*
>>            * We ran out of memory, call the OOM killer, and return the userspace
>>            * (which will retry the fault, or kill us if we got oom-killed).
>>            */
>> -        if (!user_mode(regs)) {
>> -            no_context(regs, addr);
>> -            return;
>> -        }
>>           pagefault_out_of_memory();
>>           return;
>>       } else if (fault & (VM_FAULT_SIGBUS | VM_FAULT_HWPOISON | VM_FAULT_HWPOISON_LARGE)) {
>>           /* Kernel mode? Handle exceptions or die */
>> -        if (!user_mode(regs)) {
>> -            no_context(regs, addr);
>> -            return;
>> -        }
>>           do_trap(regs, SIGBUS, BUS_ADRERR, addr);
>>           return;
>> +    } else if (fault & VM_FAULT_SIGSEGV) {
>> +        do_trap(regs, SIGSEGV, SEGV_MAPERR, addr);
>> +        return;
>>       }
>> +
>>       BUG();
>>   }
>>