Re: [PATCH bpf-next v5 02/14] bpf: Add source and instruction diagnostic context

[email protected]
Newsgroups org.kernel.vger.bpf
Message-ID <c0dc914070f578da387c3dd7216691e7d90fedd1f0a029da6c197e9be098861f@mail.kernel.org>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 04cadd987169..ffa5626411ac 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -4147,8 +4147,16 @@ static inline bool bpf_is_subprog(const struct bpf_prog *prog)
>  }
>
>  const struct bpf_line_info *bpf_find_linfo(const struct bpf_prog *prog, u32 insn_off);
> -void bpf_get_linfo_file_line(struct btf *btf, const struct bpf_line_info *linfo,
> -			     const char **filep, const char **linep, int *nump);
> +struct bpf_linfo_source {
> +	const char *file;
> +	const char *line;
> +	u32 file_name_off;
> +	int line_num;
> +	int line_col;
> +};
> +
> +void bpf_get_linfo_source(struct btf *btf, const struct bpf_line_info *linfo,
> +			  struct bpf_linfo_source *src);
>  int bpf_prog_get_file_line(struct bpf_prog *prog, unsigned long ip, const char **filep,
>  			   const char **linep, int *nump);
>  struct bpf_prog *bpf_prog_find_from_stack(void);

[ ... ]

> diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c
> index e75753552a4d..815aa7938b50 100644
> --- a/kernel/bpf/diagnostics.c
> +++ b/kernel/bpf/diagnostics.c

[ ... ]

> @@ -14,6 +63,138 @@ bool bpf_diag_enabled(const struct bpf_verifier_env *env)
>
>  static void diag_write(struct bpf_verifier_env *env, const char *fmt, ...) __printf(2, 3);
>
> +int bpf_diag_init(struct bpf_verifier_env *env)
> +{

[ ... ]

> +static void diag_print_wrapped_prefixed(struct bpf_verifier_env *env, const char *first_prefix,
> +					const char *next_prefix, const char *text)
> +{

[ ... ]

> +static void disasm_print(void *private_data, const char *fmt, ...) __printf(2, 3);
> +
> +static void disasm_print(void *private_data, const char *fmt, ...)
> +{

[ ... ]

> @@ -45,3 +399,139 @@ static void diag_write(struct bpf_verifier_env *env, const char *fmt, ...)
>  	verbose(env, "%pV", &vaf);
>  	va_end(args);
>  }
> +
> +static void diag_format_source_text(char *buf, size_t size, const char *line, int width)
> +{

[ ... ]

> +static void bpf_diag_source(struct bpf_verifier_env *env, u32 insn_idx, const char *label,
> +			    const char *fmt, ...)
> +{
                                        ^^^^

Could bpf_diag_source() be annotated __printf(4, 5)? It is a printf-style
variadic function that forwards its format string and varargs to
bpf_diag_vfmt() -> vsnprintf(), but unlike the other printf-like helpers in
this file (diag_write and disasm_print, which both carry __printf(2, 3)
annotations), bpf_diag_source() has no format attribute.

Two consequences:

At W=1, scripts/Makefile.warn enables -Wmissing-format-attribute. GCC flags
bpf_diag_source() with "function might be a candidate for 'gnu_printf' format
attribute" because it does

    va_start(args, fmt);
    msg = bpf_diag_vfmt(env, fmt, args);

and bpf_diag_vfmt() is declared __printf(2, 0), which is exactly the pattern
the warning targets.

At the call sites, none of the format strings can be validated. Later commits
in this series add call sites with formats like "R%d is not readable", "owned
resource (id=%u)", "%s changed from %s to %s", and "%s %s; depth is now %u",
but the compiler cannot check the argument counts or types match the format
specifiers.

> +	struct diag_fmt_mark mark;
> +	struct bpf_diag *diag;
> +	const char *msg;
> +	va_list args;
> +	int ret;
> +
> +	if (!bpf_diag_enabled(env))
> +		return;

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31870608176
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.