Re: [PATCH bpf-next v12 07/25] bpf: Refuse a callback that can unwind

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

On 10/16/26 8:19 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] bpf: missing might_unwind propagation to callback in bpf_check_cfg()
> --
>
> commit 1785732c5b5ae17b86f08f6a94449da36fb9c958
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf: Refuse a callback that can unwind
>
> This commit adds bpf_exc_check_callback() and calls it in
> push_callback_call() to reject callback subprograms that have
> might_unwind set.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index fce2baa6272d1..fcf0d0fe0f2ff 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -11191,6 +11191,10 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
>>   	 * callbacks
>>   	 */
>>   	env->subprog_info[subprog].is_cb = true;
>> +	err = bpf_exc_check_callback(env, subprog);
>> +	if (err)
>> +		return err;
>> +
>>   	if (bpf_pseudo_kfunc_call(insn) &&
>>   	    !is_callback_calling_kfunc(insn->imm)) {
>>   		verifier_bug(env, "kfunc %s#%d not marked as callback-calling",
> [Severity: High]
> Can a callback subprogram that transitively calls bpf_unwind() bypass
> bpf_exc_check_callback() with env->subprog_info[subprog].might_unwind
> still false?
>
> In bpf_check_cfg(), direct calls (BPF_PSEUDO_CALL), function address
> loads (BPF_PSEUDO_FUNC), and read-only map function pointer loads
> (insn_func_ptrs()) are explored in a single DFS sharing
> env->cfg.insn_state[]:
>
> kernel/bpf/cfg.c:merge_callee_effects() {
> 	caller = bpf_find_containing_subprog(env, t);
> 	callee = bpf_find_containing_subprog(env, w);
> 	caller->changes_pkt_data |= callee->changes_pkt_data;
> 	caller->might_sleep |= callee->might_sleep;
> 	caller->might_throw |= callee->might_throw;
> 	caller->might_unwind |= callee->might_unwind;
> }
>
> merge_callee_effects() relies on callee w already being EXPLORED when
> returning to caller t. However, if subprogram H has a conditional branch
> whose fallthrough loads a read-only map containing the address of
> callback cb via insn_func_ptrs() (or loads cb via BPF_PSEUDO_FUNC when H
> is explored from a cleanup pad before main's fallthrough) and whose jump
> target calls bpf_unwind(), while cb calls H via BPF_PSEUDO_CALL:
>
> 1. bpf_check_cfg() enters H (marking insn_state[w_H] = DISCOVERED) and
>     explores H's fallthrough first, before visiting H's bpf_unwind()
>     branch, so H->might_unwind is still false.
>
> 2. H's fallthrough visits cb via visit_func_ptrs_insn() or
>     visit_func_call_insn().
>
> 3. When cb visits its BPF_PSEUDO_CALL to H, merge_callee_effects() sees
>     H->might_unwind == false, and push_insn() sees
>     (insn_state[w_H] & 0xF0) == DISCOVERED and returns DONE_EXPLORING.
>     Thus cb finishes CFG exploration with cb->might_unwind == false
>     before H explores its bpf_unwind() branch and sets
>     H->might_unwind = true.
>
> 4. At the end of bpf_check_cfg(), mark_callx_might_unwind() checks
>     addr_taken_subprog_might_unwind():
>
> kernel/bpf/cfg.c:mark_callx_might_unwind() {
> 	if (!addr_taken_subprog_might_unwind(env))
> 		return;
>      ...
> }
>
>     Because the address-taken subprogram cb still has might_unwind == false,
>     addr_taken_subprog_might_unwind() returns false and skips the
>     fixed-point propagation loop.

The previous patch 6 indeed has a bug and this may cause
addr_taken_subprog_might)unwind() return false and this will cause
issues. Fixing patch 6 issue should fix the issue.

>
> 5. Neither sort_subprogs_topo() nor check_max_stack_depth() rejects the
>     program because insn_func_ptrs() edges are not call edges and async
>     BPF_PSEUDO_FUNC callbacks are skipped in stack depth checks.
>
> When push_callback_call() then calls bpf_exc_check_callback(env, cb),
> env->subprog_info[cb].might_unwind is false, allowing the unwinding
> callback to pass verification and cause a verifier/runtime control-flow
> mismatch or jump to a zero epilogue_ip when bpf_unwind() is invoked at
> runtime.
>