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

[Yonghong Song <yonghong.song@linux.dev>]

On 9/28/26 5:30 PM, sashiko-bot@kernel.org wrote:
> 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?

Will add a WARN_ON_ONCE to expose the failure if it happens.

>
>> +		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)
>>   {