2018-02-11 19:34 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
> Looks to me like there's a few unrelated issues here:
> 1. The stack protector support test scripts
> Worthwhile IMO if they (*in practice*) prevent hard-to-debug build errors or a
> subtly broken kernel from being built.
> A few questions:
>     - How do things fail with a broken stack protector implementation?
>     - How common are those broken compilers?
>     - Do you really need to pass $(KBUILD_CPPFLAGS) when testing for breakage,
>       or would a simpler static test work in practice?
>       I don't know how messy it would be to get $(KBUILD_CPPFLAGS) into
>       Kconfig, but should make sure it's actually needed in any case.
>       The scripts are already split up as
>           scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>       by the way, though only gcc-x86_32-has-stack-protector.sh and
>       gcc-x86_64-has-stack-protector.sh exist.
>     - How old do you need to go with GCC for -fno-stack-protector to give an
>       error (i.e., for not even the option to be recognized)? Is it still
>       warranted to test for it?
> Adding some CCs who worked on the stack protector test scripts.
> And yeah, I was assuming that needing support scripts would be rare, and that
> you'd usually just check whether gcc accepts the flag.
> When you Google "gcc broken stack protector", the top hits about are about the
> scripts/gcc-x86_64-has-stack-protector.sh script in the kernel throwing a 
> false
> positive by the way (fixed in 82031ea29e45 ("scripts/has-stack-protector: add
> -fno-PIE")).
> 2. Whether to hide the Kconfig stack protector alternatives or always show 
> them
> Or equivalently, whether to automatically fall back on other stack protector
> alternatives (including no stack protector) if the one specified in the 
> .config
> isn't available.
> I'll let you battle this one out. In any case, as a user, I'd want a
> super-clear message telling me what to change if the build breaks because of
> missing stack protector support.
> 3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles
> I'd just go with whatever is simplest here. I don't find the Kconfig version
> too bad, but I'm already very familiar with Kconfig, so it's harder for me to
> tell how it looks to other people.
> I'd add some comments to explain the idea in the final version.
> @Kees:
> I was assuming that the Makefiles would error out with a message if none of 
> the
> CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning.
> You could offload part of that check to Kconfig with something like
>                 def_bool CC_STACKPROTECTOR_STRONG || \
>                          CC_STACKPROTECTOR_REGULAR || \
>                          CC_STACKPROTECTOR_NONE
> CHOSEN_STACKPROTECTOR_AVAILABLE could then be checked in the Makefile.
> It has the advantage of making the constraint clear in the Kconfig file
> at least.
> You could add some kind of assert feature to Kconfig too, but IMO it's not
> warranted purely for one-offs like this at least.
> That's details though. I'd want to explain it with a comment in any case if we
> go with something like this, since it's slightly kludgy and subtle
> (CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} form a kind of choice, only you can't
> express it like that directly, since it's derived from other symbols).
> Here's an overview of the current Kconfig layout by the way, assuming
> the old no-fallback behavior and CC_STACKPROTECTOR_AUTO being
> implemented in Kconfig:
>         # Feature tests
>         # User request
>         # The actual "output" to the Makefiles
>         # Some possible output "nicities"
> Does anyone have objections to the naming or other things? I saw some
> references to "Santa's wish list" in messages of commits that dealt with other
> variables named WANT_*, though I didn't look into those cases. ;)

I think Linus's comment was dismissed here.

Linus said:

> But yes, I also reacted to your earlier " It can't silently rewrite it
> to _REGULAR because the compiler support for _STRONG regressed."
> Because it damn well can. If the compiler doesn't support
> -fstack-protector-strong, we can just fall back on -fstack-protector.
> Silently. No extra crazy complex logic for that either.

If I understood his comment correctly,
we do not need either WANT_* or _AUTO.

Kees' comment:

> In the stack-protector case, this becomes quite important, since the
> goal is to record the user's selection regardless of compiler
> capability. For example, if someone selects _REGULAR, it shouldn't
> "upgrade" to _STRONG. (Similarly for _NONE.)

No.  Kconfig will not do this silently.

"make oldconfig" (or "make silentoldconfig" as well)
always ask users about new symbols.

For example, let's say your compiler supports -fstack-protector
but not -fstack-protector-strong.
CC_STACKPROTECTOR_REGULAR is the best you can choose.

Then, let's say you upgrade your compiler and the new compiler supports
-fstack-protector-strong as well.

In this case, CC_STACKPROTECTOR_STRONG is a newly visible symbol,
so Kconfig will ask you
"Hey, you were previously using _REGULAR, but your new compiler
also supports _STRONG.  Do you want to use it?"

The "make oldconfig" will look like follows:

masahiro@grover:~/workspace/linux-kbuild$ make oldconfig
  HOSTCC  scripts/kconfig/conf.o
  HOSTLD  scripts/kconfig/conf
scripts/kconfig/conf  --oldconfig Kconfig
* Restart config...
* Linux Kernel Configuration
Stack Protector buffer overflow detection

Please notice the Strong is marked as "(NEW)".

Kconfig handles this really nicely with Linus' suggestion.

[1] Users can select only features supported by your compiler
    - this makes sense.

[2] If you upgrade your compiler and it provides more capability,
    "make (silent)oldconfig" will ask you about new choices.
     - this also makes sense.

So, the following simple implementation works well enough,
doesn't it?

        prompt "Stack Protector buffer overflow detection"

        bool "Strong"
        select CC_STACKPROTECTOR

        bool "Regular"
        depends on CC_HAS_STACKPROTECTOR
        select CC_STACKPROTECTOR

        bool "None"


BTW, do we need to use 'choice' ?

The problem of using 'choice' is,
it does not work well with allnoconfig.

For all{yes,mod}config, we want to enable as many features as possible.
For allnoconfig, we want to disable as many as possible.

However, the default of 'choice' is always the first visible value.

So, I can suggest to remove _REGULAR and _NONE.

We have just two bool options, like follows.

        bool "Use stack protector"
        depends on CC_HAS_STACKPROTECTOR

        bool "Use strong strong stack protector"
        depends on CC_STACKPROTECTOR

This will work well for all{yes,mod,no}config.

We will not have a case where
-fstack-protector-strong is supported, but -fstack-protector is not.

Best Regards
Masahiro Yamada

Reply via email to