Re: [PATCH v10 RESEND 5/9] riscv: mm: avoid enabling interrupts for nofault page faults

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] riscv: mm: fixup_exception bypasses vmalloc_fault for nofault accesses
--

commit b959120cb7ee095dc7b5e388ec2110c0a78dd4ac
Author: Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com>

riscv: mm: avoid enabling interrupts for nofault page faults

This commit modifies the RISC-V page fault handler to resolve nofault accesses
via the exception table before enabling interrupts or entering the generic
fault path. It aims to prevent in-atomic nofault accesses from inadvertently
running tracepoints or opening an interrupt window.

> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -294,6 +294,13 @@ void handle_page_fault(struct pt_regs *regs)
>  	if (kprobe_page_fault(regs, cause))
>  		return;
>  
> +	/*
> +	 * Nofault accesses must be resolved through the exception table before
> +	 * entering the generic fault path or enabling interrupts.
> +	 */
> +	if (unlikely(faulthandler_disabled()) && fixup_exception(regs))
> +		return;

[Severity: High]
Will this new check in handle_page_fault intercept faults on valid vmalloc
addresses before vmalloc_fault has a chance to execute?

When tools like BPF or perf perform a legitimate nofault read (such as via
copy_from_kernel_nofault) on a vmalloc address that has not yet been
synchronized to the current task PGD, it triggers a page fault. Since
faulthandler_disabled is true in a nofault context, it appears this new
early check will call fixup_exception and immediately return.

On 32-bit RISC-V and !MMU configurations, this bypasses the lazy PGD
synchronization performed a few lines further down:

if ((!IS_ENABLED(CONFIG_MMU) || !IS_ENABLED(CONFIG_64BIT)) &&
    unlikely(addr >= VMALLOC_START && addr < VMALLOC_END)) {
        vmalloc_fault(regs, code, addr);
        return;
}

Could this cause valid kernel vmalloc reads via nofault accessors to
spuriously fail with -EFAULT?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789974241.git.zhangzhanpeng.jasper@bytedance.com?part=5