[PATCH] gcc-plugins: disable under COMPILE_TEST

[PATCH] gcc-plugins: disable under COMPILE_TEST

[Kees Cook]

Since adding the gcc plugin development headers is required for the
gcc plugin support, we should ease into this new kernel build dependency
more slowly. For now, disable the gcc plugins under COMPILE_TEST so that
all*config builds will skip it.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/Kconfig      | 1 +
 lib/Kconfig.debug | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 83aa2a3c470d..3f06d678b1d7 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -366,6 +366,7 @@ config HAVE_GCC_PLUGINS
 menuconfig GCC_PLUGINS
 	bool "GCC plugins"
 	depends on HAVE_GCC_PLUGINS
+	depends on !COMPILE_TEST
 	help
 	  GCC plugins are loadable modules that provide extra features to the
 	  compiler. They are useful for runtime instrumentation and static analysis.
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index d40833b9b7f4..0f9981999a27 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -713,8 +713,8 @@ config KCOV
 	bool "Code coverage for fuzzing"
 	depends on ARCH_HAS_KCOV
 	select DEBUG_FS
-	select GCC_PLUGINS
-	select GCC_PLUGIN_SANCOV
+	select GCC_PLUGINS if !COMPILE_TEST
+	select GCC_PLUGIN_SANCOV if !COMPILE_TEST
 	help
 	  KCOV exposes kernel code coverage information in a form suitable
 	  for coverage-guided fuzzing (randomized testing).
-- 
2.7.4


-- 
Kees Cook
Chrome OS & Brillo Security

Re: [PATCH] gcc-plugins: disable under COMPILE_TEST

[Paul Gortmaker]

[[PATCH] gcc-plugins: disable under COMPILE_TEST] On 11/06/2016 (Sat 09:12) Kees Cook wrote:

> Since adding the gcc plugin development headers is required for the
> gcc plugin support, we should ease into this new kernel build dependency
> more slowly. For now, disable the gcc plugins under COMPILE_TEST so that
> all*config builds will skip it.

Wouldn't it be better to test compile a one line program that tries to
source the header(s) and then react accordingly?

Then at least you would get the test coverage from people who have the
headers installed who are doing all[yes|mod]config.  This "for now"
solution doesn't really have a path forward other than assuming all
distros install the plugin headers sometime in the future.

Either way, this is an improvement over the current situation, so thanks
for that.

Paul.
--

> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  arch/Kconfig      | 1 +
>  lib/Kconfig.debug | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 83aa2a3c470d..3f06d678b1d7 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -366,6 +366,7 @@ config HAVE_GCC_PLUGINS
>  menuconfig GCC_PLUGINS
>  	bool "GCC plugins"
>  	depends on HAVE_GCC_PLUGINS
> +	depends on !COMPILE_TEST
>  	help
>  	  GCC plugins are loadable modules that provide extra features to the
>  	  compiler. They are useful for runtime instrumentation and static analysis.
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index d40833b9b7f4..0f9981999a27 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -713,8 +713,8 @@ config KCOV
>  	bool "Code coverage for fuzzing"
>  	depends on ARCH_HAS_KCOV
>  	select DEBUG_FS
> -	select GCC_PLUGINS
> -	select GCC_PLUGIN_SANCOV
> +	select GCC_PLUGINS if !COMPILE_TEST
> +	select GCC_PLUGIN_SANCOV if !COMPILE_TEST
>  	help
>  	  KCOV exposes kernel code coverage information in a form suitable
>  	  for coverage-guided fuzzing (randomized testing).
> -- 
> 2.7.4
> 
> 
> -- 
> Kees Cook
> Chrome OS & Brillo Security

Re: [PATCH] gcc-plugins: disable under COMPILE_TEST

[Emese Revfy]

On Sat, 11 Jun 2016 12:29:26 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> [[PATCH] gcc-plugins: disable under COMPILE_TEST] On 11/06/2016 (Sat 09:12) Kees Cook wrote:
> 
> > Since adding the gcc plugin development headers is required for the
> > gcc plugin support, we should ease into this new kernel build dependency
> > more slowly. For now, disable the gcc plugins under COMPILE_TEST so that
> > all*config builds will skip it.
> 
> Wouldn't it be better to test compile a one line program that tries to
> source the header(s) and then react accordingly?

