Re: [PATCH bpf 1/2] bpf: backtracking shouldn't clear outer frame R1-R5 for callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Flawed heuristic for distinguishing subprog exits from callback exits causes the verifier to erroneously reject safe programs that call a subprog immediately before a looping callback.
--

commit 09e9430ffafbb3bf72d2c301a4d20da098050142
Author: Eduard Zingerman <eddyz87@gmail.com>

bpf: backtracking shouldn't clear outer frame R1-R5 for callbacks

This commit fixes precision propagation for R1 during a second iteration of
bpf_loop(). It removes the logic in backtrack_insn() that previously cleared
R1-R5 when backtracking through a callback's BPF_EXIT, ensuring precision
marks are not incorrectly reset.

> diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
> index a2b18a9f1694c..4fe9065106739 100644
> --- a/kernel/bpf/backtrack.c
> +++ b/kernel/bpf/backtrack.c
> @@ -520,37 +520,34 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
[ ... ]
>  		} else if (opcode == BPF_EXIT) {
> -			bool r0_precise;
> +			bool from_subprog_call, r0_precise;
> +
> +			/* BPF_EXIT in subprog or callback always returns
> +			 * right after the call instruction, so by checking
> +			 * whether the instruction at subseq_idx-1 is subprog
> +			 * call or not we can distinguish actual exit from
> +			 * *subprog* from exit from *callback*. In the former
> +			 * case, we need to propagate r0 precision, if
> +			 * necessary. In the former we never do that.
> +			 */
> +			from_subprog_call = subseq_idx - 1 >= 0 &&
> +					    bpf_pseudo_call(&env->prog->insnsi[subseq_idx - 1]);

[Severity: High]
Can this heuristic misclassify a callback exit as a subprog exit?

If a program calls a subprog immediately followed by an iterating callback
helper (like bpf_loop()), and we backtrack from the callback's BPF_EXIT,
subseq_idx equals the index of bpf_loop(). Checking subseq_idx - 1 evaluates
the instruction before bpf_loop(), which happens to be the subprog call.

This makes from_subprog_call incorrectly evaluate to true for the callback.

> +
> +			r0_precise = from_subprog_call && bt_is_reg_set(bt, BPF_REG_0);
>  
>  			/* Backtracking to a nested function call, 'idx' is a part of
>  			 * the inner frame 'subseq_idx' is a part of the outer frame.
>  			 * In case of a regular function call, instructions giving
>  			 * precision to registers R1-R5 should have been found already.
> -			 * In case of a callback, it is ok to have R1-R5 marked for
> -			 * backtracking, as these registers are set by the function
> -			 * invoking callback.
> +			 * In case of a callback from bpf_loop(), R{1,4} in the calling
> +			 * frame would be set as precise and that is correct.
>  			 */
> -			if (subseq_idx >= 0 && bpf_calls_callback(env, subseq_idx))
> -				for (i = BPF_REG_1; i <= BPF_REG_5; i++)
> -					bt_clear_reg(bt, i);
> -			if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
> +			if (from_subprog_call && (bt_reg_mask(bt) & BPF_REGMASK_ARGS)) {
>  				verifier_bug(env, "backtracking exit unexpected regs %x",
>  					     bt_reg_mask(bt));
>  				return -EFAULT;
>  			}

[Severity: High]
Will this incorrectly trigger the verifier bug check for callbacks?

Because bpf_loop() requests R1 precision (which falls under BPF_REGMASK_ARGS),
the misclassified from_subprog_call causes this safety check to evaluate to
true. Could this prematurely abort verification with -EFAULT and falsely reject
perfectly safe BPF programs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-bug-015-backtrack-cb-args-precise-v1-1-68a8e2a821e0@gmail.com?part=1