linux-kernel - Re: [PATCH bpf-next v3 1/3] libbpf: Refactor libbpf_probe_bpf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzYTGiAedD8zEmw16NQ6JWAtkwDU2rhGLGZjXL0H1iKO+g@mail.gmail.com>
Date: Fri, 24 Jan 2025 10:44:08 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Tao Chen <chen.dylane@...il.com>
Cc: ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, eddyz87@...il.com, 
	haoluo@...gle.com, jolsa@...nel.org, qmo@...nel.org, bpf@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 1/3] libbpf: Refactor libbpf_probe_bpf_helper

On Fri, Jan 24, 2025 at 6:44 AM Tao Chen <chen.dylane@...il.com> wrote:
>
> Extract the common part as probe_func_comm, which will be used in
> both libbpf_probe_bpf_{helper, kfunc}
>
> Signed-off-by: Tao Chen <chen.dylane@...il.com>
> ---
>  tools/lib/bpf/libbpf_probes.c | 38 ++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index 9dfbe7750f56..b73345977b4e 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -413,22 +413,20 @@ int libbpf_probe_bpf_map_type(enum bpf_map_type map_type, const void *opts)
>         return libbpf_err(ret);
>  }
>
> -int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
> -                           const void *opts)
> +static int probe_func_comm(enum bpf_prog_type prog_type, struct bpf_insn insn,
> +                          char *accepted_msgs, size_t msgs_size)
>  {
>         struct bpf_insn insns[] = {
> -               BPF_EMIT_CALL((__u32)helper_id),
> +               BPF_EXIT_INSN(),
>                 BPF_EXIT_INSN(),
>         };
>         const size_t insn_cnt = ARRAY_SIZE(insns);
> -       char buf[4096];
> -       int ret;
> +       int err;
>
> -       if (opts)
> -               return libbpf_err(-EINVAL);
> +       insns[0] = insn;
>
>         /* we can't successfully load all prog types to check for BPF helper
> -        * support, so bail out with -EOPNOTSUPP error
> +        * and kfunc support, so bail out with -EOPNOTSUPP error
>          */
>         switch (prog_type) {
>         case BPF_PROG_TYPE_TRACING:

there isn't much logic that you will extract here besides this check
whether program type can even be successfully loaded, so I wouldn't
extract probe_func_comm(), but rather extract just the check:

static bool can_probe_prog_type(enum bpf_prog_type prog_type)
{
        /* we can't successfully load all prog types to check for BPF
helper/kfunc
         * support, so check this early and bail
         */
        switch (prog_type) {
            ...: return false
        default:
            return true;
}


And just check that can_probe_prog_type() inside
libbpf_probe_bpf_helper and libbpf_probe_bpf_kfunc

pw-bot: cr

> @@ -440,10 +438,26 @@ int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helpe
>                 break;
>         }
>
> -       buf[0] = '\0';
> -       ret = probe_prog_load(prog_type, insns, insn_cnt, buf, sizeof(buf));
> -       if (ret < 0)
> -               return libbpf_err(ret);
> +       accepted_msgs[0] = '\0';
> +       err = probe_prog_load(prog_type, insns, insn_cnt, accepted_msgs, msgs_size);
> +       if (err < 0)
> +               return libbpf_err(err);
> +
> +       return 0;
> +}
> +
> +int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
> +                           const void *opts)
> +{
> +       char buf[4096];
> +       int ret;
> +
> +       if (opts)
> +               return libbpf_err(-EINVAL);
> +
> +       ret = probe_func_comm(prog_type, BPF_EMIT_CALL((__u32)helper_id), buf, sizeof(buf));
> +       if (ret)
> +               return ret;
>
>         /* If BPF verifier doesn't recognize BPF helper ID (enum bpf_func_id)
>          * at all, it will emit something like "invalid func unknown#181".
> --
> 2.43.0
>