Re: [PATCH bpf-next v1 03/17] bpf: Add source and instruction diagnostic context

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing array bounds check in `bpf_diag_func_name()` when resolving hidden subprograms.
- [Medium] Tight loop coupling silently drops BPF instructions when the diagnostic is triggered near the top of a file.
- [Low] Potential loss of the visual alignment caret (`^-- `) due to `scnprintf` truncation if the source code is heavily indented.
--

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

bpf: Add source and instruction diagnostic context

This patch adds source line and nearby instruction annotations to the BPF
verifier diagnostics. The renderer keeps source text in a fixed-width lane
and prints instructions in a stable right-hand gutter.

> diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c
> index d9a103cf3a16e..652d4627c2b0f 100644
> --- a/kernel/bpf/diagnostics.c
> +++ b/kernel/bpf/diagnostics.c

[ ... ]

> +static const char *bpf_diag_func_name(struct bpf_verifier_env *env, u32 insn_idx)
> +{
> +	const struct bpf_subprog_info *subprog;
> +	const struct bpf_func_info *info;
> +	const struct btf_type *type;
> +	int subprogno;
> +
> +	if (!env->prog->aux->func_info || !env->prog->aux->btf)
> +		return NULL;
> +
> +	subprog = bpf_find_containing_subprog(env, insn_idx);
> +	if (!subprog)
> +		return NULL;
> +
> +	subprogno = subprog - env->subprog_info;
> +	info = &env->prog->aux->func_info[subprogno];

[Severity: High]
If the verifier dynamically adds a hidden subprogram like the exception
callback, env->subprog_cnt increments but env->prog->aux->func_info_cnt
remains the same since it represents user-supplied BTF data. Can this code
cause an out-of-bounds read on the func_info array if a diagnostic is
triggered inside a hidden subprogram? Should this verify that subprogno is
less than env->prog->aux->func_info_cnt before accessing the array?

> +	type = btf_type_by_id(env->prog->aux->btf, info->type_id);
> +	if (!type)
> +		return NULL;

[ ... ]

> +static void bpf_diag_print_source_annotation(struct bpf_verifier_env *env,
> +					     int line_width, int indent,
> +					     const char *label,
> +					     const char *msg)
> +{
> +	char first_prefix[128], next_prefix[128], text[BPF_DIAG_MSG_LEN];
> +
> +	scnprintf(first_prefix, sizeof(first_prefix), "  %*s | %*s^-- ",
> +		  line_width + 4, "", indent, "");

[Severity: Low]
Since the indent comes from user-supplied BTF line info, what happens if the
source code has heavy indentation (for example, over 110 spaces)? Will
first_prefix truncate here and silently drop the "^-- " visual caret used
for alignment?

> +	scnprintf(next_prefix, sizeof(next_prefix), "  %*s | %*s    ",
> +		  line_width + 4, "", indent, "");
> +	scnprintf(text, sizeof(text), "%s: %s", label, msg);

[ ... ]

> +void bpf_diag_report_source(struct bpf_verifier_env *env, u32 insn_idx,
> +			    char marker, const char *fmt, ...)
> +{

[ ... ]

> +	for (line_num = start_line; line_num <= end_line; line_num++) {
> +		const char *line;
> +		int row = line_num - src.line_num;
> +
> +		line = line_num == src.line_num ?
> +		       src.line :
> +		       bpf_diag_find_source_line(env, src.file_name_off, line_num);
> +
> +		bpf_diag_print_source_insn_line(env,
> +						line_num == src.line_num ?
> +						">>> " : "    ",
> +						width, line_num, line,
> +						&diag_insn[row + BPF_DIAG_INSN_CONTEXT],

[Severity: Medium]
When a diagnostic is triggered near the top of a file, start_line is clamped
to 1. Because the instruction layout is coupled to the source line loop
using the line_num difference, doesn't this skip iterating over the earlier
BPF instructions that were already correctly populated in the diag_insn
buffer? Could this drop valid instructions from the verifier log output,
making diagnostics harder to read?

> +						insn_idx, insn_width);
> +		if (line_num == src.line_num)
> +			bpf_diag_print_source_annotation(env, width, indent,
> +							 label, msg);
> +	}
> +}

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