netdev - Re: [PATCH] bpf: fix uapi 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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAMuHMdV-0s_ikRmCrEcMCfkAp57Fu8WTUnJsopGagbYa+GGpbA@mail.gmail.com>
Date:   Tue, 4 Jun 2019 17:23:46 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Baruch Siach <baruch@...s.co.il>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Network Development <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>, "Dmitry V . Levin" <ldv@...linux.org>,
        Jiri Olsa <jolsa@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] bpf: fix uapi bpf_prog_info fields alignment

Hi Alexei,

On Tue, Jun 4, 2019 at 5:17 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
> On Tue, Jun 4, 2019 at 4:40 AM Baruch Siach <baruch@...s.co.il> wrote:
> > Merge commit 1c8c5a9d38f60 ("Merge
> > git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next") undid the
> > fix from commit 36f9814a494 ("bpf: fix uapi hole for 32 bit compat
> > applications") by taking the gpl_compatible 1-bit field definition from
> > commit b85fab0e67b162 ("bpf: Add gpl_compatible flag to struct
> > bpf_prog_info") as is. That breaks architectures with 16-bit alignment
> > like m68k. Widen gpl_compatible to 32-bit to restore alignment of the
> > following fields.
>
> The commit log is misleading and incorrect.
> Since compiler makes it into 16-bit field, it's a compiler bug.
> u32 in C should stay as u32 regardless of architecture.

C99 says (Section 6.7.2.1, Structure and union specifiers, Semantics)

    10  An implementation may allocate any addressable storage unit
        large enough to hold a bit-field.

$ cat hello.c
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>

struct x {
        unsigned int bit : 1;
        unsigned char byte;
};

int main(int argc, char *argv[])
{
        struct x x;

        printf("byte is at offset %zu\n", (uintptr_t)&x.byte - (uintptr_t)&x);
        printf("sizeof(x) = %zu\n", sizeof(x));
        exit(0);
}
$ gcc -Wall hello.c -o hello && ./hello
byte is at offset 1
sizeof(x) = 4
$ uname -m
x86_64

So the compiler allocates a single byte, even on a 64-bit platform!
The gap is solely determined by the alignment rule for the
successive field.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds