Re: [PATCH] arm: fix Kbuild issue caused by per-task stack protector GCC plugin

[Kees Cook <keescook@chromium.org>]

On Mon, Feb 17, 2020 at 09:30:13PM +0100, Ard Biesheuvel wrote:
> 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 <tony@atomide.com>
> Fixes: 189af4657186da08 ("ARM: smp: add support for per-task stack canaries")
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Oh! Nice find.

Acked-by: Kees Cook <keescook@chromium.org>

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

-Kees

-- 
Kees Cook

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel