Re: [PATCH bpf-next v4 01/46] bpf: Add may_goto to the loops that the verifier marks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] bpf: bpf_insert_loop_guards() logs shifted insn index instead of
  orig_idx
--

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

bpf: Add may_goto to the loops that the verifier marks

This commit introduces bpf_insert_loop_guards() to insert may_goto
instructions and subprogram exit sequences for loops marked by the
verifier during the main pass.

> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index f95af2ca3591b..6fd79c2fbdc1e 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -587,6 +587,225 @@ static int bpf_adj_moved_insn(struct bpf_insn *insn, u32 delta)
[ ... ]
> +int bpf_insert_loop_guards(struct bpf_verifier_env *env)
> +{
[ ... ]
> +	for (s = 0; s < env->subprog_cnt && !err; s++) {
> +		leave[s] = append_budget_exit(env, s);
> +		if (leave[s] < 0)
> +			err = leave[s];
> +	}
> +	/* A guard cannot jump into a patch that is in the list: 'r0 = 0' has no index yet. */
> +	err = err ?: bpf_patch_list_commit(env);
[ ... ]
> +		if (off > S16_MAX || off < S16_MIN) {
> +			verbose(env, "exit of the loop is too far from insn %d for may_goto\n", i);
> +			err = -ERANGE;
> +			break;
> +		}
> +		patch[jmp] = BPF_RAW_INSN(BPF_JMP | BPF_JCOND, 0, 0, off, 0);
> +		if (to == -2)
> +			patch[jmp] = budget_exit_insn(env, bpf_find_containing_subprog(env, i));
> +		patch[n] = env->prog->insnsi[i];
> +		/* A jump to itself is a back edge. It goes through may_goto like the others. */
> +		if (jumps_to_itself(&patch[n]))
> +			patch[n].off -= n;
> +		else
> +			err = bpf_adj_moved_insn(&patch[n], n);
> +		if (err) {
> +			verbose(env, "insn %d cannot be patched due to 16-bit range\n", i);
> +			break;
> +		}

[Severity: Low]
Should these two verbose() messages log aux[i].orig_idx instead of i?

The earlier bpf_patch_list_commit() call inserts append_budget_exit()
sequences at the end of subprograms, which shifts instruction indices i in
later subprograms away from their original positions in the user-supplied
program.

In addition, calling bpf_insert_loop_guards() in bpf_check() before
check_max_stack_depth() shifts the raw instruction indices reported by
check_exception_cb(), check_recursion(), and check_callbacks() when
check_max_stack_depth() rejects a program.

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