* [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes @ 2018-02-22 17:41 Kees Cook 2018-02-22 18:04 ` Linus Torvalds 0 siblings, 1 reply; 17+ messages in thread From: Kees Cook @ 2018-02-22 17:41 UTC (permalink / raw) To: Linus Torvalds; +Cc: Patrick McLean, Maciej S. Szmigiero, linux-kernel The header files for some structures could get included in such a way that struct attributes (specifically __randomize_layout from path.h) would be parsed as variable names instead of attributes. This could lead to some instances of a structure being unrandomized, causing nasty GPFs, etc. This patch makes sure the compiler_types.h header is included in kconfig.h so that we've always got types and struct attributes defined, since kconfig.h is included from the compiler command line. Reported-by: Patrick McLean <chutzpah@gentoo.org> Root-caused-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> Fixes: 3859a271a003 ("randstruct: Mark various structs for randomization") Signed-off-by: Kees Cook <keescook@chromium.org> --- Updated to include Tested-by. Linus, this looks ready to go. I'll send -stable patches that just fix up path.h. --- include/linux/kconfig.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h index fec5076eda91..c5fd4ee776ba 100644 --- a/include/linux/kconfig.h +++ b/include/linux/kconfig.h @@ -64,4 +64,7 @@ */ #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option)) +/* Make sure we always have all types and struct attributes defined. */ +#include <linux/compiler_types.h> + #endif /* __LINUX_KCONFIG_H */ -- 2.7.4 -- Kees Cook Pixel Security ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes 2018-02-22 17:41 [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes Kees Cook @ 2018-02-22 18:04 ` Linus Torvalds 2018-02-22 19:57 ` Kees Cook 2018-02-22 21:07 ` Rasmus Villemoes 0 siblings, 2 replies; 17+ messages in thread From: Linus Torvalds @ 2018-02-22 18:04 UTC (permalink / raw) To: Kees Cook; +Cc: Patrick McLean, Maciej S. Szmigiero, Linux Kernel Mailing List On Thu, Feb 22, 2018 at 9:41 AM, Kees Cook <keescook@chromium.org> wrote: > > Updated to include Tested-by. Linus, this looks ready to go. Ok, applied. I'm a bit worried that this ends up bypassing our automatic dependency generation. Lookie here (in a fully built tree): find . -name '*.o.cmd' | xargs grep -L linux/compiler_types.h | xargs grep -l linux/kconfig.h | while read i; do j=$(echo $i | sed 's/\.o.cmd$/\.c/' | sed 's:/\.:/:'); test -f $j && echo $j; done shows that a number of files don't end up depending on that header file, even though it's included (that "grep -l linux/kconfig,h" triggers on the command itself having that "-include linux/kconfig.h" line). It looks like "gcc -M" just doesn't list any files that get included on the command line with "-include". Now, there are very *few* files that don't end up eventually including linux/compiler_types.h _some_ way, and I checked them all, and they really are so trivial that this doesn't matter. But it worries me a bit regardless. Linus ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes 2018-02-22 18:04 ` Linus Torvalds @ 2018-02-22 19:57 ` Kees Cook 2018-02-22 20:17 ` Linus Torvalds 2018-02-22 21:07 ` Rasmus Villemoes 1 sibling, 1 reply; 17+ messages in thread From: Kees Cook @ 2018-02-22 19:57 UTC (permalink / raw) To: Linus Torvalds Cc: Patrick McLean, Maciej S. Szmigiero, Linux Kernel Mailing List On Thu, Feb 22, 2018 at 10:04 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Thu, Feb 22, 2018 at 9:41 AM, Kees Cook <keescook@chromium.org> wrote: >> >> Updated to include Tested-by. Linus, this looks ready to go. > > Ok, applied. > > I'm a bit worried that this ends up bypassing our automatic dependency > generation. > > Lookie here (in a fully built tree): > > find . -name '*.o.cmd' | > xargs grep -L linux/compiler_types.h | > xargs grep -l linux/kconfig.h | > while read i; do > j=$(echo $i | sed 's/\.o.cmd$/\.c/' | sed 's:/\.:/:'); > test -f $j && echo $j; > done > > shows that a number of files don't end up depending on that header > file, even though it's included (that "grep -l linux/kconfig,h" > triggers on the command itself having that "-include linux/kconfig.h" > line). > > It looks like "gcc -M" just doesn't list any files that get included > on the command line with "-include". Hmm. But does that mean deps for kconfig.h are broken too? That seems silly. I'll take a look... -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes 2018-02-22 19:57 ` Kees Cook @ 2018-02-22 20:17 ` Linus Torvalds 0 siblings, 0 replies; 17+ messages in thread From: Linus Torvalds @ 2018-02-22 20:17 UTC (permalink / raw) To: Kees Cook; +Cc: Patrick McLean, Maciej S. Szmigiero, Linux Kernel Mailing List On Thu, Feb 22, 2018 at 11:57 AM, Kees Cook <keescook@chromium.org> wrote: > > Hmm. But does that mean deps for kconfig.h are broken too? That seems > silly. I'll take a look... Yes, kconfig.h itself shares the same problem, but it has generally been just about the config option testing itself, so you'd normally never care. I'm not saying that fixing that too would be wrong, I'm just saying that linux/compiler_types.h tends to have way more subtle stuff in it, and get changed in ways that are not directly related to the config options that we track other ways (ie our dependency making script very much is all about those config options). Linus ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes 2018-02-22 18:04 ` Linus Torvalds 2018-02-22 19:57 ` Kees Cook @ 2018-02-22 21:07 ` Rasmus Villemoes 2018-02-22 21:22 ` Kees Cook ` (2 more replies) 1 sibling, 3 replies; 17+ messages in thread From: Rasmus Villemoes @ 2018-02-22 21:07 UTC (permalink / raw) To: Linus Torvalds, Kees Cook Cc: Patrick McLean, Maciej S. Szmigiero, Linux Kernel Mailing List On 2018-02-22 19:04, Linus Torvalds wrote: > > Lookie here (in a fully built tree): > > find . -name '*.o.cmd' | > xargs grep -L linux/compiler_types.h | > xargs grep -l linux/kconfig.h | > while read i; do > j=$(echo $i | sed 's/\.o.cmd$/\.c/' | sed 's:/\.:/:'); > test -f $j && echo $j; > done > > shows that a number of files don't end up depending on that header > file, even though it's included (that "grep -l linux/kconfig,h" > triggers on the command itself having that "-include linux/kconfig.h" > line). > > It looks like "gcc -M" just doesn't list any files that get included > on the command line with "-include". It does, both per the documentation and testing it. But fixdep explicitly removes include/linux/kconfig.h along with include/generated/autoconf.h and a few others. So when you rebuilt after adding the #include to kconfig.h, I think nothing actually got built, and no new .o.cmd files got generated. Doing a clean build does make include/linux/compiler_{types,gcc}.h and the various fake include/config/.... they "depend" on appear in e.g. lib/.clz_tab.o.cmd. The whole point of fixdep and the include/config hierarchy is to be able to remove the dependency on autoconf.h, but I'm not sure I understand why kconfig.h itself is also forcibly removed. Rasmus ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes 2018-02-22 21:07 ` Rasmus Villemoes @ 2018-02-22 21:22 ` Kees Cook 2018-02-22 21:23 ` Linus Torvalds 2018-02-22 21:34 ` Rasmus Villemoes 2 siblings, 0 replies; 17+ messages in thread From: Kees Cook @ 2018-02-22 21:22 UTC (permalink / raw) To: Rasmus Villemoes Cc: Linus Torvalds, Patrick McLean, Maciej S. Szmigiero, Linux Kernel Mailing List On Thu, Feb 22, 2018 at 1:07 PM, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > On 2018-02-22 19:04, Linus Torvalds wrote: >> >> Lookie here (in a fully built tree): >> >> find . -name '*.o.cmd' | >> xargs grep -L linux/compiler_types.h | >> xargs grep -l linux/kconfig.h | >> while read i; do >> j=$(echo $i | sed 's/\.o.cmd$/\.c/' | sed 's:/\.:/:'); >> test -f $j && echo $j; >> done >> >> shows that a number of files don't end up depending on that header >> file, even though it's included (that "grep -l linux/kconfig,h" >> triggers on the command itself having that "-include linux/kconfig.h" >> line). >> >> It looks like "gcc -M" just doesn't list any files that get included >> on the command line with "-include". > > It does, both per the documentation and testing it. But fixdep > explicitly removes include/linux/kconfig.h along with > include/generated/autoconf.h and a few others. So when you rebuilt after > adding the #include to kconfig.h, I think nothing actually got built, > and no new .o.cmd files got generated. > > Doing a clean build does make include/linux/compiler_{types,gcc}.h and > the various fake include/config/.... they "depend" on appear in e.g. > lib/.clz_tab.o.cmd. I'm seeing the same. If I do: $ find . -name '*.o.cmd' | xargs rm $ make allmodconfig $ make -j... ... $ find-command-above... I get no hits. And doing spot-testing shows dep changes are detected: $ ls -l fs/nfsd/nfs4xdr.o -rw-rw-r-- 1 kees kees 276088 Feb 22 13:15 fs/nfsd/nfs4xdr.o $ make fs/nfsd/nfs4xdr.o ...generated-files-only... $ ls -l fs/nfsd/nfs4xdr.o -rw-rw-r-- 1 kees kees 276088 Feb 22 13:15 fs/nfsd/nfs4xdr.o $ touch include/linux/compiler_types.h $ make fs/nfsd/nfs4xdr.o ...various... CC [M] fs/nfsd/nfs4xdr.o $ ls -l fs/nfsd/nfs4xdr.o -rw-rw-r-- 1 kees kees 276088 Feb 22 13:21 fs/nfsd/nfs4xdr.o Am I missing something? > The whole point of fixdep and the include/config hierarchy is to be able > to remove the dependency on autoconf.h, but I'm not sure I understand > why kconfig.h itself is also forcibly removed. Is there a mode where we're now rebuilding too much? -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes 2018-02-22 21:07 ` Rasmus Villemoes 2018-02-22 21:22 ` Kees Cook @ 2018-02-22 21:23 ` Linus Torvalds 2018-02-22 21:54 ` Linus Torvalds 2018-02-22 21:34 ` Rasmus Villemoes 2 siblings, 1 reply; 17+ messages in thread From: Linus Torvalds @ 2018-02-22 21:23 UTC (permalink / raw) To: Rasmus Villemoes Cc: Kees Cook, Patrick McLean, Maciej S. Szmigiero, Linux Kernel Mailing List On Thu, Feb 22, 2018 at 1:07 PM, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > It does, both per the documentation and testing it. But fixdep > explicitly removes include/linux/kconfig.h along with > include/generated/autoconf.h and a few others. So when you rebuilt after > adding the #include to kconfig.h, I think nothing actually got built, > and no new .o.cmd files got generated. Hah. So it was the lack of kconfig.h dependency that bit my testing ;) Linus ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes 2018-02-22 21:23 ` Linus Torvalds @ 2018-02-22 21:54 ` Linus Torvalds 0 siblings, 0 replies; 17+ messages in thread From: Linus Torvalds @ 2018-02-22 21:54 UTC (permalink / raw) To: Rasmus Villemoes Cc: Kees Cook, Patrick McLean, Maciej S. Szmigiero, Linux Kernel Mailing List On Thu, Feb 22, 2018 at 1:23 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > Hah. So it was the lack of kconfig.h dependency that bit my testing ;) Confirmed. Rasmus was right, doing a full build after cleaning everything up did fix this. So it's really just <linux/kconfig.h> itself that is missing from dependencies. Linus ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes 2018-02-22 21:07 ` Rasmus Villemoes 2018-02-22 21:22 ` Kees Cook 2018-02-22 21:23 ` Linus Torvalds @ 2018-02-22 21:34 ` Rasmus Villemoes 2018-02-22 21:56 ` Linus Torvalds 2 siblings, 1 reply; 17+ messages in thread From: Rasmus Villemoes @ 2018-02-22 21:34 UTC (permalink / raw) To: Linus Torvalds, Kees Cook Cc: Patrick McLean, Maciej S. Szmigiero, Linux Kernel Mailing List On 2018-02-22 22:07, Rasmus Villemoes wrote: > > The whole point of fixdep and the include/config hierarchy is to be able > to remove the dependency on autoconf.h, but I'm not sure I understand > why kconfig.h itself is also forcibly removed. <goes digging> Ah, 6a5be57f "fixdep: fix extraneous dependencies", to work around kconfig.h (naturally) containing the CONFIG_ pattern a few times. Hm, we should probably make sure that kconfig.h is always on the dependency list in .o.cmd, but just exclude it from being processed for the CONFIG_ pattern. Rasmus ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes 2018-02-22 21:34 ` Rasmus Villemoes @ 2018-02-22 21:56 ` Linus Torvalds 2018-02-28 19:17 ` [PATCH 1/3] fixdep: remove stale references to uml-config.h Rasmus Villemoes 0 siblings, 1 reply; 17+ messages in thread From: Linus Torvalds @ 2018-02-22 21:56 UTC (permalink / raw) To: Rasmus Villemoes Cc: Kees Cook, Patrick McLean, Maciej S. Szmigiero, Linux Kernel Mailing List On Thu, Feb 22, 2018 at 1:34 PM, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > Hm, we should probably make sure that kconfig.h is always on the > dependency list in .o.cmd, but just exclude it from being processed for > the CONFIG_ pattern. That sounds sensible to me. Not that this likely has ever bit anybody outside of this one case, but when missing dependencies cause a missed re-compile, the resulting bugs can be _really_ subtle. Linus ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] fixdep: remove stale references to uml-config.h 2018-02-22 21:56 ` Linus Torvalds @ 2018-02-28 19:17 ` Rasmus Villemoes 2018-02-28 19:17 ` [PATCH 2/3] fixdep: remove some false CONFIG_ matches Rasmus Villemoes 2018-02-28 19:17 ` [PATCH 3/3] fixdep: do not ignore kconfig.h Rasmus Villemoes 0 siblings, 2 replies; 17+ messages in thread From: Rasmus Villemoes @ 2018-02-28 19:17 UTC (permalink / raw) To: Masahiro Yamada, Michal Marek Cc: Rasmus Villemoes, Al Viro, Richard Weinberger, user-mode-linux-devel, linux-kbuild, linux-kernel uml-config.h hasn't existed in this decade (87e299e5c750 - x86, um: get rid of uml-config.h). The few remaining UML_CONFIG instances are defined directly in terms of their real CONFIG symbol in common-offsets.h, so unlike when the symbols got defined via a sed script, anything that uses UML_CONFIG_FOO now should also automatically pick up a dependency on CONFIG_FOO via the normal fixdep mechanism (since common-offsets.h should at least recursively be a dependency). Hence I believe we should actually be able to ignore the HELLO_CONFIG_BOOM cases. Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Richard Weinberger <richard@nod.at> Cc: user-mode-linux-devel@lists.sourceforge.net Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- scripts/basic/fixdep.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index fa3d39b6f23b..d7fbe545dd5d 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -93,14 +93,6 @@ * (Note: it'd be easy to port over the complete mkdep state machine, * but I don't think the added complexity is worth it) */ -/* - * Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend onto - * CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not - * fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as - * UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h, - * through arch/um/include/uml-config.h; this fixdep "bug" makes sure that - * those files will have correct dependencies. - */ #include <sys/types.h> #include <sys/stat.h> @@ -286,7 +278,6 @@ static int is_ignored_file(const char *s, int len) { return str_ends_with(s, len, "include/generated/autoconf.h") || str_ends_with(s, len, "include/generated/autoksyms.h") || - str_ends_with(s, len, "arch/um/include/uml-config.h") || str_ends_with(s, len, "include/linux/kconfig.h") || str_ends_with(s, len, ".ver"); } -- 2.15.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] fixdep: remove some false CONFIG_ matches 2018-02-28 19:17 ` [PATCH 1/3] fixdep: remove stale references to uml-config.h Rasmus Villemoes @ 2018-02-28 19:17 ` Rasmus Villemoes 2018-02-28 19:17 ` [PATCH 3/3] fixdep: do not ignore kconfig.h Rasmus Villemoes 1 sibling, 0 replies; 17+ messages in thread From: Rasmus Villemoes @ 2018-02-28 19:17 UTC (permalink / raw) To: Masahiro Yamada, Michal Marek Cc: Rasmus Villemoes, linux-kbuild, linux-kernel The string CONFIG_ quite often appears after other alphanumerics, meaning that that instance cannot be referencing a Kconfig symbol. Omitting these means make has fewer files to stat() when deciding what needs to be rebuilt - for a defconfig build, this seems to remove about 2% of the (wildcard ...) lines from the .o.cmd files. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- scripts/basic/fixdep.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index d7fbe545dd5d..1b21870d6e7f 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -225,8 +225,13 @@ static int str_ends_with(const char *s, int slen, const char *sub) static void parse_config_file(const char *p) { const char *q, *r; + const char *start = p; while ((p = strstr(p, "CONFIG_"))) { + if (p > start && (isalnum(p[-1]) || p[-1] == '_')) { + p += 7; + continue; + } p += 7; q = p; while (*q && (isalnum(*q) || *q == '_')) -- 2.15.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] fixdep: do not ignore kconfig.h 2018-02-28 19:17 ` [PATCH 1/3] fixdep: remove stale references to uml-config.h Rasmus Villemoes 2018-02-28 19:17 ` [PATCH 2/3] fixdep: remove some false CONFIG_ matches Rasmus Villemoes @ 2018-02-28 19:17 ` Rasmus Villemoes 2018-03-05 4:52 ` Masahiro Yamada 2018-03-05 14:49 ` Masahiro Yamada 1 sibling, 2 replies; 17+ messages in thread From: Rasmus Villemoes @ 2018-02-28 19:17 UTC (permalink / raw) To: Masahiro Yamada, Michal Marek Cc: Rasmus Villemoes, Linus Torvalds, linux-kbuild, linux-kernel kconfig.h was excluded from consideration by fixdep by 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false positive hits (1) include/config/.h (2) include/config/h.h (3) include/config/foo.h (1) occurred because kconfig.h contains the string CONFIG_ in a comment. However, since dee81e988674 (fixdep: faster CONFIG_ search), we have a check that the part after CONFIG_ is non-empty, so this does not happen anymore (and CONFIG_ appears by itself elsewhere, so that check is worthwhile). (2) comes from the include guard, __LINUX_KCONFIG_H. But with the previous patch, we no longer match that either. That leaves (3), which amounts to one [1] false dependency (aka stat() call done by make), which I think we can live with: We've already had one case [2] where the lack of include/linux/kconfig.h in the .o.cmd file caused a missing rebuild, and while I originally thought we should just put kconfig.h in the dependency list without parsing it for the CONFIG_ pattern, we actually do have some real CONFIG_ symbols mentioned in it, and one can imagine some translation unit that just does '#ifdef __BIG_ENDIAN' but doesn't through some other header actually depend on CONFIG_CPU_BIG_ENDIAN - so changing the target endianness could end up rebuilding the world, minus that small TU. Quoting Linus, ... when missing dependencies cause a missed re-compile, the resulting bugs can be _really_ subtle. [1] well, two, we now also have CONFIG_BOOGER/booger.h - we could change that to FOO if we care [2] https://lkml.org/lkml/2018/2/22/838 Cc: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- scripts/basic/fixdep.c | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index 1b21870d6e7f..449b68c4c90c 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -283,7 +283,6 @@ static int is_ignored_file(const char *s, int len) { return str_ends_with(s, len, "include/generated/autoconf.h") || str_ends_with(s, len, "include/generated/autoksyms.h") || - str_ends_with(s, len, "include/linux/kconfig.h") || str_ends_with(s, len, ".ver"); } -- 2.15.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] fixdep: do not ignore kconfig.h 2018-02-28 19:17 ` [PATCH 3/3] fixdep: do not ignore kconfig.h Rasmus Villemoes @ 2018-03-05 4:52 ` Masahiro Yamada 2018-03-05 8:15 ` Rasmus Villemoes 2018-03-05 14:49 ` Masahiro Yamada 1 sibling, 1 reply; 17+ messages in thread From: Masahiro Yamada @ 2018-03-05 4:52 UTC (permalink / raw) To: Rasmus Villemoes Cc: Michal Marek, Linus Torvalds, Linux Kbuild mailing list, Linux Kernel Mailing List 2018-03-01 4:17 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: > kconfig.h was excluded from consideration by fixdep by > 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false > positive hits > > (1) include/config/.h > (2) include/config/h.h > (3) include/config/foo.h > > (1) occurred because kconfig.h contains the string CONFIG_ in a > comment. However, since dee81e988674 (fixdep: faster CONFIG_ search), we > have a check that the part after CONFIG_ is non-empty, so this does not > happen anymore (and CONFIG_ appears by itself elsewhere, so that check > is worthwhile). > > (2) comes from the include guard, __LINUX_KCONFIG_H. But with the > previous patch, we no longer match that either. > > That leaves (3), which amounts to one [1] false dependency (aka stat() call > done by make), which I think we can live with: > > We've already had one case [2] where the lack of include/linux/kconfig.h in > the .o.cmd file caused a missing rebuild, and while I originally thought > we should just put kconfig.h in the dependency list without parsing it > for the CONFIG_ pattern, we actually do have some real CONFIG_ symbols > mentioned in it, and one can imagine some translation unit that just > does '#ifdef __BIG_ENDIAN' but doesn't through some other header > actually depend on CONFIG_CPU_BIG_ENDIAN - so changing the target > endianness could end up rebuilding the world, minus that small > TU. Quoting Linus, > > ... when missing dependencies cause a missed re-compile, the resulting > bugs can be _really_ subtle. > > [1] well, two, we now also have CONFIG_BOOGER/booger.h - we could change > that to FOO if we care > > [2] https://lkml.org/lkml/2018/2/22/838 > > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> Sorry, I missed to include this series in the Kbuild fixes PR I sent two days ago. I was not tracking the randomize-struct thread. I read [2] and I noticed the background of this series just now. Hopefully, I will have another opportunity of PR if this series is necessary for v4.16 (seems so) Regardless of the randomize-struct issue, this series is great! > --- > scripts/basic/fixdep.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c > index 1b21870d6e7f..449b68c4c90c 100644 > --- a/scripts/basic/fixdep.c > +++ b/scripts/basic/fixdep.c > @@ -283,7 +283,6 @@ static int is_ignored_file(const char *s, int len) > { > return str_ends_with(s, len, "include/generated/autoconf.h") || > str_ends_with(s, len, "include/generated/autoksyms.h") || > - str_ends_with(s, len, "include/linux/kconfig.h") || > str_ends_with(s, len, ".ver"); > } > > -- > 2.15.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] fixdep: do not ignore kconfig.h 2018-03-05 4:52 ` Masahiro Yamada @ 2018-03-05 8:15 ` Rasmus Villemoes 2018-03-05 9:38 ` Masahiro Yamada 0 siblings, 1 reply; 17+ messages in thread From: Rasmus Villemoes @ 2018-03-05 8:15 UTC (permalink / raw) To: Masahiro Yamada Cc: Michal Marek, Linus Torvalds, Linux Kbuild mailing list, Linux Kernel Mailing List On 5 March 2018 at 05:52, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2018-03-01 4:17 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: >> kconfig.h was excluded from consideration by fixdep by >> 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false >> positive hits [...] >> We've already had one case [2] where the lack of include/linux/kconfig.h in >> the .o.cmd file caused a missing rebuild, [...] >> [2] https://lkml.org/lkml/2018/2/22/838 > > > Sorry, I missed to include this series in the Kbuild fixes PR > I sent two days ago. > > I was not tracking the randomize-struct thread. > I read [2] and I noticed the background of this series just now. > > Hopefully, I will have another opportunity of PR > if this series is necessary for v4.16 (seems so) I should probably have put 3/3 first, if that is 4.16 material, the other two can obviously wait for 4.17. They don't really depend on each other apart from overlapping context, and the above commit message would need slight rewording if put before the others. I can do that reordering and resend if you wish. Rasmus ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] fixdep: do not ignore kconfig.h 2018-03-05 8:15 ` Rasmus Villemoes @ 2018-03-05 9:38 ` Masahiro Yamada 0 siblings, 0 replies; 17+ messages in thread From: Masahiro Yamada @ 2018-03-05 9:38 UTC (permalink / raw) To: Rasmus Villemoes Cc: Michal Marek, Linus Torvalds, Linux Kbuild mailing list, Linux Kernel Mailing List 2018-03-05 17:15 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: > On 5 March 2018 at 05:52, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: >> 2018-03-01 4:17 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: >>> kconfig.h was excluded from consideration by fixdep by >>> 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false >>> positive hits > [...] >>> We've already had one case [2] where the lack of include/linux/kconfig.h in >>> the .o.cmd file caused a missing rebuild, > [...] >>> [2] https://lkml.org/lkml/2018/2/22/838 >> >> >> Sorry, I missed to include this series in the Kbuild fixes PR >> I sent two days ago. >> >> I was not tracking the randomize-struct thread. >> I read [2] and I noticed the background of this series just now. >> >> Hopefully, I will have another opportunity of PR >> if this series is necessary for v4.16 (seems so) > > I should probably have put 3/3 first, if that is 4.16 material, the > other two can obviously wait for 4.17. They don't really depend on > each other apart from overlapping context, and the above commit > message would need slight rewording if put before the others. I can do > that reordering and resend if you wish. > > Rasmus 1/3 is obviously safe, 2/3 too. I think the current series is OK for v4.16. I will queue it up to the fixes branch shortly. Thanks! -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] fixdep: do not ignore kconfig.h 2018-02-28 19:17 ` [PATCH 3/3] fixdep: do not ignore kconfig.h Rasmus Villemoes 2018-03-05 4:52 ` Masahiro Yamada @ 2018-03-05 14:49 ` Masahiro Yamada 1 sibling, 0 replies; 17+ messages in thread From: Masahiro Yamada @ 2018-03-05 14:49 UTC (permalink / raw) To: Rasmus Villemoes Cc: Michal Marek, Linus Torvalds, Linux Kbuild mailing list, Linux Kernel Mailing List 2018-03-01 4:17 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: > kconfig.h was excluded from consideration by fixdep by > 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false > positive hits > > (1) include/config/.h > (2) include/config/h.h > (3) include/config/foo.h > > (1) occurred because kconfig.h contains the string CONFIG_ in a > comment. However, since dee81e988674 (fixdep: faster CONFIG_ search), we > have a check that the part after CONFIG_ is non-empty, so this does not > happen anymore (and CONFIG_ appears by itself elsewhere, so that check > is worthwhile). > > (2) comes from the include guard, __LINUX_KCONFIG_H. But with the > previous patch, we no longer match that either. > > That leaves (3), which amounts to one [1] false dependency (aka stat() call > done by make), which I think we can live with: > > We've already had one case [2] where the lack of include/linux/kconfig.h in > the .o.cmd file caused a missing rebuild, and while I originally thought > we should just put kconfig.h in the dependency list without parsing it > for the CONFIG_ pattern, we actually do have some real CONFIG_ symbols > mentioned in it, and one can imagine some translation unit that just > does '#ifdef __BIG_ENDIAN' but doesn't through some other header > actually depend on CONFIG_CPU_BIG_ENDIAN - so changing the target > endianness could end up rebuilding the world, minus that small > TU. Quoting Linus, > > ... when missing dependencies cause a missed re-compile, the resulting > bugs can be _really_ subtle. > > [1] well, two, we now also have CONFIG_BOOGER/booger.h - we could change > that to FOO if we care > > [2] https://lkml.org/lkml/2018/2/22/838 > > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- > scripts/basic/fixdep.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c > index 1b21870d6e7f..449b68c4c90c 100644 > --- a/scripts/basic/fixdep.c > +++ b/scripts/basic/fixdep.c > @@ -283,7 +283,6 @@ static int is_ignored_file(const char *s, int len) > { > return str_ends_with(s, len, "include/generated/autoconf.h") || > str_ends_with(s, len, "include/generated/autoksyms.h") || > - str_ends_with(s, len, "include/linux/kconfig.h") || > str_ends_with(s, len, ".ver"); > } > > -- > 2.15.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html The series, applied to linux-kbuild/fixes. Thanks! -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-03-05 14:50 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-22 17:41 [PATCH v3] kconfig.h: Include compiler types to avoid missed struct attributes Kees Cook 2018-02-22 18:04 ` Linus Torvalds 2018-02-22 19:57 ` Kees Cook 2018-02-22 20:17 ` Linus Torvalds 2018-02-22 21:07 ` Rasmus Villemoes 2018-02-22 21:22 ` Kees Cook 2018-02-22 21:23 ` Linus Torvalds 2018-02-22 21:54 ` Linus Torvalds 2018-02-22 21:34 ` Rasmus Villemoes 2018-02-22 21:56 ` Linus Torvalds 2018-02-28 19:17 ` [PATCH 1/3] fixdep: remove stale references to uml-config.h Rasmus Villemoes 2018-02-28 19:17 ` [PATCH 2/3] fixdep: remove some false CONFIG_ matches Rasmus Villemoes 2018-02-28 19:17 ` [PATCH 3/3] fixdep: do not ignore kconfig.h Rasmus Villemoes 2018-03-05 4:52 ` Masahiro Yamada 2018-03-05 8:15 ` Rasmus Villemoes 2018-03-05 9:38 ` Masahiro Yamada 2018-03-05 14:49 ` Masahiro Yamada
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.