Under certain conditions, the 64-bit '-mstack-protector-guard' flags may end up in the 32-bit vDSO flags, resulting in build failures due to the structure of clang's argument parsing of the stack protector options, which validates the arguments of the stack protector guard flags unconditionally in the frontend, choking on the 64-bit values when targeting 32-bit:
clang: error: invalid value 'r13' in 'mstack-protector-guard-reg=', expected one of: r2 clang: error: invalid value 'r13' in 'mstack-protector-guard-reg=', expected one of: r2 make[3]: *** [arch/powerpc/kernel/vdso/Makefile:85: arch/powerpc/kernel/vdso/vgettimeofday-32.o] Error 1 make[3]: *** [arch/powerpc/kernel/vdso/Makefile:87: arch/powerpc/kernel/vdso/vgetrandom-32.o] Error 1
Remove these flags by adding them to the CC32FLAGSREMOVE variable, which already handles situations similar to this. Additionally, reformat and align a comment better for the expanding CONFIG_CC_IS_CLANG block.
Cc: stable@vger.kernel.org # v6.1+ Signed-off-by: Nathan Chancellor nathan@kernel.org --- I say "Under certain conditions" because I am not entirely sure what they are. I cannot reproduce this error in my host environment but I can reproduce it in TuxMake's environment, which our CI uses:
https://storage.tuxsuite.com/public/clangbuiltlinux/continuous-integration2/...
$ tuxmake \ -a powerpc \ -k ppc64_guest_defconfig \ -r podman \ -t clang-nightly \ LLVM=1 \ config default ... clang: error: invalid value 'r13' in 'mstack-protector-guard-reg=', expected one of: r2 clang: error: invalid value 'r13' in 'mstack-protector-guard-reg=', expected one of: r2
I suspect that make 4.4 could play a difference here but the solution is quite simple here (since it is already weird with reusing flags) so I figured it was just worth doing this regardless of what the underlying reason is. --- arch/powerpc/kernel/vdso/Makefile | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/vdso/Makefile b/arch/powerpc/kernel/vdso/Makefile index 31ca5a5470047e7ac0a0f8194fd59c6a3b453b4d..c568cad6a22e6b8a8bcb04463b7c850306364804 100644 --- a/arch/powerpc/kernel/vdso/Makefile +++ b/arch/powerpc/kernel/vdso/Makefile @@ -54,10 +54,14 @@ ldflags-y += $(filter-out $(CC_AUTO_VAR_INIT_ZERO_ENABLER) $(CC_FLAGS_FTRACE) -W
CC32FLAGS := -m32 CC32FLAGSREMOVE := -mcmodel=medium -mabi=elfv1 -mabi=elfv2 -mcall-aixdesc - # This flag is supported by clang for 64-bit but not 32-bit so it will cause - # an unused command line flag warning for this file. ifdef CONFIG_CC_IS_CLANG +# This flag is supported by clang for 64-bit but not 32-bit so it will cause +# an unused command line flag warning for this file. CC32FLAGSREMOVE += -fno-stack-clash-protection +# -mstack-protector-guard values from the 64-bit build are not valid for the +# 32-bit one. clang validates the values passed to these arguments during +# parsing, even when -fno-stack-protector is passed afterwards. +CC32FLAGSREMOVE += -mstack-protector-guard% endif LD32FLAGS := -Wl,-soname=linux-vdso32.so.1 AS32FLAGS := -D__VDSO32__
--- base-commit: bee08a9e6ab03caf14481d97b35a258400ffab8f change-id: 20241030-powerpc-vdso-drop-stackp-flags-clang-ddfbf2ef27a6
Best regards,
Le 30/10/2024 à 19:41, Nathan Chancellor a écrit :
Under certain conditions, the 64-bit '-mstack-protector-guard' flags may end up in the 32-bit vDSO flags, resulting in build failures due to the structure of clang's argument parsing of the stack protector options, which validates the arguments of the stack protector guard flags unconditionally in the frontend, choking on the 64-bit values when targeting 32-bit:
clang: error: invalid value 'r13' in 'mstack-protector-guard-reg=', expected one of: r2 clang: error: invalid value 'r13' in 'mstack-protector-guard-reg=', expected one of: r2 make[3]: *** [arch/powerpc/kernel/vdso/Makefile:85: arch/powerpc/kernel/vdso/vgettimeofday-32.o] Error 1 make[3]: *** [arch/powerpc/kernel/vdso/Makefile:87: arch/powerpc/kernel/vdso/vgetrandom-32.o] Error 1
Remove these flags by adding them to the CC32FLAGSREMOVE variable, which already handles situations similar to this. Additionally, reformat and align a comment better for the expanding CONFIG_CC_IS_CLANG block.
Is the problem really exclusively for 32-bit VDSO on 64-bit kernel ? In any case, it is just wrong to have anything related to stack protection in VDSO, for this reason we have the following in Makefile:
ccflags-y += $(call cc-option, -fno-stack-protector)
If it is not enough, should we have more complete ?
Christophe
Hi!
On Wed, Nov 06, 2024 at 09:55:58AM +0100, Christophe Leroy wrote:
Le 30/10/2024 à 19:41, Nathan Chancellor a écrit :
Under certain conditions, the 64-bit '-mstack-protector-guard' flags may end up in the 32-bit vDSO flags, resulting in build failures due to the structure of clang's argument parsing of the stack protector options, which validates the arguments of the stack protector guard flags unconditionally in the frontend, choking on the 64-bit values when targeting 32-bit:
clang: error: invalid value 'r13' in 'mstack-protector-guard-reg=', expected one of: r2 clang: error: invalid value 'r13' in 'mstack-protector-guard-reg=', expected one of: r2 make[3]: *** [arch/powerpc/kernel/vdso/Makefile:85: arch/powerpc/kernel/vdso/vgettimeofday-32.o] Error 1 make[3]: *** [arch/powerpc/kernel/vdso/Makefile:87: arch/powerpc/kernel/vdso/vgetrandom-32.o] Error 1
Remove these flags by adding them to the CC32FLAGSREMOVE variable, which already handles situations similar to this. Additionally, reformat and align a comment better for the expanding CONFIG_CC_IS_CLANG block.
Is the problem really exclusively for 32-bit VDSO on 64-bit kernel ? In any case, it is just wrong to have anything related to stack protection in VDSO, for this reason we have the following in Makefile:
ccflags-y += $(call cc-option, -fno-stack-protector)
If it is not enough, should we have more complete ?
The -mstack-protector-guard-reg= doesn't do anything if you aren't doing stack protection. It allows any base register (so, r1..r31). Setting it to any valid reg should be fine and not do anything harmful, unless perhaps you *do* enable stack protector, then it better be the expected stuff ;-)
Apparently clang does not implement it correctly? This is just a clang bug, please report it with them?
(r2 is the default for -m32, r13 is the default for -m64, it appears that clang does not implement this option at all, it simply checks if you set the default, and explodes if not).
Segher
Hi Christophe and Segher,
On Wed, Nov 06, 2024 at 07:37:52AM -0600, Segher Boessenkool wrote:
On Wed, Nov 06, 2024 at 09:55:58AM +0100, Christophe Leroy wrote:
Le 30/10/2024 à 19:41, Nathan Chancellor a écrit :
Under certain conditions, the 64-bit '-mstack-protector-guard' flags may end up in the 32-bit vDSO flags, resulting in build failures due to the structure of clang's argument parsing of the stack protector options, which validates the arguments of the stack protector guard flags unconditionally in the frontend, choking on the 64-bit values when targeting 32-bit:
clang: error: invalid value 'r13' in 'mstack-protector-guard-reg=', expected one of: r2 clang: error: invalid value 'r13' in 'mstack-protector-guard-reg=', expected one of: r2 make[3]: *** [arch/powerpc/kernel/vdso/Makefile:85: arch/powerpc/kernel/vdso/vgettimeofday-32.o] Error 1 make[3]: *** [arch/powerpc/kernel/vdso/Makefile:87: arch/powerpc/kernel/vdso/vgetrandom-32.o] Error 1
Remove these flags by adding them to the CC32FLAGSREMOVE variable, which already handles situations similar to this. Additionally, reformat and align a comment better for the expanding CONFIG_CC_IS_CLANG block.
Is the problem really exclusively for 32-bit VDSO on 64-bit kernel ?
As far as I can tell, yes, as I do not think there are any other places where flags for targeting one word size were being used when targeting the other word size.
In any case, it is just wrong to have anything related to stack protection in VDSO, for this reason we have the following in Makefile:
ccflags-y += $(call cc-option, -fno-stack-protector)
If it is not enough, should we have more complete ?
That should be enough to disable the stack protector from my understanding. It is just that clang's argument validation happens even with -fno-stack-protector, so the flags need to contain valid values for the target. This is true for GCC as well, it just supports any base register like Segher mentions below so it does not hit any issue here:
$ powerpc64-linux-gcc -fno-stack-protector -mstack-protector-guard=tls -mstack-protector-guard-reg=r50 -c -o /dev/null -x c /dev/null cc1: error: 'r50' is not a valid base register in '-mstack-protector-guard-reg=' cc1: error: '-mstack-protector-guard=tls' needs a valid base register
The -mstack-protector-guard-reg= doesn't do anything if you aren't doing stack protection. It allows any base register (so, r1..r31). Setting it to any valid reg should be fine and not do anything harmful, unless perhaps you *do* enable stack protector, then it better be the expected stuff ;-)
Apparently clang does not implement it correctly? This is just a clang bug, please report it with them?
(r2 is the default for -m32, r13 is the default for -m64, it appears that clang does not implement this option at all, it simply checks if you set the default, and explodes if not).
Not sure that I would say it has not been implemented correctly, more that it has not been implemented in the same manner as GCC. Keith chose not to open up support for arbitrary registers to keep the implementation of this option in LLVM simple:
https://lore.kernel.org/linuxppc-dev/87o73uvaq5.fsf@keithp.com/
Cheers, Nathan
Hi!
On Wed, Nov 06, 2024 at 08:21:14AM -0700, Nathan Chancellor wrote:
(r2 is the default for -m32, r13 is the default for -m64, it appears that clang does not implement this option at all, it simply checks if you set the default, and explodes if not).
Not sure that I would say it has not been implemented correctly, more that it has not been implemented in the same manner as GCC. Keith chose not to open up support for arbitrary registers to keep the implementation of this option in LLVM simple:
LLVM claims to be compatible to GCC. It is not. This is a bug. As it is, LLVM can not be used to compile the PowerPC kernel.
These flags (-mstack-protector-guard-{reg,offset}=) are there *specifically* so that the user can choose to use something different from the default. I added this (back in 2017) because the kernel needed it. Some other GCC ports (aarch64, arm, riscv, x86) have followed suit since then, btw.
Segher
On Wed, 30 Oct 2024 11:41:37 -0700, Nathan Chancellor wrote:
Under certain conditions, the 64-bit '-mstack-protector-guard' flags may end up in the 32-bit vDSO flags, resulting in build failures due to the structure of clang's argument parsing of the stack protector options, which validates the arguments of the stack protector guard flags unconditionally in the frontend, choking on the 64-bit values when targeting 32-bit:
[...]
Applied to powerpc/next.
[1/1] powerpc/vdso: Drop -mstack-protector-guard flags in 32-bit files with clang https://git.kernel.org/powerpc/c/d677ce521334d8f1f327cafc8b1b7854b0833158
cheers
linux-stable-mirror@lists.linaro.org
-
Christophe Leroy -
Michael Ellerman -
Nathan Chancellor -
Segher Boessenkool