linux-kernel - RE: [RFC] bpf.2: Use standard types and attributes

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: <78af3c302dd5447887f4a14cd4629119@AcuMS.aculab.com>
Date:   Sat, 24 Apr 2021 20:43:30 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Alexei Starovoitov' <alexei.starovoitov@...il.com>,
        Alejandro Colomar <alx.manpages@...il.com>,
        bpf <bpf@...r.kernel.org>
CC:     "Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>,
        linux-man <linux-man@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "libc-alpha@...rceware.org" <libc-alpha@...rceware.org>,
        "gcc-patches@....gnu.org" <gcc-patches@....gnu.org>
Subject: RE: [RFC] bpf.2: Use standard types and attributes

From: Alexei Starovoitov
> Sent: 24 April 2021 00:20
> 
> On Fri, Apr 23, 2021 at 4:15 PM Alejandro Colomar
> <alx.manpages@...il.com> wrote:
> >
> > Some manual pages are already using C99 syntax for integral
> > types 'uint32_t', but some aren't.  There are some using kernel
> > syntax '__u32'.  Fix those.
> >
> > Some pages also document attributes, using GNU syntax
> > '__attribute__((xxx))'.  Update those to use the shorter and more
> > portable C2x syntax, which hasn't been standardized yet, but is
> > already implemented in GCC, and available through either --std=c2x
> > or any of the --std=gnu... options.
> >
> > Signed-off-by: Alejandro Colomar <alx.manpages@...il.com>
> > ---
> >  man2/bpf.2 | 47 +++++++++++++++++++++++------------------------
> >  1 file changed, 23 insertions(+), 24 deletions(-)
> >
> > diff --git a/man2/bpf.2 b/man2/bpf.2
> > index 6e1ffa198..204f01bfc 100644
> > --- a/man2/bpf.2
> > +++ b/man2/bpf.2
> > @@ -188,39 +188,38 @@ commands:
> >  .EX
> >  union bpf_attr {
> >      struct {    /* Used by BPF_MAP_CREATE */
> > -        __u32         map_type;
> > -        __u32         key_size;    /* size of key in bytes */
> > -        __u32         value_size;  /* size of value in bytes */
> > -        __u32         max_entries; /* maximum number of entries
> > -                                      in a map */
> > +        uint32_t    map_type;
> > +        uint32_t    key_size;    /* size of key in bytes */
> > +        uint32_t    value_size;  /* size of value in bytes */
> > +        uint32_t    max_entries; /* maximum number of entries
> > +                                    in a map */
> 
> Nack.
> The man page should describe the kernel api the way it is in .h file.

And the code below is no more portable that a #pragma'.
It is probably worse than __attribute__((aligned(8)))
+            uint64_t [[gnu::aligned(8)]] value;
The standards committee are smoking dope again.
At least the '__aligned_u64 value;' form stands a reasonable
chance of being converted by cpp into whatever your compiler supports.

OTOH the bfp developers want shooting for defining a structure
with hidden padding fields.
It they ensured that all 64bit fields were aligned they wouldn't
need the __aligned_u64 at all.
And would be much less likely to leak kernel stack to userspace.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)