[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK7LNATs4uHnNHgESXcUEjpONZra=GvkuHMaDwsx0hbyUGY99w@mail.gmail.com>
Date: Sat, 3 May 2025 15:12:23 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Kees Cook <kees@...nel.org>
Cc: Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas.schier@...ux.dev>,
linux-hardening@...r.kernel.org, linux-kbuild@...r.kernel.org,
Petr Pavlu <petr.pavlu@...e.com>, Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Justin Stitt <justinstitt@...gle.com>, Marco Elver <elver@...gle.com>,
Andrey Konovalov <andreyknvl@...il.com>, Andrey Ryabinin <ryabinin.a.a@...il.com>,
Richard Weinberger <richard@....at>, Anton Ivanov <anton.ivanov@...bridgegreys.com>,
Johannes Berg <johannes@...solutions.net>, linux-kernel@...r.kernel.org,
kasan-dev@...glegroups.com, linux-um@...ts.infradead.org
Subject: Re: [PATCH v2 1/3] gcc-plugins: Force full rebuild when plugins change
On Sat, May 3, 2025 at 7:54 AM Kees Cook <kees@...nel.org> wrote:
>
> There was no dependency between the plugins changing and the rest of the
> kernel being built. Enforce this by including a synthetic header file
> when using plugins, that is regenerated any time the plugins are built.
>
> This cannot be included via '-include ...' because Makefiles use the
> "filter-out" string function, which removes individual words. Removing
> all instances of "-include" from the CFLAGS will cause a lot of
> problems. :)
>
> Instead, use -I to include the gcc-plugins directory, and depend on the
> new -DGCC_PLUGINS_ENABLED flag to include the generated header file via
> include/linux/compiler-version.h, which is already being used to control
> full rebuilds. The UM build requires that the -I be explicitly added.
>
> Signed-off-by: Kees Cook <kees@...nel.org>
> ---
> Cc: Masahiro Yamada <masahiroy@...nel.org>
> Cc: Nathan Chancellor <nathan@...nel.org>
> Cc: Nicolas Schier <nicolas.schier@...ux.dev>
> Cc: <linux-hardening@...r.kernel.org>
> Cc: <linux-kbuild@...r.kernel.org>
> ---
> arch/um/Makefile | 1 +
> include/linux/compiler-version.h | 4 ++++
> scripts/Makefile.gcc-plugins | 2 +-
> scripts/gcc-plugins/Makefile | 8 ++++++++
> 4 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/um/Makefile b/arch/um/Makefile
> index 1d36a613aad8..8cc0f22ebefa 100644
> --- a/arch/um/Makefile
> +++ b/arch/um/Makefile
> @@ -72,6 +72,7 @@ USER_CFLAGS = $(patsubst $(KERNEL_DEFINES),,$(patsubst -I%,,$(KBUILD_CFLAGS))) \
> $(ARCH_INCLUDE) $(MODE_INCLUDE) $(filter -I%,$(CFLAGS)) \
> -D_FILE_OFFSET_BITS=64 -idirafter $(srctree)/include \
> -idirafter $(objtree)/include -D__KERNEL__ -D__UM_HOST__ \
> + -I$(objtree)/scripts/gcc-plugins \
> -include $(srctree)/include/linux/compiler-version.h \
> -include $(srctree)/include/linux/kconfig.h
>
> diff --git a/include/linux/compiler-version.h b/include/linux/compiler-version.h
> index 573fa85b6c0c..08943df04ebb 100644
> --- a/include/linux/compiler-version.h
> +++ b/include/linux/compiler-version.h
> @@ -12,3 +12,7 @@
> * and add dependency on include/config/CC_VERSION_TEXT, which is touched
> * by Kconfig when the version string from the compiler changes.
> */
> +
> +#ifdef GCC_PLUGINS_ENABLED
> +#include "gcc-plugins-deps.h"
> +#endif
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index 5b8a8378ca8a..468bb8faa9d1 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -38,7 +38,7 @@ export DISABLE_STACKLEAK_PLUGIN
>
> # All the plugin CFLAGS are collected here in case a build target needs to
> # filter them out of the KBUILD_CFLAGS.
> -GCC_PLUGINS_CFLAGS := $(strip $(addprefix -fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y)) $(gcc-plugin-cflags-y))
> +GCC_PLUGINS_CFLAGS := $(strip $(addprefix -fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y)) $(gcc-plugin-cflags-y)) -I$(objtree)/scripts/gcc-plugins -DGCC_PLUGINS_ENABLED
This still relies on no-space after the -I option.
> export GCC_PLUGINS_CFLAGS
>
> # Add the flags to the build!
> diff --git a/scripts/gcc-plugins/Makefile b/scripts/gcc-plugins/Makefile
> index 320afd3cf8e8..24671d39ec90 100644
> --- a/scripts/gcc-plugins/Makefile
> +++ b/scripts/gcc-plugins/Makefile
> @@ -66,3 +66,11 @@ quiet_cmd_plugin_cxx_o_c = HOSTCXX $@
>
> $(plugin-objs): $(obj)/%.o: $(src)/%.c FORCE
> $(call if_changed_dep,plugin_cxx_o_c)
> +
> +quiet_cmd_gcc_plugins_updated = UPDATE $@
> + cmd_gcc_plugins_updated = echo '/* $^ */' > $(obj)/gcc-plugins-deps.h
I think 'touch' should be enough.
If some plugins are disabled, it is detected by the normal if_changed rule.
> +
> +$(obj)/gcc-plugins-deps.h: $(plugin-single) $(plugin-multi) FORCE
> + $(call if_changed,gcc_plugins_updated)
> +
> +always-y += gcc-plugins-deps.h
> --
> 2.34.1
>
I think it is simpler to place the header
in include/generated/.
I attached my suggestion below:
diff --git a/arch/um/Makefile b/arch/um/Makefile
index 8cc0f22ebefa..1d36a613aad8 100644
--- a/arch/um/Makefile
+++ b/arch/um/Makefile
@@ -72,7 +72,6 @@ USER_CFLAGS = $(patsubst
$(KERNEL_DEFINES),,$(patsubst -I%,,$(KBUILD_CFLAGS))) \
$(ARCH_INCLUDE) $(MODE_INCLUDE) $(filter -I%,$(CFLAGS)) \
-D_FILE_OFFSET_BITS=64 -idirafter $(srctree)/include \
-idirafter $(objtree)/include -D__KERNEL__ -D__UM_HOST__ \
- -I$(objtree)/scripts/gcc-plugins \
-include $(srctree)/include/linux/compiler-version.h \
-include $(srctree)/include/linux/kconfig.h
diff --git a/include/linux/compiler-version.h b/include/linux/compiler-version.h
index 08943df04ebb..ea3d533dc04a 100644
--- a/include/linux/compiler-version.h
+++ b/include/linux/compiler-version.h
@@ -14,5 +14,5 @@
*/
#ifdef GCC_PLUGINS_ENABLED
-#include "gcc-plugins-deps.h"
+#include <generated/gcc-plugins-deps.h>
#endif
diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 67b045a66157..f9b51c2c2158 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -44,7 +44,7 @@ export DISABLE_ARM_SSP_PER_TASK_PLUGIN
# All the plugin CFLAGS are collected here in case a build target needs to
# filter them out of the KBUILD_CFLAGS.
-GCC_PLUGINS_CFLAGS := $(strip $(addprefix
-fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y))
$(gcc-plugin-cflags-y)) -I$(objtree)/scripts/gcc-plugins
-DGCC_PLUGINS_ENABLED
+GCC_PLUGINS_CFLAGS := $(strip $(addprefix
-fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y))
$(gcc-plugin-cflags-y)) -DGCC_PLUGINS_ENABLED
export GCC_PLUGINS_CFLAGS
# Add the flags to the build!
diff --git a/scripts/gcc-plugins/Makefile b/scripts/gcc-plugins/Makefile
index 24671d39ec90..b354c0f9f66d 100644
--- a/scripts/gcc-plugins/Makefile
+++ b/scripts/gcc-plugins/Makefile
@@ -67,10 +67,10 @@ quiet_cmd_plugin_cxx_o_c = HOSTCXX $@
$(plugin-objs): $(obj)/%.o: $(src)/%.c FORCE
$(call if_changed_dep,plugin_cxx_o_c)
-quiet_cmd_gcc_plugins_updated = UPDATE $@
- cmd_gcc_plugins_updated = echo '/* $^ */' > $(obj)/gcc-plugins-deps.h
+quiet_cmd_gcc_plugins_updated = TOUCH $@
+ cmd_gcc_plugins_updated = touch $@
-$(obj)/gcc-plugins-deps.h: $(plugin-single) $(plugin-multi) FORCE
+$(obj)/../../include/generated/gcc-plugins-deps.h: $(plugin-single)
$(plugin-multi) FORCE
$(call if_changed,gcc_plugins_updated)
-always-y += gcc-plugins-deps.h
+always-y += ../../include/generated/gcc-plugins-deps.h
--
Best Regards
Masahiro Yamada
Powered by blists - more mailing lists