Re: [PATCH bpf-next v4 16/46] 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 to consider:
- [High] bpf: missing exit->speculative propagation in bpf_flow_return()
--

commit 96816fe78c2229631b095b8d13a8d312f8d07492
Author: Alexei Starovoitov <ast@kernel.org>

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

This commit adds 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.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d42c67d6d70b2..5e66d6768fcf7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11757,8 +11757,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?

When a subprogram verified on its own spawns a speculative path via
sanitize_speculative_path() that reaches BPF_EXIT, caller->speculative is
false in the waiting caller state.  Leaving caller->speculative false on
return treats the speculative return path as architectural execution in the
caller: instructions reached only along that path are marked seen (skipping
dead-code sanitization), speculative return bounds in R0/R2 are joined into
the caller's architectural state, and recoverable speculative errors in the
caller are rejected instead of being mitigated with a speculation barrier.

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