The scripts/gcc-plugin.sh script does exactly that.
 
> Then at least you would get the test coverage from people who have the
> headers installed who are doing all[yes|mod]config.  This "for now"
> solution doesn't really have a path forward other than assuming all
> distros install the plugin headers sometime in the future.
> 
> Either way, this is an improvement over the current situation, so thanks
> for that.

If it is not too late I think this patch would be better:


When there is no gcc plugin support then don't compile the plugins
(but still print a warning). This allows building allyes/allmod configs
until the gcc plugin headers get installed.
    
Signed-off-by: Emese Revfy <re.emese@gmail.com>
---
 Makefile                     | 6 +++---
 scripts/Makefile.gcc-plugins | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index a49c075..715210c 100644
--- a/Makefile
+++ b/Makefile
@@ -623,15 +623,15 @@ endif
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
+include scripts/Makefile.gcc-plugins
+
 PHONY += gcc-plugins
 gcc-plugins: scripts_basic
-ifdef CONFIG_GCC_PLUGINS
+ifneq ($(GCC_PLUGINS_CFLAGS),)
 	$(Q)$(MAKE) $(build)=scripts/gcc-plugins
 endif
 	@:
 
-include scripts/Makefile.gcc-plugins
-
 ifdef CONFIG_READABLE_ASM
 # Disable optimizations that make assembler listings hard to read.
 # reorder blocks reorders the control in the function
diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index c7372cb..2f101ea 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -21,6 +21,7 @@ ifdef CONFIG_GCC_PLUGINS
         CFLAGS_KCOV := $(SANCOV_PLUGIN)
       else
         $(warning warning: cannot use CONFIG_KCOV: -fsanitize-coverage=trace-pc is not supported by compiler)
+        CFLAGS_KCOV =
       endif
     endif
   endif
@@ -37,13 +38,12 @@ ifdef CONFIG_GCC_PLUGINS
       else
         $(warning warning: your gcc version does not support plugins, you should upgrade it to gcc 4.5 at least)
       endif
+      GCC_PLUGINS_CFLAGS =
     endif
-  else
-    # SANCOV_PLUGIN can be only in CFLAGS_KCOV because avoid duplication.
-    GCC_PLUGINS_CFLAGS := $(filter-out $(SANCOV_PLUGIN), $(GCC_PLUGINS_CFLAGS))
   endif
 
-  KBUILD_CFLAGS += $(GCC_PLUGINS_CFLAGS)
+  # SANCOV_PLUGIN can be only in CFLAGS_KCOV because avoid duplication.
+  KBUILD_CFLAGS += $(filter-out $(SANCOV_PLUGIN), $(GCC_PLUGINS_CFLAGS))
   GCC_PLUGIN := $(gcc-plugin-y)
 
 endif

--
2.8.1

Re: [PATCH] gcc-plugins: disable under COMPILE_TEST

[Kees Cook]

On Sun, Jun 12, 2016 at 3:12 PM, Emese Revfy <re.emese@gmail.com> wrote:
> On Sat, 11 Jun 2016 12:29:26 -0400
> Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
>
>> [[PATCH] gcc-plugins: disable under COMPILE_TEST] On 11/06/2016 (Sat 09:12) Kees Cook wrote:
>>
>> > Since adding the gcc plugin development headers is required for the
>> > gcc plugin support, we should ease into this new kernel build dependency
>> > more slowly. For now, disable the gcc plugins under COMPILE_TEST so that
>> > all*config builds will skip it.
>>
>> Wouldn't it be better to test compile a one line program that tries to
>> source the header(s) and then react accordingly?
>
> The scripts/gcc-plugin.sh script does exactly that.
>
>> Then at least you would get the test coverage from people who have the
>> headers installed who are doing all[yes|mod]config.  This "for now"
>> solution doesn't really have a path forward other than assuming all
>> distros install the plugin headers sometime in the future.
>>
>> Either way, this is an improvement over the current situation, so thanks
>> for that.
>
> If it is not too late I think this patch would be better:

