Re: [PATCH bpf-next v8 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: verifier bypass via BPF exception unwind abort on traced frames
--

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

bpf, x86: Dispatch exception cleanup pads at run time

This commit introduces runtime dispatch of exception cleanup pads using the
ORC unwinder. It implements arch_bpf_stack_walk_ra() to traverse the stack
and rewrite return addresses to point to their corresponding cleanup pads.

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

[ ... ]

> @@ -4614,6 +4628,32 @@ 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 slot holds a function graph or kretprobe
> +		 * trampoline, not @addr, so it cannot be rewritten. Stopping
> +		 * leaves BPF frames returning to unverified paths, so warn.
> +		 */
> +		if (WARN_ON_ONCE(READ_ONCE_NOCHECK(*ra) != addr))
> +			break;

[Severity: High]
The comment above notes that this leaves BPF frames returning to unverified
paths. Since the verifier assumes bpf_throw() does not return and prunes
instructions following it, won't aborting the stack walk here allow the
execution of those unverified instructions?

Could a user with tracing permissions intentionally attach a tracer (like
kretprobe or fgraph) to trigger this break, bypassing the verifier and
forcing the execution of unverified BPF code?

> +		if (!consume_fn(cookie, (u64)addr, (u64)state.sp, (u64)state.bp, (u64 *)ra))
> +			break;
> +	}
> +#endif
> +}

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