Re: [PATCH bpf-next v5 14/14] bpf: Report Policy helper and kfunc errors

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 64c5c31ed230..ff028a8c1cca 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -2924,6 +2924,10 @@ static int add_subprogs(struct bpf_verifier_env *env)
>
>  		if (!env->bpf_capable) {
>  			verbose(env, "loading/calling other bpf or kernel functions are allowed for CAP_BPF and CAP_SYS_ADMIN\n");
> +			bpf_diag_policy(
> +				env, i, "BPF-to-BPF function call",
> +				"loading or calling other BPF functions requires CAP_BPF or CAP_SYS_ADMIN",
> +				"Load this program with the required capability, or avoid BPF-to-BPF function calls in unprivileged programs.");
>  			return -EPERM;
>  		}

Does the operation string here accurately describe both cases? This
branch can fire for two types of instructions. The loop filter above
checks:

    if (!bpf_pseudo_func(insn) && !bpf_pseudo_call(insn))
            continue;

So it matches both actual BPF-to-BPF calls (bpf_pseudo_call) and
ld_imm64 instructions that load a subprogram address for later use as
a callback argument (bpf_pseudo_func).

The pre-existing verbose() message handles this correctly: "loading/
calling other bpf or kernel functions". The new policy report's reason
string also handles it: "loading or calling other BPF functions
requires...". But the operation string hardcodes "BPF-to-BPF function
call", which is only accurate for one of the two cases.

An unprivileged program containing a bpf_loop() callback but no actual
bpf2bpf calls would get a report whose source annotation points at an
`rX = <subprog>` ld_imm64 (which print_bpf_insn() renders as an
assignment) while stating "policy check failed for BPF-to-BPF function
call" and suggesting the user "avoid BPF-to-BPF function calls" that
don't exist in the program.

Would a conditional string based on bpf_pseudo_func(insn) vs
bpf_pseudo_call(insn) be more accurate here, or perhaps wording that
covers both cases like the verbose() message does?

The add_kfuncs() change at 2980 doesn't have this issue since its loop
filter checks only bpf_pseudo_kfunc_call(), so "kernel function call"
is always accurate there.

[ ... ]


---
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