linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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: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: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: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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).