Re: [PATCH bpf-next v7 12/22] bpf, x86: Dispatch exception cleanup pads at run time

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: exception unwinding aborted when bpf_unwind is traced via kretprobe
--

commit f3ba2238caee72a50c097a775a0290b3b66ab98b
Author: Yonghong Song <yonghong.song@linux.dev>

bpf, x86: Dispatch exception cleanup pads at run time

This commit updates the BPF JIT on x86 to dispatch exception cleanup pads at
run time by writing directly to the return-address slot. It introduces
arch_bpf_stack_walk_ra() to support unwinding alongside the ORC unwinder.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 6c7a0578760e7..d4feade5b5c72 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c

[ ... ]

> @@ -4614,6 +4628,34 @@ void arch_bpf_stack_walk(bool (*consume_fn)(void *cookie, u64 ip, u64 sp, u64 bp
>  #endif
>  }
>  
> +void arch_bpf_stack_walk_ra(bool (*consume_fn)(void *cookie, u64 ip, u64 sp, u64 bp, u64 *ra),
> +			    void *cookie)
> +{
> +#if defined(CONFIG_UNWINDER_ORC)
> +	struct unwind_state state;
> +	unsigned long addr, *ra;
> +
> +	for (unwind_start(&state, current, NULL, NULL); !unwind_done(&state);
> +	     unwind_next_frame(&state)) {
> +		addr = unwind_get_return_address(&state);
> +		ra = unwind_get_return_address_ptr(&state);
> +		if (!addr || !ra)
> +			break;
> +		/*
> +		 * A traced return: the unwinder recovered @addr from under a
> +		 * function graph or kretprobe trampoline, which is what the
> +		 * slot itself still holds. Writing there would skip the
> +		 * trampoline and leave its entry for the next hooked return
> +		 * to pop.
> +		 */
> +		if (READ_ONCE_NOCHECK(*ra) != addr)
> +			break;

[Severity: High]
Will this unconditionally abort the unwinder if a kernel function like
bpf_unwind() is traced via a kretprobe or function graph tracer?

When a BPF program triggers an exception, bpf_unwind() invokes
arch_bpf_stack_walk_ra(). The unwinder processes kernel frames first. If
a user has legitimately attached a tracer to bpf_unwind(), the stack return
address slot (*ra) for that frame will hold the tracer's trampoline, while
the unwinder recovers the real return address (addr).

Because this evaluates to true and breaks the loop, it seems the walk stops
completely before it even reaches the BPF frames. Could this result in a
silent failure of BPF exception handling and resource leaks (e.g., unreleased
spinlocks or memory) since the cleanup pads are never executed?

> +		if (!consume_fn(cookie, (u64)addr, (u64)state.sp, (u64)state.bp, (u64 *)ra))
> +			break;
> +	}
> +#endif
> +}
> +
>  void bpf_arch_poke_desc_update(struct bpf_jit_poke_descriptor *poke,
>  			       struct bpf_prog *new, struct bpf_prog *old)
>  {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929001601.3242665-1-yonghong.song@linux.dev?part=12