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 <masahiroy@xxxxxxxxxx> > Cc: Michal Marek <michal.lkml@xxxxxxxxxxx> > Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > Cc: Jonathan Corbet <corbet@xxxxxxx> > Cc: James Morris <jmorris@xxxxxxxxx> > Cc: "Serge E. Hallyn" <serge@xxxxxxxxxx> > Cc: Nathan Chancellor <nathan@xxxxxxxxxx> > Cc: linux-hardening@xxxxxxxxxxxxxxx > Cc: linux-kbuild@xxxxxxxxxxxxxxx > Cc: linux-doc@xxxxxxxxxxxxxxx > Cc: linux-security-module@xxxxxxxxxxxxxxx > Cc: llvm@xxxxxxxxxxxxxxx > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> Seems reasonable to me. Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx> 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 >