linux-kernel - Re: [PATCH v2 bpf-next 1/4] libbpf: add btf_has_size() and btf

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: <CAEf4BzYvPiWnJYfVjg5qXaefYOsR1QHHzMfB6XFUSVeOA9W8Rg@mail.gmail.com>
Date:   Wed, 20 Jan 2021 20:11:17 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alan Maguire <alan.maguire@...cle.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, Martin Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        john fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, morbo@...gle.com,
        Shuah Khan <shuah@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 bpf-next 1/4] libbpf: add btf_has_size() and btf_int() inlines

On Sun, Jan 17, 2021 at 2:22 PM Alan Maguire <alan.maguire@...cle.com> wrote:
>
> BTF type data dumping will use them in later patches, and they
> are useful generally when handling BTF data.
>
> Signed-off-by: Alan Maguire <alan.maguire@...cle.com>
> ---
>  tools/lib/bpf/btf.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index 1237bcd..0c48f2e 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -294,6 +294,20 @@ static inline bool btf_is_datasec(const struct btf_type *t)
>         return btf_kind(t) == BTF_KIND_DATASEC;
>  }
>
> +static inline bool btf_has_size(const struct btf_type *t)
> +{
> +       switch (BTF_INFO_KIND(t->info)) {
> +       case BTF_KIND_INT:
> +       case BTF_KIND_STRUCT:
> +       case BTF_KIND_UNION:
> +       case BTF_KIND_ENUM:
> +       case BTF_KIND_DATASEC:
> +               return true;
> +       default:
> +               return false;
> +       }
> +}

it's not clear what "has_size" means, actually. E.g., array type
definitely has size, it's not just as readily available. And you are
actually misusing this in your algorithm, I'll point it out in the
respective patch. Please remove this, or if absolutely necessary move
into btf_dump.c as an inner static function.

> +
>  static inline __u8 btf_int_encoding(const struct btf_type *t)
>  {
>         return BTF_INT_ENCODING(*(__u32 *)(t + 1));
> @@ -309,6 +323,11 @@ static inline __u8 btf_int_bits(const struct btf_type *t)
>         return BTF_INT_BITS(*(__u32 *)(t + 1));
>  }
>
> +static inline __u32 btf_int(const struct btf_type *t)
> +{
> +       return *(__u32 *)(t + 1);
> +}
> +

there is btf_int_encoding(), btf_ind_offset() and btf_int_bits() that
properly decompose what btf_int() above returns. I'm not convinced
btf_int() has to be exposed as a public API.

>  static inline struct btf_array *btf_array(const struct btf_type *t)
>  {
>         return (struct btf_array *)(t + 1);
> --
> 1.8.3.1
>