GCC plugins should only exist when some compiler feature needs to be
proven but does not exist in either GCC nor Clang. For example, if a
desired feature is already in Clang, it should be added to GCC upstream.
Document this explicitly.
Additionally, mark the plugins with matching upstream GCC features as
removable past their respective GCC versions.
Cc: Masahiro Yamada <[email protected]>
Cc: Michal Marek <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: James Morris <[email protected]>
Cc: "Serge E. Hallyn" <[email protected]>
Cc: Nathan Chancellor <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
Documentation/kbuild/gcc-plugins.rst | 26 ++++++++++++++++++++++++++
scripts/gcc-plugins/Kconfig | 4 ++--
security/Kconfig.hardening | 9 ++++++---
3 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/Documentation/kbuild/gcc-plugins.rst b/Documentation/kbuild/gcc-plugins.rst
index 3349966f213d..4b28c7a4032f 100644
--- a/Documentation/kbuild/gcc-plugins.rst
+++ b/Documentation/kbuild/gcc-plugins.rst
@@ -32,6 +32,32 @@ This infrastructure was ported from grsecurity [6]_ and PaX [7]_.
.. [7] https://pax.grsecurity.net/
+Purpose
+=======
+
+GCC plugins are designed to provide a place to experiment with potential
+compiler features that are neither in GCC nor Clang upstream. Once
+their utility is proven, the goal is to upstream the feature into GCC
+(and Clang), and then to finally remove them from the kernel once the
+feature is available in all supported versions of GCC.
+
+Specifically, new plugins should implement only features that have no
+upstream compiler support (in either GCC or Clang).
+
+When a feature exists in Clang but not GCC, effort should be made to
+bring the feature to upstream GCC (rather than just as a kernel-specific
+GCC plugin), so the entire ecosystem can benefit from it.
+
+Similarly, even if a feature provided by a GCC plugin does *not* exist
+in Clang, but the feature is proven to be useful, effort should be spent
+to upstream the feature to GCC (and Clang).
+
+After a feature is available in upstream GCC, the plugin will be made
+unbuildable for the corresponding GCC version (and later). Once all
+kernel-supported versions of GCC provide the feature, the plugin will
+be removed from the kernel.
+
+
Files
=====
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index ab9eb4cbe33a..3f5d3580ec06 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -37,6 +37,8 @@ config GCC_PLUGIN_CYC_COMPLEXITY
config GCC_PLUGIN_SANCOV
bool
+ # Plugin can be removed once the kernel only supports GCC 6.1.0+
+ depends on !CC_HAS_SANCOV_TRACE_PC
help
This plugin inserts a __sanitizer_cov_trace_pc() call at the start of
basic blocks. It supports all gcc versions with plugin support (from
@@ -83,8 +85,6 @@ config GCC_PLUGIN_RANDSTRUCT
the existing seed and will be removed by a make mrproper or
make distclean.
- Note that the implementation requires gcc 4.7 or newer.
-
This plugin was ported from grsecurity/PaX. More information at:
* https://grsecurity.net/
* https://pax.grsecurity.net/
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 90cbaff86e13..d30c6225de74 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -53,7 +53,8 @@ choice
config GCC_PLUGIN_STRUCTLEAK_USER
bool "zero-init structs marked for userspace (weak)"
- depends on GCC_PLUGINS
+ # Plugin can be removed once the kernel only supports GCC 12+
+ depends on GCC_PLUGINS && !CC_HAS_AUTO_VAR_INIT_ZERO
select GCC_PLUGIN_STRUCTLEAK
help
Zero-initialize any structures on the stack containing
@@ -64,7 +65,8 @@ choice
config GCC_PLUGIN_STRUCTLEAK_BYREF
bool "zero-init structs passed by reference (strong)"
- depends on GCC_PLUGINS
+ # Plugin can be removed once the kernel only supports GCC 12+
+ depends on GCC_PLUGINS && !CC_HAS_AUTO_VAR_INIT_ZERO
depends on !(KASAN && KASAN_STACK)
select GCC_PLUGIN_STRUCTLEAK
help
@@ -82,7 +84,8 @@ choice
config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
bool "zero-init everything passed by reference (very strong)"
- depends on GCC_PLUGINS
+ # Plugin can be removed once the kernel only supports GCC 12+
+ depends on GCC_PLUGINS && !CC_HAS_AUTO_VAR_INIT_ZERO
depends on !(KASAN && KASAN_STACK)
select GCC_PLUGIN_STRUCTLEAK
help
--
2.30.2
On Wed, Oct 20, 2021 at 7:35 PM Kees Cook <[email protected]> wrote:
>
> +Purpose
> +=======
Sounds good to me.
> config GCC_PLUGIN_SANCOV
> bool
> + # Plugin can be removed once the kernel only supports GCC 6.1.0+
Since we are just giving the major in the other cases below, I would
just say GCC 6+ here (the numbering scheme changed in GCC 5 already).
Thanks for adding the versions, by the way -- this is useful long-term
and not always done for other things...
Reviewed-by: Miguel Ojeda <[email protected]>
Cheers,
Miguel
On Wed, Oct 20, 2021 at 10:35:53AM -0700, Kees Cook wrote:
> GCC plugins should only exist when some compiler feature needs to be
> proven but does not exist in either GCC nor Clang. For example, if a
> desired feature is already in Clang, it should be added to GCC upstream.
> Document this explicitly.
>
> Additionally, mark the plugins with matching upstream GCC features as
> removable past their respective GCC versions.
>
> Cc: Masahiro Yamada <[email protected]>
> Cc: Michal Marek <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Jonathan Corbet <[email protected]>
> Cc: James Morris <[email protected]>
> Cc: "Serge E. Hallyn" <[email protected]>
> Cc: Nathan Chancellor <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Kees Cook <[email protected]>
Seems reasonable to me.
Reviewed-by: Nathan Chancellor <[email protected]>
One comment below.
> ---
> Documentation/kbuild/gcc-plugins.rst | 26 ++++++++++++++++++++++++++
> scripts/gcc-plugins/Kconfig | 4 ++--
> security/Kconfig.hardening | 9 ++++++---
> 3 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/kbuild/gcc-plugins.rst b/Documentation/kbuild/gcc-plugins.rst
> index 3349966f213d..4b28c7a4032f 100644
> --- a/Documentation/kbuild/gcc-plugins.rst
> +++ b/Documentation/kbuild/gcc-plugins.rst
> @@ -32,6 +32,32 @@ This infrastructure was ported from grsecurity [6]_ and PaX [7]_.
> .. [7] https://pax.grsecurity.net/
>
>
> +Purpose
> +=======
> +
> +GCC plugins are designed to provide a place to experiment with potential
> +compiler features that are neither in GCC nor Clang upstream. Once
> +their utility is proven, the goal is to upstream the feature into GCC
> +(and Clang), and then to finally remove them from the kernel once the
> +feature is available in all supported versions of GCC.
> +
> +Specifically, new plugins should implement only features that have no
> +upstream compiler support (in either GCC or Clang).
> +
> +When a feature exists in Clang but not GCC, effort should be made to
> +bring the feature to upstream GCC (rather than just as a kernel-specific
> +GCC plugin), so the entire ecosystem can benefit from it.
> +
> +Similarly, even if a feature provided by a GCC plugin does *not* exist
> +in Clang, but the feature is proven to be useful, effort should be spent
> +to upstream the feature to GCC (and Clang).
> +
> +After a feature is available in upstream GCC, the plugin will be made
> +unbuildable for the corresponding GCC version (and later). Once all
> +kernel-supported versions of GCC provide the feature, the plugin will
> +be removed from the kernel.
> +
> +
> Files
> =====
>
> diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> index ab9eb4cbe33a..3f5d3580ec06 100644
> --- a/scripts/gcc-plugins/Kconfig
> +++ b/scripts/gcc-plugins/Kconfig
> @@ -37,6 +37,8 @@ config GCC_PLUGIN_CYC_COMPLEXITY
>
> config GCC_PLUGIN_SANCOV
> bool
> + # Plugin can be removed once the kernel only supports GCC 6.1.0+
> + depends on !CC_HAS_SANCOV_TRACE_PC
This symbol is not user selectable and the one place that does select it
only does so when !CC_HAS_SANCOV_TRACE_PC so this seems pointless to me.
Keep the comment, ditch the depends?
> help
> This plugin inserts a __sanitizer_cov_trace_pc() call at the start of
> basic blocks. It supports all gcc versions with plugin support (from
> @@ -83,8 +85,6 @@ config GCC_PLUGIN_RANDSTRUCT
> the existing seed and will be removed by a make mrproper or
> make distclean.
>
> - Note that the implementation requires gcc 4.7 or newer.
> -
> This plugin was ported from grsecurity/PaX. More information at:
> * https://grsecurity.net/
> * https://pax.grsecurity.net/
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index 90cbaff86e13..d30c6225de74 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -53,7 +53,8 @@ choice
>
> config GCC_PLUGIN_STRUCTLEAK_USER
> bool "zero-init structs marked for userspace (weak)"
> - depends on GCC_PLUGINS
> + # Plugin can be removed once the kernel only supports GCC 12+
> + depends on GCC_PLUGINS && !CC_HAS_AUTO_VAR_INIT_ZERO
> select GCC_PLUGIN_STRUCTLEAK
> help
> Zero-initialize any structures on the stack containing
> @@ -64,7 +65,8 @@ choice
>
> config GCC_PLUGIN_STRUCTLEAK_BYREF
> bool "zero-init structs passed by reference (strong)"
> - depends on GCC_PLUGINS
> + # Plugin can be removed once the kernel only supports GCC 12+
> + depends on GCC_PLUGINS && !CC_HAS_AUTO_VAR_INIT_ZERO
> depends on !(KASAN && KASAN_STACK)
> select GCC_PLUGIN_STRUCTLEAK
> help
> @@ -82,7 +84,8 @@ choice
>
> config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
> bool "zero-init everything passed by reference (very strong)"
> - depends on GCC_PLUGINS
> + # Plugin can be removed once the kernel only supports GCC 12+
> + depends on GCC_PLUGINS && !CC_HAS_AUTO_VAR_INIT_ZERO
> depends on !(KASAN && KASAN_STACK)
> select GCC_PLUGIN_STRUCTLEAK
> help
> --
> 2.30.2
>
On Wed, Oct 20, 2021 at 07:44:19PM +0200, Miguel Ojeda wrote:
> On Wed, Oct 20, 2021 at 7:35 PM Kees Cook <[email protected]> wrote:
> >
> > +Purpose
> > +=======
>
> Sounds good to me.
>
> > config GCC_PLUGIN_SANCOV
> > bool
> > + # Plugin can be removed once the kernel only supports GCC 6.1.0+
>
> Since we are just giving the major in the other cases below, I would
> just say GCC 6+ here (the numbering scheme changed in GCC 5 already).
Sure; now updated.
> Thanks for adding the versions, by the way -- this is useful long-term
> and not always done for other things...
Yeah, I always struggled to find when options were added to GCC, so I
wanted this for my poor brain too. :)
> Reviewed-by: Miguel Ojeda <[email protected]>
Thanks!
--
Kees Cook
On Wed, Oct 20, 2021 at 10:45:43AM -0700, Nathan Chancellor wrote:
> On Wed, Oct 20, 2021 at 10:35:53AM -0700, Kees Cook wrote:
> > GCC plugins should only exist when some compiler feature needs to be
> > proven but does not exist in either GCC nor Clang. For example, if a
> > desired feature is already in Clang, it should be added to GCC upstream.
> > Document this explicitly.
> >
> > Additionally, mark the plugins with matching upstream GCC features as
> > removable past their respective GCC versions.
> >
> > Cc: Masahiro Yamada <[email protected]>
> > Cc: Michal Marek <[email protected]>
> > Cc: Nick Desaulniers <[email protected]>
> > Cc: Jonathan Corbet <[email protected]>
> > Cc: James Morris <[email protected]>
> > Cc: "Serge E. Hallyn" <[email protected]>
> > Cc: Nathan Chancellor <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Kees Cook <[email protected]>
>
> Seems reasonable to me.
>
> Reviewed-by: Nathan Chancellor <[email protected]>
Thanks!
>
> One comment below.
>
> > ---
> > Documentation/kbuild/gcc-plugins.rst | 26 ++++++++++++++++++++++++++
> > scripts/gcc-plugins/Kconfig | 4 ++--
> > security/Kconfig.hardening | 9 ++++++---
> > 3 files changed, 34 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/kbuild/gcc-plugins.rst b/Documentation/kbuild/gcc-plugins.rst
> > index 3349966f213d..4b28c7a4032f 100644
> > --- a/Documentation/kbuild/gcc-plugins.rst
> > +++ b/Documentation/kbuild/gcc-plugins.rst
> > @@ -32,6 +32,32 @@ This infrastructure was ported from grsecurity [6]_ and PaX [7]_.
> > .. [7] https://pax.grsecurity.net/
> >
> >
> > +Purpose
> > +=======
> > +
> > +GCC plugins are designed to provide a place to experiment with potential
> > +compiler features that are neither in GCC nor Clang upstream. Once
> > +their utility is proven, the goal is to upstream the feature into GCC
> > +(and Clang), and then to finally remove them from the kernel once the
> > +feature is available in all supported versions of GCC.
> > +
> > +Specifically, new plugins should implement only features that have no
> > +upstream compiler support (in either GCC or Clang).
> > +
> > +When a feature exists in Clang but not GCC, effort should be made to
> > +bring the feature to upstream GCC (rather than just as a kernel-specific
> > +GCC plugin), so the entire ecosystem can benefit from it.
> > +
> > +Similarly, even if a feature provided by a GCC plugin does *not* exist
> > +in Clang, but the feature is proven to be useful, effort should be spent
> > +to upstream the feature to GCC (and Clang).
> > +
> > +After a feature is available in upstream GCC, the plugin will be made
> > +unbuildable for the corresponding GCC version (and later). Once all
> > +kernel-supported versions of GCC provide the feature, the plugin will
> > +be removed from the kernel.
> > +
> > +
> > Files
> > =====
> >
> > diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> > index ab9eb4cbe33a..3f5d3580ec06 100644
> > --- a/scripts/gcc-plugins/Kconfig
> > +++ b/scripts/gcc-plugins/Kconfig
> > @@ -37,6 +37,8 @@ config GCC_PLUGIN_CYC_COMPLEXITY
> >
> > config GCC_PLUGIN_SANCOV
> > bool
> > + # Plugin can be removed once the kernel only supports GCC 6.1.0+
> > + depends on !CC_HAS_SANCOV_TRACE_PC
>
> This symbol is not user selectable and the one place that does select it
> only does so when !CC_HAS_SANCOV_TRACE_PC so this seems pointless to me.
>
> Keep the comment, ditch the depends?
I had a similar thought, and in the end, I decided I wanted to always
enforce the GCC feature check through a depends, with a comment about
the expected version. I want to make sure we don't use plugins if an
upstream feature is already available. It happens that SANCOV was
effectively the first to do this, but it did so on the other side and I
wanted it repeated here so it was "self contained".
-Kees
--
Kees Cook