netdev - Re: [PATCH bpf-next 06/10] libbpf: Change signature of bpf_prog

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: <CAEf4BzZUiNC_QcnfzbS9jctVNqNOEBio29k6Qtdd=Fq9WFM6PQ@mail.gmail.com>
Date:   Wed, 5 Oct 2022 20:19:37 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     bpf@...r.kernel.org, razor@...ckwall.org, ast@...nel.org,
        andrii@...nel.org, martin.lau@...ux.dev, john.fastabend@...il.com,
        joannelkoong@...il.com, memxor@...il.com, toke@...hat.com,
        joe@...ium.io, netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next 06/10] libbpf: Change signature of bpf_prog_query

On Tue, Oct 4, 2022 at 4:12 PM Daniel Borkmann <daniel@...earbox.net> wrote:
>
> Minor signature change for bpf_prog_query() API, no change in behavior.
> An alternative option would be to add a new libbpf introspection API
> with close to 1:1 implementation of bpf_prog_query() but with changed
> prog_ids pointer. Given the change is just minor enough, we went for
> the first option here.
>
> Co-developed-by: Nikolay Aleksandrov <razor@...ckwall.org>
> Signed-off-by: Nikolay Aleksandrov <razor@...ckwall.org>
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> ---
>  tools/lib/bpf/bpf.c | 2 +-
>  tools/lib/bpf/bpf.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 1d49a0352836..18b1e91cc469 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -846,7 +846,7 @@ int bpf_prog_query_opts(int target_fd,
>  }
>
>  int bpf_prog_query(int target_fd, enum bpf_attach_type type, __u32 query_flags,
> -                  __u32 *attach_flags, __u32 *prog_ids, __u32 *prog_cnt)
> +                  __u32 *attach_flags, void *prog_ids, __u32 *prog_cnt)
>  {
>         LIBBPF_OPTS(bpf_prog_query_opts, opts);
>         int ret;
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 9c50beabdd14..bef7a5282188 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -386,7 +386,7 @@ LIBBPF_API int bpf_prog_query_opts(int target_fd,
>                                    struct bpf_prog_query_opts *opts);
>  LIBBPF_API int bpf_prog_query(int target_fd, enum bpf_attach_type type,
>                               __u32 query_flags, __u32 *attach_flags,
> -                             __u32 *prog_ids, __u32 *prog_cnt);
> +                             void *prog_ids, __u32 *prog_cnt);

ugh, this is pretty nasty. Let's not do that. Have you though about
re-using prog_attach_flags (we can add a union to name the field
differently) to return prios instead of adding struct bpf_query_info?
This would be consistent with other uses cases that use PROG_ATTACH
and PROG_QUERY approach?


>
>  LIBBPF_API int bpf_raw_tracepoint_open(const char *name, int prog_fd);
>  LIBBPF_API int bpf_task_fd_query(int pid, int fd, __u32 flags, char *buf,
> --
> 2.34.1
>