I don't like this because it means if someone specifically selects
some plugins in their .config, and the headers are missing, the kernel
will successfully compile. For many plugins, this results in a kernel
that lacks the requested security features, and that I really do not
want to have happening. I'm okay leaving these disabled for compile
tests for now. We can revisit this once more distros have plugins
enabled by default.

-Kees

>
>
> When there is no gcc plugin support then don't compile the plugins
> (but still print a warning). This allows building allyes/allmod configs
> until the gcc plugin headers get installed.
>
> Signed-off-by: Emese Revfy <re.emese@gmail.com>
> ---
>  Makefile                     | 6 +++---
>  scripts/Makefile.gcc-plugins | 8 ++++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index a49c075..715210c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -623,15 +623,15 @@ endif
>  # Tell gcc to never replace conditional load with a non-conditional one
>  KBUILD_CFLAGS  += $(call cc-option,--param=allow-store-data-races=0)
>
> +include scripts/Makefile.gcc-plugins
> +
>  PHONY += gcc-plugins
>  gcc-plugins: scripts_basic
> -ifdef CONFIG_GCC_PLUGINS
> +ifneq ($(GCC_PLUGINS_CFLAGS),)
>         $(Q)$(MAKE) $(build)=scripts/gcc-plugins
>  endif
>         @:
>
> -include scripts/Makefile.gcc-plugins
> -
>  ifdef CONFIG_READABLE_ASM
>  # Disable optimizations that make assembler listings hard to read.
>  # reorder blocks reorders the control in the function
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index c7372cb..2f101ea 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -21,6 +21,7 @@ ifdef CONFIG_GCC_PLUGINS
>          CFLAGS_KCOV := $(SANCOV_PLUGIN)
>        else
>          $(warning warning: cannot use CONFIG_KCOV: -fsanitize-coverage=trace-pc is not supported by compiler)
> +        CFLAGS_KCOV =
>        endif
>      endif
>    endif
> @@ -37,13 +38,12 @@ ifdef CONFIG_GCC_PLUGINS
>        else
>          $(warning warning: your gcc version does not support plugins, you should upgrade it to gcc 4.5 at least)
>        endif
> +      GCC_PLUGINS_CFLAGS =
>      endif
> -  else
> -    # SANCOV_PLUGIN can be only in CFLAGS_KCOV because avoid duplication.
> -    GCC_PLUGINS_CFLAGS := $(filter-out $(SANCOV_PLUGIN), $(GCC_PLUGINS_CFLAGS))
>    endif
>
> -  KBUILD_CFLAGS += $(GCC_PLUGINS_CFLAGS)
> +  # SANCOV_PLUGIN can be only in CFLAGS_KCOV because avoid duplication.
> +  KBUILD_CFLAGS += $(filter-out $(SANCOV_PLUGIN), $(GCC_PLUGINS_CFLAGS))
>    GCC_PLUGIN := $(gcc-plugin-y)
>
>  endif
>
> --
> 2.8.1



-- 
Kees Cook
Chrome OS & Brillo Security

Re: [PATCH] gcc-plugins: disable under COMPILE_TEST

[Emese Revfy]

On Sun, 12 Jun 2016 15:25:39 -0700
Kees Cook <keescook@chromium.org> wrote:

> I don't like this because it means if someone specifically selects
> some plugins in their .config, and the headers are missing, the kernel
> will successfully compile. For many plugins, this results in a kernel
> that lacks the requested security features, and that I really do not
> want to have happening. I'm okay leaving these disabled for compile
> tests for now. We can revisit this once more distros have plugins
> enabled by default.

You are right. Your patch is safer.

-- 
Emese

Re: [PATCH] gcc-plugins: disable under COMPILE_TEST

[Sudip Mukherjee]

On Sat, Jun 11, 2016 at 09:12:12AM -0700, Kees Cook wrote:
> Since adding the gcc plugin development headers is required for the
> gcc plugin support, we should ease into this new kernel build dependency
> more slowly. For now, disable the gcc plugins under COMPILE_TEST so that
> all*config builds will skip it.

