Re: [PATCH bpf-next 5/8] bpf: Add may_goto to loops that are not walked to the end

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf: kernel panic via interpreter fallback for unbounded loops
--

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

bpf: Add may_goto to loops that are not walked to the end

This commit bounds BPF loop iterations at runtime by injecting a may_goto
instruction on the back-edges of loops that are not fully walked. If the
loop budget is exhausted, execution aborts by terminating in bpf_throw().

[ ... ]
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 2add8001c3ec3..2b965fa4bacc0 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
[ ... ]
> @@ -1537,6 +1537,143 @@ static int add_hidden_subprog(struct bpf_verifier_env *env, struct bpf_insn *pat
>  /* Do various post-verification rewrites in a single program pass.
>   * These rewrites simplify JIT and interpreter implementations.
>   */
> +/*
> + * The walk with widened loops is done. Loops where some state came back to
> + * a state that was still walked were not walked to the end and have to be
> + * bounded at run time.
> + */
> +int bpf_commit_loop_guards(struct bpf_verifier_env *env)
> +{
> +	struct bpf_insn_aux_data *aux = env->insn_aux_data;
> +	int i;
> +
> +	for (i = 0; i < env->prog->len; i++) {
> +		if (!aux[i].scc || !test_bit(aux[i].scc, env->scc_converged))
> +			continue;
> +		if (aux[i].guard_impossible) {
> +			verbose(env, "loop with back-edge at insn %d cannot be bounded\n", i);
> +			return -E2BIG;
> +		}
> +		if (aux[i].guard_pending) {
> +			aux[i].loop_guard = true;
> +			env->seen_exception = true;

[Severity: Critical]
Does this sequence allow a kernel panic if the BPF program falls back to the
interpreter?

When the verifier implicitly injects a bpf_throw call as a runtime loop guard
by setting env->seen_exception = true here, it doesn't appear to set
env->prog->jit_required = 1.

If the system falls back to the BPF interpreter, for example due to memory
constraints or program size, the interpreter will hit the loop guard and
execute bpf_throw(). When this happens, arch_bpf_stack_walk() cannot find any
BPF JIT frames, returning without setting ctx.aux and leaving it NULL.
bpf_throw() then unconditionally dereferences ctx.aux, which triggers a NULL
pointer dereference.

Should env->prog->jit_required = 1 be set here, similar to what happens for
explicit bpf_throw kfunc calls, to prevent this fallback crash?

> +		}
> +	}
> +	return 0;
> +}

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