* Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
@ 2016-06-12 22:12 ` Emese Revfy
0 siblings, 0 replies; 23+ messages in thread
From: Emese Revfy @ 2016-06-12 22:12 UTC (permalink / raw)
To: Paul Gortmaker
Cc: Kees Cook, Michal Marek, Stephen Rothwell, Sudip Mukherjee,
Linux-Next, linux-kernel, kernel-hardening
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
@ 2016-06-12 22:12 ` Emese Revfy
0 siblings, 0 replies; 23+ messages in thread
From: Emese Revfy @ 2016-06-12 22:12 UTC (permalink / raw)
To: Paul Gortmaker
Cc: Kees Cook, Michal Marek, Stephen Rothwell, Sudip Mukherjee,
Linux-Next, linux-kernel, kernel-hardening
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [kernel-hardening] Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
2016-06-12 22:12 ` Emese Revfy
@ 2016-06-12 22:25 ` Kees Cook
-1 siblings, 0 replies; 23+ messages in thread
From: Kees Cook @ 2016-06-12 22:25 UTC (permalink / raw)
To: Emese Revfy
Cc: Paul Gortmaker, Michal Marek, Stephen Rothwell, Sudip Mukherjee,
Linux-Next, LKML, kernel-hardening@lists.openwall.com
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
@ 2016-06-12 22:25 ` Kees Cook
0 siblings, 0 replies; 23+ messages in thread
From: Kees Cook @ 2016-06-12 22:25 UTC (permalink / raw)
To: Emese Revfy
Cc: Paul Gortmaker, Michal Marek, Stephen Rothwell, Sudip Mukherjee,
Linux-Next, LKML, kernel-hardening@lists.openwall.com
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [kernel-hardening] Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
2016-06-12 22:25 ` Kees Cook
@ 2016-06-13 0:18 ` Emese Revfy
-1 siblings, 0 replies; 23+ messages in thread
From: Emese Revfy @ 2016-06-13 0:18 UTC (permalink / raw)
To: Kees Cook
Cc: Paul Gortmaker, Michal Marek, Stephen Rothwell, Sudip Mukherjee,
Linux-Next, LKML, kernel-hardening@lists.openwall.com
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
@ 2016-06-13 0:18 ` Emese Revfy
0 siblings, 0 replies; 23+ messages in thread
From: Emese Revfy @ 2016-06-13 0:18 UTC (permalink / raw)
To: Kees Cook
Cc: Paul Gortmaker, Michal Marek, Stephen Rothwell, Sudip Mukherjee,
Linux-Next, LKML, kernel-hardening@lists.openwall.com
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [kernel-hardening] Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
2016-06-13 0:18 ` Emese Revfy
@ 2016-06-13 18:32 ` Austin S. Hemmelgarn
-1 siblings, 0 replies; 23+ messages in thread
From: Austin S. Hemmelgarn @ 2016-06-13 18:32 UTC (permalink / raw)
To: Emese Revfy, Kees Cook
Cc: Paul Gortmaker, Michal Marek, Stephen Rothwell, Sudip Mukherjee,
Linux-Next, LKML, kernel-hardening@lists.openwall.com
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
@ 2016-06-13 18:32 ` Austin S. Hemmelgarn
0 siblings, 0 replies; 23+ messages in thread
From: Austin S. Hemmelgarn @ 2016-06-13 18:32 UTC (permalink / raw)
To: Emese Revfy, Kees Cook
Cc: Paul Gortmaker, Michal Marek, Stephen Rothwell, Sudip Mukherjee,
Linux-Next, LKML, kernel-hardening@lists.openwall.com
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [kernel-hardening] Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
2016-06-13 18:32 ` Austin S. Hemmelgarn
@ 2016-06-13 20:11 ` Kees Cook
-1 siblings, 0 replies; 23+ messages in thread
From: Kees Cook @ 2016-06-13 20:11 UTC (permalink / raw)
To: Austin S. Hemmelgarn
Cc: Emese Revfy, Paul Gortmaker, Michal Marek, Stephen Rothwell,
Sudip Mukherjee, Linux-Next, LKML,
kernel-hardening@lists.openwall.com
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
@ 2016-06-13 20:11 ` Kees Cook
0 siblings, 0 replies; 23+ messages in thread
From: Kees Cook @ 2016-06-13 20:11 UTC (permalink / raw)
To: Austin S. Hemmelgarn
Cc: Emese Revfy, Paul Gortmaker, Michal Marek, Stephen Rothwell,
Sudip Mukherjee, Linux-Next, LKML,
kernel-hardening@lists.openwall.com
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [kernel-hardening] Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
2016-06-13 20:11 ` Kees Cook
@ 2016-06-14 2:01 ` Michael Ellerman
-1 siblings, 0 replies; 23+ messages in thread
From: Michael Ellerman @ 2016-06-14 2:01 UTC (permalink / raw)
To: Kees Cook, Austin S. Hemmelgarn
Cc: Emese Revfy, Paul Gortmaker, Michal Marek, Stephen Rothwell,
Sudip Mukherjee, Linux-Next, LKML,
kernel-hardening@lists.openwall.com
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] gcc-plugins: disable under COMPILE_TEST
@ 2016-06-14 2:01 ` Michael Ellerman
0 siblings, 0 replies; 23+ messages in thread
From: Michael Ellerman @ 2016-06-14 2:01 UTC (permalink / raw)
To: Kees Cook, Austin S. Hemmelgarn
Cc: Emese Revfy, Paul Gortmaker, Michal Marek, Stephen Rothwell,
Sudip Mukherjee, Linux-Next, LKML,
kernel-hardening@lists.openwall.com
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
^ permalink raw reply [flat|nested] 23+ messages in thread