Re: [PATCH 3/5] LoongArch: BPF: Add exceptions (bpf_throw) support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] `arch_bpf_stack_walk()` uses inline assembly to capture the live `$r22` register, incorrectly assuming it holds the unmodified BPF frame pointer.

Pre-existing issues:
- [High] Tail calls within exception programs (or arena programs) cause a kernel panic (NULL pointer dereference) due to a hardcoded stack offset macro.
--

commit 7327176c407071584a340964ddba1f924e66533a
Author: George Guo <guodongtai@kylinos.cn>

LoongArch: BPF: Add exceptions (bpf_throw) support

Implement BPF exception support, advertised via
bpf_jit_supports_exceptions(). bpf_throw() unwinds the stack to find the
exception boundary program's frame and then invokes its exception
callback with that frame's stack and frame pointers.

> diff --git a/arch/loongarch/kernel/stacktrace.c b/arch/loongarch/kernel/stacktrace.c
> index 387dc4d3c4868..718c98b3f1fc4 100644
> --- a/arch/loongarch/kernel/stacktrace.c
> +++ b/arch/loongarch/kernel/stacktrace.c
[ ... ]
> @@ -40,6 +41,57 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
>  	}
>  }
>  
> +#ifdef CONFIG_UNWINDER_ORC
[ ... ]
> +void arch_bpf_stack_walk(bool (*consume_fn)(void *cookie, u64 ip, u64 sp, u64 bp),
> +			 void *cookie)
> +{
> +	unsigned long fp;
> +
> +	/*
> +	 * Capture the live frame pointer ($r22/$fp) here, before handing off to
> +	 * the worker. The kernel is built with -fomit-frame-pointer, so $fp is
> +	 * an ordinary callee-saved register that is preserved across the call
> +	 * from the JITed BPF program into bpf_throw() down to here, and thus
> +	 * still points at the innermost BPF frame. The ORC frame-pointer
> +	 * fallback walks the BPF frames up to the exception boundary from it.
> +	 *
> +	 * This must be a thin wrapper with no large stack locals: the worker
> +	 * uses $r22 to address its frame, which would clobber the live $fp
> +	 * before it could be read. __builtin_frame_address() cannot be used
> +	 * either, as it is $sp-derived and would yield a kernel-stack frame.
> +	 */
> +	asm volatile("move %0, $r22" : "=r"(fp));

[Severity: Critical]
Could the compiler clobber the live $r22 register before we capture it
here?

Since the kernel is built with -fomit-frame-pointer, bpf_throw() is
compiled as a standard C function and the compiler might use $r22 as a
general-purpose callee-saved register for local variables (such as the
cookie argument).

If $r22 is clobbered during bpf_throw(), this inline assembly will capture
the clobbered value instead of the unmodified BPF frame pointer. The
exception callback then blindly trusts this propagated value (passed via
$a2) to anchor its stack frame:

arch/loongarch/net/bpf_jit.c:build_prologue() {
    ...
    move_reg(ctx, LOONGARCH_GPR_FP, LOONGARCH_GPR_A2);
    emit_insn(ctx, addid, LOONGARCH_GPR_SP, LOONGARCH_GPR_FP, -stack_adjust);
    ...
}

Could this allow a forged chain of frame pointers to dictate the callback's
return address, leading to a severe regression where arbitrary kernel code
execution is possible?

> +	walk_stackframe_bpf(consume_fn, cookie, fp);
> +}
> +#endif /* CONFIG_UNWINDER_ORC */
> +
>  int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry,
>  			     void *cookie, struct task_struct *task)
>  {

> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index c410b02e64bef..22527428f0b34 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
[ ... ]
> @@ -203,7 +223,7 @@ static void build_prologue(struct jit_ctx *ctx)
>  	store_offset -= sizeof(long);
>  	emit_insn(ctx, std, LOONGARCH_GPR_S5, LOONGARCH_GPR_SP, store_offset);
>  
> -	if (ctx->arena_vm_start) {
> +	if (ctx->arena_vm_start || is_exception_prog) {
>  		store_offset -= sizeof(long);
>  		emit_insn(ctx, std, REG_ARENA, LOONGARCH_GPR_SP, store_offset);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does forcing REG_ARENA to be
pushed for exception programs expose an issue with the tail call layout?

When is_exception_prog is true, this adds an extra 8 bytes to the stack.
However, emit_bpf_tail_call() relies on a hardcoded stack offset macro:

arch/loongarch/net/bpf_jit.c:emit_bpf_tail_call() {
    ...
    int tcc_ptr_off = BPF_TAIL_CALL_CNT_PTR_STACK_OFF(ctx->stack_size);
    ...
    emit_insn(ctx, ldd, t3, REG_TCC, 0);
    ...
}

Since BPF_TAIL_CALL_CNT_PTR_STACK_OFF(ctx->stack_size) evaluates to a fixed
offset (stack_size - 80) regardless of whether REG_ARENA was pushed, does
this cause emit_bpf_tail_call() to incorrectly fetch the TCC integer
instead of TCC_PTR?

If it fetches the integer (often 0) and immediately dereferences it, would
tail calls within exception programs or arena programs cause a NULL pointer
dereference?

> @@ -212,6 +232,7 @@ static void build_prologue(struct jit_ctx *ctx)
>  
>  	emit_insn(ctx, addid, LOONGARCH_GPR_FP, LOONGARCH_GPR_SP, stack_adjust);
>  
> +setup_bpf_fp:
>  	if (ctx->priv_sp_used) {
>  		/* Set up the private stack pointer and the BPF frame pointer */
>  		void __percpu *priv_stack_ptr;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618033809.98253-1-dongtai.guo@linux.dev?part=3