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

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

On 10/1/26 6:49 AM, sashiko-bot@kernel.org wrote:
> 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?

Okay, this has been mentioned a few times. I think I should follow previous
suggestions e.g. mark bpf_unwind() as notrace etc. Will fix.

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