But don't you think that people who are actually building the kernel
with allmodconfig or allyesconfig and/or with COMPILE_TEST enabled are
just doing that to build test the kernel. Atleast for me, when I am
testing i will always do allmodconfig but when I am building which is
to be deployed somewhere then I am building with a particular config.

Regards
Sudip

Re: [PATCH] gcc-plugins: disable under COMPILE_TEST

[Sedat Dilek]

On Sat, Jun 11, 2016 at 6:12 PM, Kees Cook <keescook@chromium.org> wrote:
> Since adding the gcc plugin development headers is required for the
> gcc plugin support, we should ease into this new kernel build dependency
> more slowly. For now, disable the gcc plugins under COMPILE_TEST so that
> all*config builds will skip it.
>

[ This might be a bit off-topic - Feel free to answer ]

Hi,

I want to try that new "GCC-plugin" feature.
Do you have a Git repo for "easy-testing"?

Does the kernel's build-system check for installed "gcc-plugin
development headers"?
Which GCC versions support "gcc-plugin" feature?
I am here on Ubuntu/precise AMD64 and have gcc-4.6.4 and gcc-4.9.2.

[ Optional ]
What about the topic and support for "LLVM/Clang and hardening" of the
Linux-kernel?

Regards,
- Sedat -

> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  arch/Kconfig      | 1 +
>  lib/Kconfig.debug | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 83aa2a3c470d..3f06d678b1d7 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -366,6 +366,7 @@ config HAVE_GCC_PLUGINS
>  menuconfig GCC_PLUGINS
>         bool "GCC plugins"
>         depends on HAVE_GCC_PLUGINS
> +       depends on !COMPILE_TEST
>         help
>           GCC plugins are loadable modules that provide extra features to the
>           compiler. They are useful for runtime instrumentation and static analysis.
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index d40833b9b7f4..0f9981999a27 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -713,8 +713,8 @@ config KCOV
>         bool "Code coverage for fuzzing"
>         depends on ARCH_HAS_KCOV
>         select DEBUG_FS
> -       select GCC_PLUGINS
> -       select GCC_PLUGIN_SANCOV
> +       select GCC_PLUGINS if !COMPILE_TEST
> +       select GCC_PLUGIN_SANCOV if !COMPILE_TEST
>         help
>           KCOV exposes kernel code coverage information in a form suitable
>           for coverage-guided fuzzing (randomized testing).
> --
> 2.7.4
>
>
> --
> Kees Cook
> Chrome OS & Brillo Security
> --
> To unsubscribe from this list: send the line "unsubscribe linux-next" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] gcc-plugins: disable under COMPILE_TEST

[Austin S. Hemmelgarn]

On 2016-06-12 20:18, Emese Revfy wrote:
> On Sun, 12 Jun 2016 15:25:39 -0700
> Kees Cook <keescook@chromium.org> wrote:
>
>> I don't like this because it means if someone specifically selects
>> some plugins in their .config, and the headers are missing, the kernel
>> will successfully compile. For many plugins, this results in a kernel
>> that lacks the requested security features, and that I really do not
>> want to have happening. I'm okay leaving these disabled for compile
>> tests for now. We can revisit this once more distros have plugins
>> enabled by default.
>
> You are right. Your patch is safer.
>
Why not make it so that if COMPILE_TEST is enabled, the build warns if 
it can't find the headers, otherwise it fails?  That way, people who are 
doing all*config builds but don't have the headers will still get some 
build coverage, and the people who are enabling it as a security feature 
will still get build failures.

Re: [PATCH] gcc-plugins: disable under COMPILE_TEST

[Kees Cook]

