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] [day] [month] [year] [list]

Message-ID: <CAA6waGKtG_fE3Q+Dig9K1L3D9FRErQmpoMtMbPzuzkyODYXV7A@mail.gmail.com>
Date:   Mon, 2 Mar 2020 15:57:39 +0900
From:   Yoshiki Komachi <komachi.yoshiki@...il.com>
To:     Andrii Nakryiko <andrii.nakryiko@...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

2020年2月29日(土) 9:23 Andrii Nakryiko <andrii.nakryiko@...il.com>:
>
> 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.

Thank you for kind comments!
I will add a selftest program, and submit the next version later.

Best regards,

> >         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
> >