Re: [PATCH bpf-next v1 13/17] bpf: Report Program Structure CFG errors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `reason` stack buffer in `check_subprogs()` is undersized and will truncate the diagnostic message for moderately sized BPF programs.
- [Medium] The newly added diagnostic suggestion explicitly advises users to use `throw` to end subprograms, contradicting the verifier's actual enforcement logic.
--

commit 18889a5137c9a55cdba7f071532572199ed7120c
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Report Program Structure CFG errors

This commit augments selected subprogram CFG validation failures with
Program Structure reports. It covers jumps leaving the current subprogram
and subprograms falling through into the next.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e30d79149cc7d..325c162e3df0d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -2879,6 +2879,7 @@ static int check_subprogs(struct bpf_verifier_env *env)
>  	int i, subprog_start, subprog_end, off, cur_subprog = 0;
>  	struct bpf_subprog_info *subprog = env->subprog_info;
>  	struct bpf_insn *insn = env->prog->insnsi;
> +	char reason[160];
>  	int insn_cnt = env->prog->len;
>  
>  	/* now check that all jumps are within the same subprog */
> @@ -2907,6 +2908,15 @@ static int check_subprogs(struct bpf_verifier_env *env)
>  		off = i + bpf_jmp_offset(&insn[i]) + 1;
>  		if (off < subprog_start || off >= subprog_end) {
>  			verbose(env, "jump out of range from insn %d to %d\n", i, off);
> +			scnprintf(reason, sizeof(reason),
> +				  "Instruction %d jumps to instruction %d, but subprogram %d only contains instructions %d through %d. A branch target must stay inside "
> +				  "the same subprogram",
> +				  i, off, cur_subprog, subprog_start,
> +				  subprog_end - 1);

[Severity: Medium]
Does this code risk truncating the diagnostic message?

The format string contains 142 characters of static text and accepts 5 integer
arguments. Because BPF programs can contain up to 1,000,000 instructions,
instruction indices can take up to 7 digits.

Formatting these integers can add up to 31 characters, resulting in a string
length of around 173 bytes.

Since the reason buffer is limited to 160 bytes, scnprintf() will silently
truncate the output and the user will see a cut-off diagnostic message.

> +			bpf_diag_report_program_structure(env, i,
> +							  "jump out of range",
> +							  reason,
> +							  "Keep branch targets within the same subprogram, or use an explicit subprogram call.");
>  			return -EINVAL;
>  		}
>  next:
> @@ -2919,6 +2929,13 @@ static int check_subprogs(struct bpf_verifier_env *env)
>  			    code != (BPF_JMP32 | BPF_JA) &&
>  			    code != (BPF_JMP | BPF_JA)) {
>  				verbose(env, "last insn is not an exit or jmp\n");
> +				scnprintf(reason, sizeof(reason),
> +					  "Subprogram %d reaches its last instruction %d without an exit or jump, so control could continue into the next subprogram",
> +					  cur_subprog, i);
> +				bpf_diag_report_program_structure(env, i,
> +								  "subprogram can fall through",
> +								  reason,
> +								  "End each subprogram with exit, throw, or an explicit jump that keeps control flow inside the subprogram.");

[Severity: Medium]
Will this suggestion mislead users trying to end their subprogram with a throw?

The newly added suggestion advises users to use throw to end subprograms.

However, the immediately preceding logic in check_subprogs() verifies the final
instruction using:

    if (code != (BPF_JMP | BPF_EXIT) &&
        code != (BPF_JMP32 | BPF_JA) &&
        code != (BPF_JMP | BPF_JA)) {

This condition does not account for bpf_throw(), which is a kfunc and evaluates
as a BPF_JMP | BPF_CALL instruction.

If a user writes assembly following the newly provided suggestion and places a
bpf_throw call as the absolute end of their subprogram, won't the verifier
still reject it and print this exact error?

>  				return -EINVAL;
>  			}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605063412.974640-1-memxor@gmail.com?part=13