arm: fix Kbuild issue caused by per-task stack protector GCC plugin

When using plugins, GCC requires that the -fplugin= options precedes any of its plugin arguments appearing on the command line as well. This is usually not a concern, but as it turns out, this requirement is causing some issues with ARM's per-task stack protector plugin and Kbuild's implementation of $(cc-option).

When the per-task stack protector plugin is enabled, and we tweak the implementation of cc-option not to pipe the stderr output of GCC to /dev/null, the following output is generated when GCC is executed in the context of cc-option:

  cc1: error: plugin arm_ssp_per_task_plugin should be specified before \
         -fplugin-arg-arm_ssp_per_task_plugin-tso=1 in the command line
  cc1: error: plugin arm_ssp_per_task_plugin should be specified before \
         -fplugin-arg-arm_ssp_per_task_plugin-offset=24 in the command line

These errors will cause any option passed to cc-option to be treated as unsupported, which is obviously incorrect.

The cause of this issue is the fact that the -fplugin= argument is added to GCC_PLUGINS_CFLAGS, whereas the arguments above are added to KBUILD_CFLAGS, and the contents of the former get filtered out of the latter before being passed to the GCC running the cc-option test, and so the -fplugin= option does not appear at all on the GCC command line.

Adding the arguments to GCC_PLUGINS_CFLAGS instead of KBUILD_CFLAGS would be the correct approach here, if it weren't for the fact that we are using $(eval) to defer the moment that they are added until after asm-offsets.h is generated, which is after the point where the contents of GCC_PLUGINS_CFLAGS are added to KBUILD_CFLAGS. So instead, we have to add our plugin arguments to both.

For similar reasons, we cannot append DISABLE_ARM_SSP_PER_TASK_PLUGIN to KBUILD_CFLAGS, as it will be passed to GCC when executing in the context of cc-option, whereas the other plugin arguments will have been filtered out, resulting in a similar error and false negative result as above. So add it to ccflags-y instead.

Reported-by Tony Lindgren <[email protected]>

Fixes 189af4657186da08 ("ARM: smp: add support for per-task stack canaries")

Cc Kees Cook <[email protected]>

Cc Russell King <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Masahiro Yamada <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm/Makefile                 | 4 +++-
 arch/arm/boot/compressed/Makefile | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

arch/arm/Makefile +3 -1

index db857d07114f..1fc32b611f8a 100644
@@ -307,13 +307,15 @@ endif
 ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y)
 prepare: stack_protector_prepare
 stack_protector_prepare: prepare0