On Mon, Jun 13, 2016 at 11:32 AM, Austin S. Hemmelgarn
<ahferroin7@gmail.com> wrote:
> On 2016-06-12 20:18, Emese Revfy wrote:
>>
>> On Sun, 12 Jun 2016 15:25:39 -0700
>> Kees Cook <keescook@chromium.org> wrote:
>>
>>> I don't like this because it means if someone specifically selects
>>> some plugins in their .config, and the headers are missing, the kernel
>>> will successfully compile. For many plugins, this results in a kernel
>>> that lacks the requested security features, and that I really do not
>>> want to have happening. I'm okay leaving these disabled for compile
>>> tests for now. We can revisit this once more distros have plugins
>>> enabled by default.
>>
>>
>> You are right. Your patch is safer.
>>
> Why not make it so that if COMPILE_TEST is enabled, the build warns if it
> can't find the headers, otherwise it fails?  That way, people who are doing
> all*config builds but don't have the headers will still get some build
> coverage, and the people who are enabling it as a security feature will
> still get build failures.

I don't see a clear way to do this, but if you can find a way to make
that happen, please send a patch! :)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

Re: [PATCH] gcc-plugins: disable under COMPILE_TEST

[Kees Cook]

On Mon, Jun 13, 2016 at 1:40 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> On Sat, Jun 11, 2016 at 6:12 PM, Kees Cook <keescook@chromium.org> wrote:
>> Since adding the gcc plugin development headers is required for the
>> gcc plugin support, we should ease into this new kernel build dependency
>> more slowly. For now, disable the gcc plugins under COMPILE_TEST so that
>> all*config builds will skip it.
>>
>
> [ This might be a bit off-topic - Feel free to answer ]
>
> Hi,
>
> I want to try that new "GCC-plugin" feature.
> Do you have a Git repo for "easy-testing"?

Start with linux-next. It has the basic infrastructure. The
"latent_entropy" plugin is in my kssp tree here:
http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=kspp/gcc-plugins/latent_entropy
though it is not the most up to date version.

> Does the kernel's build-system check for installed "gcc-plugin
> development headers"?

Yes, when the plugins have been selected.

> Which GCC versions support "gcc-plugin" feature?

gcc-4.5 and newer.

> I am here on Ubuntu/precise AMD64 and have gcc-4.6.4 and gcc-4.9.2.

I strongly recommend upgrading to Ubuntu 16.04, but regardless, using
gcc 4.9 should be fine.

> [ Optional ]
> What about the topic and support for "LLVM/Clang and hardening" of the
> Linux-kernel?

I haven't been involved in that project, sorry.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

Re: [PATCH] gcc-plugins: disable under COMPILE_TEST

[Michael Ellerman]

On Mon, 2016-06-13 at 13:11 -0700, Kees Cook wrote:
> On Mon, Jun 13, 2016 at 11:32 AM, Austin S. Hemmelgarn
> <ahferroin7@gmail.com> wrote:
> > On 2016-06-12 20:18, Emese Revfy wrote:
> > > 
> > > On Sun, 12 Jun 2016 15:25:39 -0700
> > > Kees Cook <keescook@chromium.org> wrote:
> > > 
> > > > I don't like this because it means if someone specifically selects
> > > > some plugins in their .config, and the headers are missing, the kernel
> > > > will successfully compile. For many plugins, this results in a kernel
> > > > that lacks the requested security features, and that I really do not
> > > > want to have happening. I'm okay leaving these disabled for compile
> > > > tests for now. We can revisit this once more distros have plugins
> > > > enabled by default.
> > > 
> > > You are right. Your patch is safer.
> > > 
> > Why not make it so that if COMPILE_TEST is enabled, the build warns if it
> > can't find the headers, otherwise it fails?  That way, people who are doing
> > all*config builds but don't have the headers will still get some build
> > coverage, and the people who are enabling it as a security feature will
> > still get build failures.
> 
> I don't see a clear way to do this, but if you can find a way to make
> that happen, please send a patch! :)

Another option is to make the top-level option negative, that way when it's
enabled by allmod/yes the plugins are turned off.

So eg. you would have:

config DISABLE_GCC_PLUGINS
	bool "Disable building GCC plugins"
	default y
	...
	
This makes all the problems with allmod/yes go away, and means you always honor
the users intent - when DISABLE_GCC_PLUGINS=n you can fail the build if you
can't build the plugins.

The downside is the logic's a bit awkward, ie. to enable the plugins you have to
disable the option which disables them.

cheers