Re: [PATCH bpf-next v3 16/45] bpf: Make the state after a call from the caller's state and an exit state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: missing exit->speculative propagation in bpf_flow_return()
--

commit 7ddf7363f0596e710519469bc5585f001bf30b01
Author: Alexei Starovoitov <ast@kernel.org>

bpf: Make the state after a call from the caller's state and an exit state

This commit introduces bpf_flow_return() and BPF_INHERITED_ID to construct
the caller's post-call verifier state from the waiting caller state and the
callee's exit state when verifying a static subprogram on its own. Held
reference IDs are mapped to positional names at subprog entry and restored
or refreshed upon return, falling back to caller-context verification if
inherited references or locks were released.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 07a9097e3d4e1..d8543eb8bb195 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11748,8 +11748,146 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins

[ ... ]

> +int bpf_flow_return(struct bpf_verifier_env *env, struct bpf_verifier_state *caller,
> +		    const struct bpf_verifier_state *exit, int call_idx)
> +{
> +	struct bpf_func_state *frame = caller->frame[caller->curframe];
> +	const struct bpf_func_state *callee = exit->frame[0];
> +	struct bpf_idmap *idmap = &env->idmap_scratch;
> +	u32 caller_rcu = caller->active_rcu_locks, caller_preempt = caller->active_preempt_locks;
> +	struct bpf_reference_state *held;
> +	struct bpf_reg_state *reg;
> +	u32 i, j, nregs, nheld, kept = 0, id;
> +	int err;
> +
> +	if (exit->unwound && !bpf_is_unwind_goto(&env->prog->insnsi[call_idx + 1])) {
> +		verbose(env, "unwind_exit, but no unwind_goto after the call at insn %d\n",
> +			call_idx);
> +		return -EINVAL;
> +	}
> +	caller->unwound = exit->unwound;

[Severity: High]
Should bpf_flow_return() also propagate exit->speculative to
caller->speculative here?

When !env->bypass_spec_v1, a non-speculative caller entering a static
subprogram verified on its own has caller->speculative == false. Inside the
subprogram, a statically predicted conditional branch or pointer ALU
operation can spawn a speculative path via sanitize_speculative_path() ->
push_stack(..., true) with speculative = true.

When that speculative path reaches BPF_EXIT, process_bpf_exit_full() ->
exit_func_on_its_own() -> bpf_flow_exit() -> flow_apply_exit() calls
bpf_flow_return() with exit->speculative == true and caller->speculative ==
false, and then delivers the resulting state to call_idx + 1 with
caller->speculative still false.

Because the post-call state loses its speculative flag:

1. sanitize_mark_insn_seen() marks instructions reached only along the
   callee's speculative exit path as seen, preventing sanitize_dead_code()
   from patching architecturally dead instructions after the call.

2. If the speculative return value in R0/R2 triggers an unsafe operation in
   the caller that would normally be recovered with a speculation barrier
   via error_recoverable_with_nospec(err) && state->speculative in
   do_check_block(), state->speculative is false, so no nospec barrier is
   inserted and the program is rejected.

3. flow_merge() and bpf_states_join() can join this speculative return state
   with the architectural return state, polluting the caller's architectural
   return register bounds with speculative values.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106035111.3315285-1-alexei.starovoitov@gmail.com?part=16