linux-kernel - Re: RANDSTRUCT structs need linux/compiler_types.h (Was: [nfsd4] potentially hardware breaking regression in 4.14-rc and 4.13.11)

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: <CAGXu5jK3ZEs5SauEMHA3BnAyM1zbOkoG5cbM3B44_67TH1EZ=A@mail.gmail.com>
Date:   Wed, 21 Feb 2018 15:34:34 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     "Maciej S. Szmigiero" <mail@...iej.szmigiero.name>,
        Patrick McLean <chutzpah@...too.org>,
        Emese Revfy <re.emese@...il.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Bruce Fields <bfields@...hat.com>,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
        Thorsten Leemhuis <regressions@...mhuis.info>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>
Subject: Re: RANDSTRUCT structs need linux/compiler_types.h (Was: [nfsd4]
 potentially hardware breaking regression in 4.14-rc and 4.13.11)

On Wed, Feb 21, 2018 at 2:47 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> And other attribute specifiers we encourage people to put in other
> parts of the type, like __user etc, so they don't have that same
> parsing issue.

Looking at other attributes we use on structs, we may have similar
risks for these:

__packed
____cacheline_aligned
____cacheline_aligned_in_smp
____cacheline_internodealigned_in_smp

But they just haven't been used in places that we could trip over it
as badly, AFAICT.

> I guess one _extreme_ fix for this would be to put
>
>     extern struct nostruct __randomize_layout;
>
> in our include/linux/kconfig.h, which I think we end up always
> including first thanks to having it on the command line.

We could do that for all the above, but I wonder if the real problem
is our convention of using "regular" names for these kinds of
attributes instead of parameterized names. If we always used something
like:

#define __struct(x)   __attribute__(x)

We'd avoid it, but we'd uglify our struct attributes:

struct thing { ... } __struct(randomize_layout);

though trying this now creates other problems. Hmmm.

(Regardless, let me send the nfs fix separately...)

-Kees

>
> Because if you do that, you actually get an error:
>
>     CC [M]  fs/nfsd/nfs4xdr.o
>   In file included from ./include/linux/fs_struct.h:5:0,
>                    from fs/nfsd/nfs4xdr.c:36:
>   ./include/linux/path.h:11:3: error: conflicting types for ‘__randomize_layout’
>    } __randomize_layout;
>      ^~~~~~~~~~~~~~~~~~
>   In file included from <command-line>:0:0:
>   ././include/linux/kconfig.h:8:28: note: previous declaration of
> ‘__randomize_layout’ was here
>        extern struct nostruct __randomize_layout;
>                               ^~~~~~~~~~~~~~~~~~
>   make[1]: *** [scripts/Makefile.build:317: fs/nfsd/nfs4xdr.o] Error 1
>
> and we would have figured this out immediately.
>
> Broken example patch appended, in case somebody wants to play with
> something like this or comes up with a better model entirely..
>
>                Linus
>
> ---
>
> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> index fec5076eda91..537dacb83380 100644
> --- a/include/linux/kconfig.h
> +++ b/include/linux/kconfig.h
> @@ -4,6 +4,10 @@
>
>  #include <generated/autoconf.h>
>
> +#ifndef __ASSEMBLY__
> + extern struct nostruct __randomize_layout;
> +#endif
> +
>  #define __ARG_PLACEHOLDER_1 0,
>  #define __take_second_arg(__ignored, val, ...) val



-- 
Kees Cook
Pixel Security