[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2f83c6a3-8db0-46f7-928a-8bf585e4a3b9@ghiti.fr>
Date: Tue, 30 Jul 2024 14:43:02 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: Zhe Qiao <qiaozhe@...as.ac.cn>, paul.walmsley@...ive.com,
palmer@...belt.com, aou@...s.berkeley.edu, surenb@...gle.com,
alexghiti@...osinc.com, akpm@...ux-foundation.org,
wangkefeng.wang@...wei.com, willy@...radead.org
Cc: linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] riscv/mm: Add handling for VM_FAULT_SIGSEGV in
mm_fault_error()
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
>
> Signed-off-by: Zhe Qiao <qiaozhe@...as.ac.cn>
> ---
> 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();
> }
>
Powered by blists - more mailing lists