From: Linus Torvalds <torvalds@linux-foundation.org> To: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name> Cc: Kees Cook <keescook@chromium.org>, Patrick McLean <chutzpah@gentoo.org>, Emese Revfy <re.emese@gmail.com>, Al Viro <viro@zeniv.linux.org.uk>, Bruce Fields <bfields@redhat.com>, "Darrick J. Wong" <darrick.wong@oracle.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Linux NFS Mailing List <linux-nfs@vger.kernel.org>, Thorsten Leemhuis <regressions@leemhuis.info>, "kernel-hardening@lists.openwall.com" <kernel-hardening@lists.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) Date: Wed, 21 Feb 2018 14:47:44 -0800 [thread overview] Message-ID: <CA+55aFz-AD8rWb66rjN-YhAtPXukMiGSFfW3nBMCp8k1RYUvOQ@mail.gmail.com> (raw) In-Reply-To: <6be06ce5-87e6-0d9d-55b9-6c70c3578ecf@maciej.szmigiero.name> On Wed, Feb 21, 2018 at 2:19 PM, Maciej S. Szmigiero <mail@maciej.szmigiero.name> wrote: > > One can see that offsets used to access various members of struct path are > different, and also that the original file from step 3 contains an object > named "__randomize_layout". Whee. Thanks for root-causing this issue, and this syntax of ours is clearly *much* too fragile. We actually have similar issues with some of our other attributes, where out nice "helpful" attribute shorthand can end up being just silently interpreted as a variable name if they aren't defined in time. For most of our other attributes, it just doesn't matter all that much if some user doesn't happen to see the attribute. For __randomize_layout, it's obviously very fatal, and silently just generates crazy code. I'm not entirely sure what the right solution is, because it's obviously much too easy to miss some #include by mistake. It's easy to say "you should always include the proper header", but if a failure to do so doesn't end up with any warnings or errors, but just silent bad code generation, it's much too fragile. I wonder if we could change the syntax of that "__randomize_layout" thing. Some of our related helper macros (ie randomized_struct_fields_start/end) don't have the same problem, because if you don't have the define for them, the compiler will complain about bad syntax. 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. 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. 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
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org> To: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name> Cc: Kees Cook <keescook@chromium.org>, Patrick McLean <chutzpah@gentoo.org>, Emese Revfy <re.emese@gmail.com>, Al Viro <viro@zeniv.linux.org.uk>, Bruce Fields <bfields@redhat.com>, "Darrick J. Wong" <darrick.wong@oracle.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Linux NFS Mailing List <linux-nfs@vger.kernel.org>, Thorsten Leemhuis <regressions@leemhuis.info>, "kernel-hardening@lists.openwall.com" <kernel-hardening@lists.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) Date: Wed, 21 Feb 2018 14:47:44 -0800 [thread overview] Message-ID: <CA+55aFz-AD8rWb66rjN-YhAtPXukMiGSFfW3nBMCp8k1RYUvOQ@mail.gmail.com> (raw) In-Reply-To: <6be06ce5-87e6-0d9d-55b9-6c70c3578ecf@maciej.szmigiero.name> On Wed, Feb 21, 2018 at 2:19 PM, Maciej S. Szmigiero <mail@maciej.szmigiero.name> wrote: > > One can see that offsets used to access various members of struct path ar= e > different, and also that the original file from step 3 contains an object > named "__randomize_layout". Whee. Thanks for root-causing this issue, and this syntax of ours is clearly *much* too fragile. We actually have similar issues with some of our other attributes, where out nice "helpful" attribute shorthand can end up being just silently interpreted as a variable name if they aren't defined in time. For most of our other attributes, it just doesn't matter all that much if some user doesn't happen to see the attribute. For __randomize_layout, it's obviously very fatal, and silently just generates crazy code. I'm not entirely sure what the right solution is, because it's obviously much too easy to miss some #include by mistake. It's easy to say "you should always include the proper header", but if a failure to do so doesn't end up with any warnings or errors, but just silent bad code generation, it's much too fragile. I wonder if we could change the syntax of that "__randomize_layout" thing. Some of our related helper macros (ie randomized_struct_fields_start/end) don't have the same problem, because if you don't have the define for them, the compiler will complain about bad syntax. 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. 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. 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 =E2=80=98__rand= omize_layout=E2=80=99 } __randomize_layout; ^~~~~~~~~~~~~~~~~~ In file included from <command-line>:0:0: ././include/linux/kconfig.h:8:28: note: previous declaration of =E2=80=98__randomize_layout=E2=80=99 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
next prev parent reply other threads:[~2018-02-21 22:47 UTC|newest] Thread overview: 82+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-11-09 0:43 [nfsd4] potentially hardware breaking regression in 4.14-rc and 4.13.11 Patrick McLean 2017-11-09 2:40 ` Linus Torvalds 2017-11-09 3:45 ` Al Viro 2017-11-09 19:34 ` Patrick McLean 2017-11-09 19:38 ` Al Viro 2017-11-09 19:42 ` Patrick McLean 2017-11-09 19:37 ` Al Viro 2017-11-09 19:51 ` Patrick McLean 2017-11-09 20:04 ` Linus Torvalds 2017-11-09 21:16 ` Al Viro 2017-11-10 1:58 ` Patrick McLean 2017-11-10 13:53 ` Arnd Bergmann 2017-11-10 18:42 ` Linus Torvalds 2017-11-10 23:26 ` Patrick McLean 2017-11-11 0:27 ` Patrick McLean 2017-11-11 2:36 ` Linus Torvalds 2017-11-11 2:36 ` [kernel-hardening] " Linus Torvalds 2017-11-11 2:36 ` Linus Torvalds 2017-11-11 16:13 ` Kees Cook 2017-11-11 16:13 ` [kernel-hardening] " Kees Cook 2017-11-11 16:13 ` Kees Cook 2017-11-11 17:31 ` Linus Torvalds 2017-11-11 17:31 ` [kernel-hardening] " Linus Torvalds 2017-11-11 17:31 ` Linus Torvalds 2017-11-13 22:48 ` Patrick McLean 2017-11-13 22:48 ` [kernel-hardening] " Patrick McLean 2017-11-13 22:48 ` Patrick McLean 2017-11-17 0:54 ` Kees Cook 2017-11-17 0:54 ` [kernel-hardening] " Kees Cook 2017-11-17 0:54 ` Kees Cook 2017-11-17 19:03 ` Patrick McLean 2017-11-17 19:03 ` [kernel-hardening] " Patrick McLean 2017-11-17 19:03 ` Patrick McLean 2017-11-17 21:26 ` Kees Cook 2017-11-17 21:26 ` [kernel-hardening] " Kees Cook 2017-11-17 21:26 ` Kees Cook 2017-11-18 0:27 ` Patrick McLean 2017-11-18 0:27 ` [kernel-hardening] " Patrick McLean 2017-11-18 0:27 ` Patrick McLean 2017-11-18 0:55 ` Linus Torvalds 2017-11-18 0:55 ` [kernel-hardening] " Linus Torvalds 2017-11-18 0:55 ` Linus Torvalds 2017-11-18 1:54 ` Patrick McLean 2017-11-18 1:54 ` [kernel-hardening] " Patrick McLean 2017-11-18 1:54 ` Patrick McLean 2017-11-18 5:14 ` Kees Cook 2017-11-18 5:14 ` [kernel-hardening] " Kees Cook 2017-11-18 5:14 ` Kees Cook 2017-11-18 5:29 ` Linus Torvalds 2017-11-18 5:29 ` [kernel-hardening] " Linus Torvalds 2017-11-18 5:29 ` Linus Torvalds 2017-11-18 8:20 ` Kees Cook 2017-11-18 8:20 ` [kernel-hardening] " Kees Cook 2017-11-18 8:20 ` Kees Cook 2018-02-21 22:19 ` RANDSTRUCT structs need linux/compiler_types.h (Was: [nfsd4] potentially hardware breaking regression in 4.14-rc and 4.13.11) Maciej S. Szmigiero 2018-02-21 22:47 ` Linus Torvalds [this message] 2018-02-21 22:47 ` Linus Torvalds 2018-02-21 23:34 ` Kees Cook 2018-02-21 23:34 ` Kees Cook 2018-03-05 9:27 ` Masahiro Yamada 2018-03-05 9:27 ` Masahiro Yamada 2018-03-05 19:15 ` Kees Cook 2018-03-05 19:18 ` Linus Torvalds 2018-02-21 22:52 ` Kees Cook 2018-02-21 23:24 ` Linus Torvalds 2018-02-22 0:12 ` Kees Cook 2018-02-22 0:22 ` Linus Torvalds 2018-02-22 0:23 ` Kees Cook 2018-02-22 0:27 ` Kees Cook 2017-11-11 1:13 ` [nfsd4] potentially hardware breaking regression in 4.14-rc and 4.13.11 J. Bruce Fields 2017-11-11 2:32 ` Al Viro 2017-11-10 1:47 ` Patrick McLean 2017-11-09 20:47 ` J. Bruce Fields 2017-11-09 23:07 ` Patrick McLean 2017-11-13 22:59 ` bit tweaks [was: Re: [nfsd4] potentially hardware breaking regression in 4.14-rc and 4.13.11] Rasmus Villemoes 2017-11-13 23:30 ` Linus Torvalds 2017-11-13 23:54 ` Linus Torvalds 2017-11-14 22:24 ` Rasmus Villemoes 2017-11-14 22:43 ` Linus Torvalds 2017-11-14 23:53 ` Rasmus Villemoes 2017-11-15 0:02 ` Linus Torvalds 2017-11-11 2:47 ` [nfsd4] potentially hardware breaking regression in 4.14-rc and 4.13.11 Alan Cox
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CA+55aFz-AD8rWb66rjN-YhAtPXukMiGSFfW3nBMCp8k1RYUvOQ@mail.gmail.com \ --to=torvalds@linux-foundation.org \ --cc=bfields@redhat.com \ --cc=chutzpah@gentoo.org \ --cc=darrick.wong@oracle.com \ --cc=keescook@chromium.org \ --cc=kernel-hardening@lists.openwall.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nfs@vger.kernel.org \ --cc=mail@maciej.szmigiero.name \ --cc=re.emese@gmail.com \ --cc=regressions@leemhuis.info \ --cc=viro@zeniv.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.