Re: [PATCH bpf-next v4 02/13] bpf: Split subprogram and kfunc collection

On Wed, Aug 5, 2026 at 2:05 PM Kumar Kartikeya Dwivedi <[email protected]> wrote:
>
> add_subprog_and_kfunc() combines two operations with different ordering
> requirements. Subprogram discovery must precede validation of func_info and
> line_info, while kfunc descriptors are only needed by the verifier after its
> initial program setup is complete.
>
> Split the helper into add_subprogs() and add_kfuncs() so each operation can be
> placed according to its actual dependencies. Keep both calls adjacent and in
> their existing phase for now, and add short comments describing their roles.
>
> No functional change is intended for valid programs.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>

Reviewed-by: Amery Hung <[email protected]>

> ---
>  kernel/bpf/verifier.c | 41 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 129e50888b90..24b163c2bd63 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2842,7 +2842,7 @@ int bpf_add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, u16 offset)
>         return 0;
>  }
>
> -static int add_subprog_and_kfunc(struct bpf_verifier_env *env)
> +static int add_subprogs(struct bpf_verifier_env *env)
>  {
>         struct bpf_subprog_info *subprog = env->subprog_info;
>         int i, ret, insn_cnt = env->prog->len, ex_cb_insn;
> @@ -2854,8 +2854,7 @@ static int add_subprog_and_kfunc(struct bpf_verifier_env *env)
>                 return ret;
>
>         for (i = 0; i < insn_cnt; i++, insn++) {
> -               if (!bpf_pseudo_func(insn) && !bpf_pseudo_call(insn) &&
> -                   !bpf_pseudo_kfunc_call(insn))
> +               if (!bpf_pseudo_func(insn) && !bpf_pseudo_call(insn))
>                         continue;
>
>                 if (!env->bpf_capable) {
> @@ -2863,11 +2862,7 @@ static int add_subprog_and_kfunc(struct bpf_verifier_env *env)
>                         return -EPERM;
>                 }
>
> -               if (bpf_pseudo_func(insn) || bpf_pseudo_call(insn))
> -                       ret = add_subprog(env, i + insn->imm + 1);
> -               else
> -                       ret = bpf_add_kfunc_call(env, insn->imm, insn->off);
> -
> +               ret = add_subprog(env, i + insn->imm + 1);
>                 if (ret < 0)
>                         return ret;
>         }
> @@ -2905,6 +2900,28 @@ static int add_subprog_and_kfunc(struct bpf_verifier_env *env)
>         return 0;
>  }
>
> +static int add_kfuncs(struct bpf_verifier_env *env)
> +{
> +       struct bpf_insn *insn = env->prog->insnsi;
> +       int i, ret, insn_cnt = env->prog->len;
> +
> +       for (i = 0; i < insn_cnt; i++, insn++) {
> +               if (!bpf_pseudo_kfunc_call(insn))
> +                       continue;
> +
> +               if (!env->bpf_capable) {
> +                       verbose(env, "loading/calling other bpf or kernel functions are allowed for CAP_BPF and CAP_SYS_ADMIN\n");

nit: Should this and the error message in add_subprogs() be updated to
be more specific?

> +                       return -EPERM;
> +               }
> +
> +               ret = bpf_add_kfunc_call(env, insn->imm, insn->off);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
>  static int check_subprogs(struct bpf_verifier_env *env)
>  {
>         int i, subprog_start, subprog_end, off, cur_subprog = 0;
> @@ -20322,7 +20339,13 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
>         if (ret < 0)
>                 goto skip_full_check;
>
> -       ret = add_subprog_and_kfunc(env);
> +       /* Discover all subprograms before validating their layout and BTF. */
> +       ret = add_subprogs(env);
> +       if (ret < 0)
> +               goto skip_full_check;
> +
> +       /* Collect the kfunc descriptors used during verification. */
> +       ret = add_kfuncs(env);
>         if (ret < 0)
>                 goto skip_full_check;
>
> --
> 2.53.0
>
>