netdev - Re: [PATCH bpf] bpf: btf: Fix BTF verification of the enum size in struct/union

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: <CAEf4Bza9J3e=dfzDud6KY7_=4Qv77YqW2srfdxKg9ieiUCJgXw@mail.gmail.com>
Date:   Fri, 28 Feb 2020 16:23:41 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Yoshiki Komachi <komachi.yoshiki@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf] bpf: btf: Fix BTF verification of the enum size in struct/union

On Thu, Feb 27, 2020 at 6:07 PM Yoshiki Komachi
<komachi.yoshiki@...il.com> wrote:
>
> btf_enum_check_member() checked if the size of "enum" as a struct
> member exceeded struct_size or not. Then, the function definitely
> compared it with the size of "int" now. Therefore, errors could occur
> when the size of the "enum" type was changed.
>
> Although the size of "enum" is 4-byte by default, users can change it
> as needed (e.g., the size of the following test variable is not 4-byte
> but 1-byte). It can be used as a struct member as below:
>
> enum test : char {

you can't specify that in pure C, but this will work for C:

struct {
    enum { X, Y, Z } __attribute__((packed)) e;
} tmp;

Please add such a selftest, as part of fixing this bug. Thanks!

Otherwise logic looks good.

>         X,
>         Y,
>         Z,
> };
>
> struct {
>         char a;
>         enum test b;
>         char c;
> } tmp;
>
> With the setup above, when I tried to load BTF, the error was given
> as below:
>
> ------------------------------------------------------------------
>
> [58] STRUCT (anon) size=3 vlen=3
>         a type_id=55 bits_offset=0
>         b type_id=59 bits_offset=8
>         c type_id=55 bits_offset=16
> [59] ENUM test size=1 vlen=3
>         X val=0
>         Y val=1
>         Z val=2
>
> [58] STRUCT (anon) size=3 vlen=3
>         b type_id=59 bits_offset=8 Member exceeds struct_size
>
> libbpf: Error loading .BTF into kernel: -22.
>
> ------------------------------------------------------------------
>
> The related issue was previously fixed by the commit 9eea98497951 ("bpf:
> fix BTF verification of enums"). On the other hand, this patch fixes
> my explained issue by using the correct size of "enum" declared in
> BPF programs.
>
> Fixes: 179cde8cef7e ("bpf: btf: Check members of struct/union")
> Signed-off-by: Yoshiki Komachi <komachi.yoshiki@...il.com>
> ---
>  kernel/bpf/btf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 7871400..32ab922 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -2418,7 +2418,7 @@ static int btf_enum_check_member(struct btf_verifier_env *env,
>
>         struct_size = struct_type->size;
>         bytes_offset = BITS_ROUNDDOWN_BYTES(struct_bits_off);
> -       if (struct_size - bytes_offset < sizeof(int)) {
> +       if (struct_size - bytes_offset < member_type->size) {
>                 btf_verifier_log_member(env, struct_type, member,
>                                         "Member exceeds struct_size");
>                 return -EINVAL;
> --
> 1.8.3.1
>