-	$(eval KBUILD_CFLAGS += \
+	$(eval SSP_PLUGIN_CFLAGS := \
 		-fplugin-arg-arm_ssp_per_task_plugin-tso=$(shell	\
 			awk '{if ($$2 == "THREAD_SZ_ORDER") print $$3;}'\
 				include/generated/asm-offsets.h)	\
 		-fplugin-arg-arm_ssp_per_task_plugin-offset=$(shell	\
 			awk '{if ($$2 == "TI_STACK_CANARY") print $$3;}'\
 				include/generated/asm-offsets.h))
+	$(eval KBUILD_CFLAGS += $(SSP_PLUGIN_CFLAGS))
+	$(eval GCC_PLUGINS_CFLAGS += $(SSP_PLUGIN_CFLAGS))
 endif
 
 all:	$(notdir $(KBUILD_IMAGE))

arch/arm/boot/compressed/Makefile +2 -2

index da599c3a1193..9c11e7490292 100644
@@ -101,7 +101,6 @@ clean-files += piggy_data lib1funcs.S ashldi3.S bswapsdi2.S \
 		$(libfdt) $(libfdt_hdrs) hyp-stub.S
 
 KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
-KBUILD_CFLAGS += $(DISABLE_ARM_SSP_PER_TASK_PLUGIN)
 
 ifeq ($(CONFIG_FUNCTION_TRACER),y)
 ORIG_CFLAGS := $(KBUILD_CFLAGS)

@@ -117,7 +116,8 @@ CFLAGS_fdt_ro.o := $(nossp-flags-y)
 CFLAGS_fdt_rw.o := $(nossp-flags-y)
 CFLAGS_fdt_wip.o := $(nossp-flags-y)
 
-ccflags-y := -fpic $(call cc-option,-mno-single-pic-base,) -fno-builtin -I$(obj)
+ccflags-y := -fpic $(call cc-option,-mno-single-pic-base,) -fno-builtin \
+	     -I$(obj) $(DISABLE_ARM_SSP_PER_TASK_PLUGIN)
 asflags-y := -DZIMAGE
 
 # Supply kernel BSS size to the decompressor via a linker symbol.

Signature · 1 line

2.17.1

When using plugins, GCC requires that the -fplugin= options precedes any of its plugin arguments appearing on the command line as well. This is usually not a concern, but as it turns out, this requirement is causing some issues with ARM's per-task stack protector plugin and Kbuild's implementation of $(cc-option).

When the per-task stack protector plugin is enabled, and we tweak the implementation of cc-option not to pipe the stderr output of GCC to /dev/null, the following output is generated when GCC is executed in the context of cc-option:

  cc1: error: plugin arm_ssp_per_task_plugin should be specified before \
         -fplugin-arg-arm_ssp_per_task_plugin-tso=1 in the command line
  cc1: error: plugin arm_ssp_per_task_plugin should be specified before \
         -fplugin-arg-arm_ssp_per_task_plugin-offset=24 in the command line

These errors will cause any option passed to cc-option to be treated as unsupported, which is obviously incorrect.

The cause of this issue is the fact that the -fplugin= argument is added to GCC_PLUGINS_CFLAGS, whereas the arguments above are added to KBUILD_CFLAGS, and the contents of the former get filtered out of the latter before being passed to the GCC running the cc-option test, and so the -fplugin= option does not appear at all on the GCC command line.

Adding the arguments to GCC_PLUGINS_CFLAGS instead of KBUILD_CFLAGS would be the correct approach here, if it weren't for the fact that we are using $(eval) to defer the moment that they are added until after asm-offsets.h is generated, which is after the point where the contents of GCC_PLUGINS_CFLAGS are added to KBUILD_CFLAGS. So instead, we have to add our plugin arguments to both.

For similar reasons, we cannot append DISABLE_ARM_SSP_PER_TASK_PLUGIN to KBUILD_CFLAGS, as it will be passed to GCC when executing in the context of cc-option, whereas the other plugin arguments will have been filtered out, resulting in a similar error and false negative result as above. So add it to ccflags-y instead.

Reported-by Tony Lindgren <[email protected]>

Fixes 189af4657186da08 ("ARM: smp: add support for per-task stack canaries")

Cc Kees Cook <[email protected]>

Cc Russell King <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Masahiro Yamada <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

Quoted 39 lines · Ard Biesheuvel

When using plugins, GCC requires that the -fplugin= options precedes any of its plugin arguments appearing on the command line as well. This is usually not a concern, but as it turns out, this requirement is causing some issues with ARM's per-task stack protector plugin and Kbuild's implementation of $(cc-option).

When the per-task stack protector plugin is enabled, and we tweak the implementation of cc-option not to pipe the stderr output of GCC to /dev/null, the following output is generated when GCC is executed in the context of cc-option:

  cc1: error: plugin arm_ssp_per_task_plugin should be specified before \
         -fplugin-arg-arm_ssp_per_task_plugin-tso=1 in the command line
  cc1: error: plugin arm_ssp_per_task_plugin should be specified before \
         -fplugin-arg-arm_ssp_per_task_plugin-offset=24 in the command line

These errors will cause any option passed to cc-option to be treated as unsupported, which is obviously incorrect.

The cause of this issue is the fact that the -fplugin= argument is added to GCC_PLUGINS_CFLAGS, whereas the arguments above are added to KBUILD_CFLAGS, and the contents of the former get filtered out of the latter before being passed to the GCC running the cc-option test, and so the -fplugin= option does not appear at all on the GCC command line.

Adding the arguments to GCC_PLUGINS_CFLAGS instead of KBUILD_CFLAGS would be the correct approach here, if it weren't for the fact that we are using $(eval) to defer the moment that they are added until after asm-offsets.h is generated, which is after the point where the contents of GCC_PLUGINS_CFLAGS are added to KBUILD_CFLAGS. So instead, we have to add our plugin arguments to both.

For similar reasons, we cannot append DISABLE_ARM_SSP_PER_TASK_PLUGIN to KBUILD_CFLAGS, as it will be passed to GCC when executing in the context of cc-option, whereas the other plugin arguments will have been filtered out, resulting in a similar error and false negative result as above. So add it to ccflags-y instead.

Quoted 7 lines

Reported-by Tony Lindgren <[email protected]>

Fixes 189af4657186da08 ("ARM: smp: add support for per-task stack canaries")

Cc Kees Cook <[email protected]>

Cc Russell King <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Masahiro Yamada <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

Oh! Nice find.

Acked-by Kees Cook <[email protected]>

I assume this should just go directly into the arm patch tracker, or should it go via Masahiro's tree?