Re: [PATCH bpf-next v1 1/6] bpf: implement KF_IMPLICIT_PROG_AUX_ARG flag

Alexei Starovoitov
2025-09-25T09:49:44Z

On Wed, Sep 24, 2025 at 10:17 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>
> Define KF_IMPLICIT_PROG_AUX_ARG and handle it in the BPF verifier.
>
> The mechanism of patching is exactly the same as for __prog parameter
> annotation: in check_kfunc_args() detect the relevant parameter and
> remember regno in cur_aux(env)->arg_prog.
>
> Then the (unchanged in this patch) fixup_kfunc_call() adds a mov
> instruction to set the actual pointer to prog_aux.
>
> The caveat for KF_IMPLICIT_PROG_AUX_ARG is in implicitness. We have to
> separately check that the number of arguments is under
> MAX_BPF_FUNC_REG_ARGS.
>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---
> include/linux/btf.h | 3 +++
> kernel/bpf/verifier.c | 43 ++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index f06976ffb63f..479ee96c2c97 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -79,6 +79,9 @@
> #define KF_ARENA_RET (1 << 13) /* kfunc returns an arena pointer */
> #define KF_ARENA_ARG1 (1 << 14) /* kfunc takes an arena pointer as its first argument */
> #define KF_ARENA_ARG2 (1 << 15) /* kfunc takes an arena pointer as its second argument */
> +/* kfunc takes a pointer to struct bpf_prog_aux as the last argument,
> + * passed implicitly in BPF */

This is neither networking nor kernel comment style.
Pls use proper kernel comment style in a new code,
and reformat old net/bpf style when adjusting old comments.

> +#define KF_IMPLICIT_PROG_AUX_ARG (1 << 16)

The name is too verbose imo.
How about
KF_HIDDEN_PROG_ARG
or
KF_PROG_LAST_ARG

"Implicit" is not 100% correct, since it's very explicit
in kfunc definition in C, but removed from BTF.
"Hidden" is also not an exact fit for the same reasons.
Hence my preference is KF_PROG_LAST_ARG.

"aux" part is also an implementation detail.

> /*
> * Tag marking a kernel function as a kfunc. This is meant to minimize the
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e892df386eed..f1f9ea21f99b 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11948,6 +11948,11 @@ static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta)
> return meta->kfunc_flags & KF_RCU_PROTECTED;
> }
>
> +static bool is_kfunc_with_implicit_prog_aux_arg(struct bpf_kfunc_call_arg_meta *meta)
> +{
> + return meta->kfunc_flags & KF_IMPLICIT_PROG_AUX_ARG;
> +}
> +
> static bool is_kfunc_arg_mem_size(const struct btf *btf,
> const struct btf_param *arg,
> const struct bpf_reg_state *reg)
> @@ -12029,6 +12034,18 @@ static bool is_kfunc_arg_prog(const struct btf *btf, const struct btf_param *arg
> return btf_param_match_suffix(btf, arg, "__prog");
> }
>
> +static int set_kfunc_arg_prog_regno(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta, u32 regno)
> +{
> + if (meta->arg_prog) {
> + verifier_bug(env, "Only 1 prog->aux argument supported per-kfunc");
> + return -EFAULT;
> + }
> + meta->arg_prog = true;
> + cur_aux(env)->arg_prog = regno;
> +
> + return 0;
> +}
> +
> static bool is_kfunc_arg_scalar_with_name(const struct btf *btf,
> const struct btf_param *arg,
> const char *name)
> @@ -13050,6 +13067,21 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> return -EINVAL;
> }
>
> + /* KF_IMPLICIT_PROG_AUX_ARG means that the kfunc has one less argument in BTF,
> + * so we have to set_kfunc_arg_prog_regno() outside the arg check loop.
> + */

Use kernel comment style.

> + if (is_kfunc_with_implicit_prog_aux_arg(meta)) {
> + if (nargs + 1 > MAX_BPF_FUNC_REG_ARGS) {
> + verifier_bug(env, "A kfunc with KF_IMPLICIT_PROG_AUX_ARG flag has %d > %d args",
> + nargs + 1, MAX_BPF_FUNC_REG_ARGS);
> + return -EFAULT;
> + }
> + u32 regno = nargs + 1;

Variable declaration should be first in the block
followed by a blank line.

Also I would remove this double "> MAX_BPF_FUNC_REG_ARGS" check.
Move if (is_kfunc_with_prog_last_arg(meta))
couple lines above before the check,
and actual_nargs = nargs + 1;
if (actual_nargs > MAX_BPF_FUNC_REG_ARGS)
to cover both cases.
I wouldn't worry that verbose() isn't too specific.
If it prints nargs and actual_nargs whoever develops a kfunc
can get an idea.
Also in the future there is a good chance we will add more
KF_FOO_LAST_ARG flags to cleanup other *_impl() kfuncs
that have a special last argument, like bpf_rbtree_add_impl.
If all of them copy paste "> MAX_BPF_FUNC_REG_ARGS" check
it will be too verbose. Hence one nargs check for them all.

pw-bot: cr