| Message ID | alpine.DEB.2.02.1312301331520.10482@chino.kir.corp.google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 30, 2013 at 1:37 PM, David Rientjes <rientjes@google.com> wrote: > 8779657d29c0 ("stackprotector: Introduce CONFIG_CC_STACKPROTECTOR_STRONG") > causes the build to break when the compiler doesn't support > -fstack-protector-strong: > > cc1: error: unrecognized command line option ‘-fstack-protector-strong’ > cc1: error: unrecognized command line option ‘-fstack-protector-strong’ > > with at least gcc 4.6.3. > > Instead of breaking the build, just warn of the failure and disable the > feature. NAK. If you have selected CONFIG_CC_STACKPROTECTOR_STRONG, the build the fail hard. Without this, it means you'll end up with kernels that build and show a stackprotector option in their config, which is false. -Kees > > Signed-off-by: David Rientjes <rientjes@google.com> > --- > Makefile | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > --- a/Makefile > +++ b/Makefile > @@ -603,10 +603,11 @@ ifdef CONFIG_CC_STACKPROTECTOR_REGULAR > -fstack-protector not supported by compiler)) > endif > else ifdef CONFIG_CC_STACKPROTECTOR_STRONG > - stackp-flag := -fstack-protector-strong > - ifeq ($(call cc-option, $(stackp-flag)),) > + ifeq ($(call cc-option, -fstack-protector-strong),) > $(warning Cannot use CONFIG_CC_STACKPROTECTOR_STRONG: \ > -fstack-protector-strong not supported by compiler) > + else > + stackp-flag := -fstack-protector-strong > endif > else > # Force off for distro compilers that enable stack protector by default.
On 12/30/2013 1:37 PM, David Rientjes wrote: > 8779657d29c0 ("stackprotector: Introduce CONFIG_CC_STACKPROTECTOR_STRONG") > causes the build to break when the compiler doesn't support > -fstack-protector-strong: > > cc1: error: unrecognized command line option ‘-fstack-protector-strong’ > cc1: error: unrecognized command line option ‘-fstack-protector-strong’ > > with at least gcc 4.6.3. > > Instead of breaking the build, just warn of the failure and disable the > feature. ideally it also falls back to the less strict one, rather than not using stack protector at all... -- 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
On Mon, Dec 30, 2013 at 4:45 PM, Kees Cook <keescook@chromium.org> wrote: > > NAK. If you have selected CONFIG_CC_STACKPROTECTOR_STRONG, the build > the fail hard. Without this, it means you'll end up with kernels that > build and show a stackprotector option in their config, which is > false. What we really really want to do is to have some way to add config options based on shell scripts and compiler support. That would also get rid of a lot of Makefile trickery etc. Then we could just make CC_STACKPROTECTOR_STRONG depend on CC_SUPPORTS_STACKPROTECTOR_STRONG or whatever. Linus -- 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
Linus, All, On 2013-12-31 16:16 -0800, Linus Torvalds spake thusly: > On Mon, Dec 30, 2013 at 4:45 PM, Kees Cook <keescook@chromium.org> wrote: > > > > NAK. If you have selected CONFIG_CC_STACKPROTECTOR_STRONG, the build > > the fail hard. Without this, it means you'll end up with kernels that > > build and show a stackprotector option in their config, which is > > false. > > What we really really want to do is to have some way to add config > options based on shell scripts and compiler support. That would also > get rid of a lot of Makefile trickery etc. > > Then we could just make CC_STACKPROTECTOR_STRONG depend on > CC_SUPPORTS_STACKPROTECTOR_STRONG or whatever. Sam Ravnborg suggested somethink along those lines back in July: http://marc.info/?l=linux-kbuild&m=137399785206527&w=2 and a tentative implementation: http://marc.info/?l=linux-kbuild&m=137409581406434&w=2 Basically, that would give something like: config CC_SUPPORTS_STACKPROTECTOR_STRONG bool option exec="some/script/to/test-gcc -fstack-protector-strong" config CC_STACKPROTECTOR_STRONG bool "enable stack-protector strong" depends on CC_SUPPORTS_STACKPROTECTOR_STRONG Would that be something that match what you suggested above? Sam, there were some comments on that patch of yours. Do you want to update it and resubmit it? And, Happy New Year to All! Regards, Yann E. MORIN.
On Wed, Jan 1, 2014 at 3:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > On 2013-12-31 16:16 -0800, Linus Torvalds spake thusly: >> >> What we really really want to do is to have some way to add config >> options based on shell scripts and compiler support. That would also >> get rid of a lot of Makefile trickery etc. >> >> Then we could just make CC_STACKPROTECTOR_STRONG depend on >> CC_SUPPORTS_STACKPROTECTOR_STRONG or whatever. > > Sam Ravnborg suggested somethink along those lines back in July: > http://marc.info/?l=linux-kbuild&m=137399785206527&w=2 > and a tentative implementation: > http://marc.info/?l=linux-kbuild&m=137409581406434&w=2 Ack. Looks good to me. I've wanted this for a long time for other reasons, we should finally just do it. That said, we should make sure that the shell execution thing gets access to $(CC) etc variables that we have in > Basically, that would give something like: > > config CC_SUPPORTS_STACKPROTECTOR_STRONG > bool > option exec="some/script/to/test-gcc -fstack-protector-strong" For the compiler options, it would hopefully be sufficient to just do something like config CC_SUPPORTS_STACKPROTECTOR_STRONG bool option exec="$CC -fstack-protector-strong -c empty.c" or something like that. No? Linus -- 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
On 01/01/2014 11:33 AM, Linus Torvalds wrote: > > Ack. Looks good to me. I've wanted this for a long time for other > reasons, we should finally just do it. > Yes, we keep avoiding doing this by layering hack upon hack, but really... it needs to be done. The chief objection I've heard is that it makes "make oldconfig" necessary after a compiler change, but that seems entirely reasonable. -hpa -- 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
Linus, All, On 2014-01-01 11:33 -0800, Linus Torvalds spake thusly: > On Wed, Jan 1, 2014 at 3:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > > > On 2013-12-31 16:16 -0800, Linus Torvalds spake thusly: > >> > >> What we really really want to do is to have some way to add config > >> options based on shell scripts and compiler support. That would also > >> get rid of a lot of Makefile trickery etc. > >> > >> Then we could just make CC_STACKPROTECTOR_STRONG depend on > >> CC_SUPPORTS_STACKPROTECTOR_STRONG or whatever. > > > > Sam Ravnborg suggested somethink along those lines back in July: > > http://marc.info/?l=linux-kbuild&m=137399785206527&w=2 > > and a tentative implementation: > > http://marc.info/?l=linux-kbuild&m=137409581406434&w=2 > > Ack. Looks good to me. I've wanted this for a long time for other > reasons, we should finally just do it. > > That said, we should make sure that the shell execution thing gets > access to $(CC) etc variables that we have in This requires exporting them from the Makefiles (they are, in Makefile:391 and below). > > Basically, that would give something like: > > > > config CC_SUPPORTS_STACKPROTECTOR_STRONG > > bool > > option exec="some/script/to/test-gcc -fstack-protector-strong" > > For the compiler options, it would hopefully be sufficient to just do > something like > > config CC_SUPPORTS_STACKPROTECTOR_STRONG > bool > option exec="$CC -fstack-protector-strong -c empty.c" > > or something like that. No? This is an implementation detail, but the original patch expected the result to be 'y' or 'n' (or empty=='n') on stdout. That way, it could also be used to fill-in config options that are strings, or ints. Hence the use of a script. But H. Peter suggested it should only return a boolean, which seems entirely reasonable, given the purpose of this. In this case, using 'y' or 'n' from stdout, or 0 or !0 from the exit code are equally easy. Also, using a single shell script allows to fix/enhance all of those calls in a single place, and avoids duplicating all the check logic in every tests (eg. who is going to create empty.c in your example? Clean up the output file?). And since kconfig is run from the top-level of the Linux source tree (even for out-of-tree builds), we can safely use a path relative to that to call our script(s). I'll wait a bit until the end of the holiday season before I poke Sam again on this. Regards, Yann E. MORIN.
Hi all. > > > > config CC_SUPPORTS_STACKPROTECTOR_STRONG > > bool > > option exec="$CC -fstack-protector-strong -c empty.c" > > > > or something like that. No? > > This is an implementation detail, but the original patch expected the > result to be 'y' or 'n' (or empty=='n') on stdout. That way, it could > also be used to fill-in config options that are strings, or ints. Hence > the use of a script. > > But H. Peter suggested it should only return a boolean, which seems > entirely reasonable, given the purpose of this. In this case, using 'y' > or 'n' from stdout, or 0 or !0 from the exit code are equally easy. > > Also, using a single shell script allows to fix/enhance all of those > calls in a single place, and avoids duplicating all the check logic in > every tests (eg. who is going to create empty.c in your example? Clean > up the output file?). And since kconfig is run from the top-level of the > Linux source tree (even for out-of-tree builds), we can safely use a > path relative to that to call our script(s). > > I'll wait a bit until the end of the holiday season before I poke Sam > again on this. The thinking behind the exec option was that it should return a string, and then the content of the string were parsed depending on the type used in the kconfig language. So for a bool "y" and "n" would be recognized. For tristate in adddition "m" would be recognized. For int we should be able to parse numbers. And string would be string. An if an exec'ed command gave e return code != 0 then this should result in a warning, so user is told that the attempt to execute /bin/some_thing_random failed. Otherwise we would end in situations were this would be difficult to debug. I have no time to actually implement the above proposal - sorry! But things are busy at my day-time job etc. So I hope someone can step in and help here. PS. I have suffered from a faulty linux box + change of mail provider. And in the end I deleted all mails from the last three months. This was much quicker than to actually read them :-) Sam -- 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
diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -603,10 +603,11 @@ ifdef CONFIG_CC_STACKPROTECTOR_REGULAR -fstack-protector not supported by compiler)) endif else ifdef CONFIG_CC_STACKPROTECTOR_STRONG - stackp-flag := -fstack-protector-strong - ifeq ($(call cc-option, $(stackp-flag)),) + ifeq ($(call cc-option, -fstack-protector-strong),) $(warning Cannot use CONFIG_CC_STACKPROTECTOR_STRONG: \ -fstack-protector-strong not supported by compiler) + else + stackp-flag := -fstack-protector-strong endif else # Force off for distro compilers that enable stack protector by default.
8779657d29c0 ("stackprotector: Introduce CONFIG_CC_STACKPROTECTOR_STRONG") causes the build to break when the compiler doesn't support -fstack-protector-strong: cc1: error: unrecognized command line option ‘-fstack-protector-strong’ cc1: error: unrecognized command line option ‘-fstack-protector-strong’ with at least gcc 4.6.3. Instead of breaking the build, just warn of the failure and disable the feature. Signed-off-by: David Rientjes <rientjes@google.com> --- Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)