From: Ard Biesheuvel ardb@kernel.org
GCC and Clang both implement stack protector support based on Thread Local Storage (TLS) variables, and this is used in the kernel to implement per-task stack cookies, by copying a task's stack cookie into a per-CPU variable every time it is scheduled in.
Both now also implement -mstack-protector-guard-symbol=, which permits the TLS variable to be specified directly. This is useful because it will allow us to move away from using a fixed offset of 40 bytes into the per-CPU area on x86_64, which requires a lot of special handling in the per-CPU code and the runtime relocation code.
However, while GCC is rather lax in its implementation of this command line option, Clang actually requires that the provided symbol name refers to a TLS variable (i.e., one declared with __thread), although it also permits the variable to be undeclared entirely, in which case it will use an implicit declaration of the right type.
The upshot of this is that Clang will emit the correct references to the stack cookie variable in most cases, e.g.,
10d: 64 a1 00 00 00 00 mov %fs:0x0,%eax 10f: R_386_32 __stack_chk_guard
However, if a non-TLS definition of the symbol in question is visible in the same compilation unit (which amounts to the whole of vmlinux if LTO is enabled), it will drop the per-CPU prefix and emit a load from a bogus address.
Work around this by using a symbol name that never occurs in C code, and emit it as an alias in the linker script.
Fixes: 3fb0fdb3bbe7 ("x86/stackprotector/32: Make the canary into a regular percpu variable") Cc: stable@vger.kernel.org Cc: Fangrui Song i@maskray.me Cc: Uros Bizjak ubizjak@gmail.com Cc: Nathan Chancellor nathan@kernel.org Cc: Andy Lutomirski luto@kernel.org Link: https://github.com/ClangBuiltLinux/linux/issues/1854 Signed-off-by: Ard Biesheuvel ardb@kernel.org Signed-off-by: Brian Gerst brgerst@gmail.com --- arch/x86/Makefile | 5 +++-- arch/x86/entry/entry.S | 16 ++++++++++++++++ arch/x86/include/asm/asm-prototypes.h | 3 +++ arch/x86/kernel/cpu/common.c | 2 ++ arch/x86/kernel/vmlinux.lds.S | 3 +++ 5 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile index cd75e78a06c1..5b773b34768d 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -142,9 +142,10 @@ ifeq ($(CONFIG_X86_32),y)
ifeq ($(CONFIG_STACKPROTECTOR),y) ifeq ($(CONFIG_SMP),y) - KBUILD_CFLAGS += -mstack-protector-guard-reg=fs -mstack-protector-guard-symbol=__stack_chk_guard + KBUILD_CFLAGS += -mstack-protector-guard-reg=fs \ + -mstack-protector-guard-symbol=__ref_stack_chk_guard else - KBUILD_CFLAGS += -mstack-protector-guard=global + KBUILD_CFLAGS += -mstack-protector-guard=global endif endif else diff --git a/arch/x86/entry/entry.S b/arch/x86/entry/entry.S index 324686bca368..b7ea3e8e9ecc 100644 --- a/arch/x86/entry/entry.S +++ b/arch/x86/entry/entry.S @@ -51,3 +51,19 @@ EXPORT_SYMBOL_GPL(mds_verw_sel); .popsection
THUNK warn_thunk_thunk, __warn_thunk + +#ifndef CONFIG_X86_64 +/* + * Clang's implementation of TLS stack cookies requires the variable in + * question to be a TLS variable. If the variable happens to be defined as an + * ordinary variable with external linkage in the same compilation unit (which + * amounts to the whole of vmlinux with LTO enabled), Clang will drop the + * segment register prefix from the references, resulting in broken code. Work + * around this by avoiding the symbol used in -mstack-protector-guard-symbol= + * entirely in the C code, and use an alias emitted by the linker script + * instead. + */ +#ifdef CONFIG_STACKPROTECTOR +EXPORT_SYMBOL(__ref_stack_chk_guard); +#endif +#endif diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h index 25466c4d2134..3674006e3974 100644 --- a/arch/x86/include/asm/asm-prototypes.h +++ b/arch/x86/include/asm/asm-prototypes.h @@ -20,3 +20,6 @@ extern void cmpxchg8b_emu(void); #endif
+#if defined(__GENKSYMS__) && defined(CONFIG_STACKPROTECTOR) +extern unsigned long __ref_stack_chk_guard; +#endif diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 8f41ab219cf1..9d42bd15e06c 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -2091,8 +2091,10 @@ void syscall_init(void)
#ifdef CONFIG_STACKPROTECTOR DEFINE_PER_CPU(unsigned long, __stack_chk_guard); +#ifndef CONFIG_SMP EXPORT_PER_CPU_SYMBOL(__stack_chk_guard); #endif +#endif
#endif /* CONFIG_X86_64 */
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index 410546bacc0f..d61c3584f3e6 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -468,6 +468,9 @@ SECTIONS . = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE), "kernel image bigger than KERNEL_IMAGE_SIZE");
+/* needed for Clang - see arch/x86/entry/entry.S */ +PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); + #ifdef CONFIG_X86_64 /* * Per-cpu symbols which need to be offset from __per_cpu_load
On Tue, Nov 05, 2024 at 10:57:46AM -0500, Brian Gerst wrote:
From: Ard Biesheuvel ardb@kernel.org
GCC and Clang both implement stack protector support based on Thread Local Storage (TLS) variables, and this is used in the kernel to implement per-task stack cookies, by copying a task's stack cookie into a per-CPU variable every time it is scheduled in.
Both now also implement -mstack-protector-guard-symbol=, which permits the TLS variable to be specified directly. This is useful because it will allow us to move away from using a fixed offset of 40 bytes into the per-CPU area on x86_64, which requires a lot of special handling in the per-CPU code and the runtime relocation code.
However, while GCC is rather lax in its implementation of this command line option, Clang actually requires that the provided symbol name refers to a TLS variable (i.e., one declared with __thread), although it also permits the variable to be undeclared entirely, in which case it will use an implicit declaration of the right type.
The upshot of this is that Clang will emit the correct references to the stack cookie variable in most cases, e.g.,
10d: 64 a1 00 00 00 00 mov %fs:0x0,%eax 10f: R_386_32 __stack_chk_guard
However, if a non-TLS definition of the symbol in question is visible in the same compilation unit (which amounts to the whole of vmlinux if LTO is enabled), it will drop the per-CPU prefix and emit a load from a bogus address.
Work around this by using a symbol name that never occurs in C code, and emit it as an alias in the linker script.
Fixes: 3fb0fdb3bbe7 ("x86/stackprotector/32: Make the canary into a regular percpu variable") Cc: stable@vger.kernel.org Cc: Fangrui Song i@maskray.me Cc: Uros Bizjak ubizjak@gmail.com Cc: Nathan Chancellor nathan@kernel.org Cc: Andy Lutomirski luto@kernel.org Link: https://github.com/ClangBuiltLinux/linux/issues/1854 Signed-off-by: Ard Biesheuvel ardb@kernel.org Signed-off-by: Brian Gerst brgerst@gmail.com
From https://lore.kernel.org/20241016021045.GA1000009@thelio-3990X/:
Reviewed-by: Nathan Chancellor nathan@kernel.org Tested-by: Nathan Chancellor nathan@kernel.org
arch/x86/Makefile | 5 +++-- arch/x86/entry/entry.S | 16 ++++++++++++++++ arch/x86/include/asm/asm-prototypes.h | 3 +++ arch/x86/kernel/cpu/common.c | 2 ++ arch/x86/kernel/vmlinux.lds.S | 3 +++ 5 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile index cd75e78a06c1..5b773b34768d 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -142,9 +142,10 @@ ifeq ($(CONFIG_X86_32),y) ifeq ($(CONFIG_STACKPROTECTOR),y) ifeq ($(CONFIG_SMP),y)
KBUILD_CFLAGS += -mstack-protector-guard-reg=fs -mstack-protector-guard-symbol=__stack_chk_guard
KBUILD_CFLAGS += -mstack-protector-guard-reg=fs \-mstack-protector-guard-symbol=__ref_stack_chk_guard else
KBUILD_CFLAGS += -mstack-protector-guard=global
KBUILD_CFLAGS += -mstack-protector-guard=global endifelse diff --git a/arch/x86/entry/entry.S b/arch/x86/entry/entry.S index 324686bca368..b7ea3e8e9ecc 100644 --- a/arch/x86/entry/entry.S +++ b/arch/x86/entry/entry.S @@ -51,3 +51,19 @@ EXPORT_SYMBOL_GPL(mds_verw_sel); .popsection THUNK warn_thunk_thunk, __warn_thunk
+#ifndef CONFIG_X86_64 +/*
- Clang's implementation of TLS stack cookies requires the variable in
- question to be a TLS variable. If the variable happens to be defined as an
- ordinary variable with external linkage in the same compilation unit (which
- amounts to the whole of vmlinux with LTO enabled), Clang will drop the
- segment register prefix from the references, resulting in broken code. Work
- around this by avoiding the symbol used in -mstack-protector-guard-symbol=
- entirely in the C code, and use an alias emitted by the linker script
- instead.
- */
+#ifdef CONFIG_STACKPROTECTOR +EXPORT_SYMBOL(__ref_stack_chk_guard); +#endif +#endif diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h index 25466c4d2134..3674006e3974 100644 --- a/arch/x86/include/asm/asm-prototypes.h +++ b/arch/x86/include/asm/asm-prototypes.h @@ -20,3 +20,6 @@ extern void cmpxchg8b_emu(void); #endif +#if defined(__GENKSYMS__) && defined(CONFIG_STACKPROTECTOR) +extern unsigned long __ref_stack_chk_guard; +#endif diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 8f41ab219cf1..9d42bd15e06c 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -2091,8 +2091,10 @@ void syscall_init(void) #ifdef CONFIG_STACKPROTECTOR DEFINE_PER_CPU(unsigned long, __stack_chk_guard); +#ifndef CONFIG_SMP EXPORT_PER_CPU_SYMBOL(__stack_chk_guard); #endif +#endif #endif /* CONFIG_X86_64 */ diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index 410546bacc0f..d61c3584f3e6 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -468,6 +468,9 @@ SECTIONS . = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE), "kernel image bigger than KERNEL_IMAGE_SIZE"); +/* needed for Clang - see arch/x86/entry/entry.S */ +PROVIDE(__ref_stack_chk_guard = __stack_chk_guard);
#ifdef CONFIG_X86_64 /*
- Per-cpu symbols which need to be offset from __per_cpu_load
-- 2.47.0
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 577c134d311b9b94598d7a0c86be1f431f823003 Gitweb: https://git.kernel.org/tip/577c134d311b9b94598d7a0c86be1f431f823003 Author: Ard Biesheuvel ardb@kernel.org AuthorDate: Tue, 05 Nov 2024 10:57:46 -05:00 Committer: Borislav Petkov (AMD) bp@alien8.de CommitterDate: Fri, 08 Nov 2024 13:16:00 +01:00
x86/stackprotector: Work around strict Clang TLS symbol requirements
GCC and Clang both implement stack protector support based on Thread Local Storage (TLS) variables, and this is used in the kernel to implement per-task stack cookies, by copying a task's stack cookie into a per-CPU variable every time it is scheduled in.
Both now also implement -mstack-protector-guard-symbol=, which permits the TLS variable to be specified directly. This is useful because it will allow to move away from using a fixed offset of 40 bytes into the per-CPU area on x86_64, which requires a lot of special handling in the per-CPU code and the runtime relocation code.
However, while GCC is rather lax in its implementation of this command line option, Clang actually requires that the provided symbol name refers to a TLS variable (i.e., one declared with __thread), although it also permits the variable to be undeclared entirely, in which case it will use an implicit declaration of the right type.
The upshot of this is that Clang will emit the correct references to the stack cookie variable in most cases, e.g.,
10d: 64 a1 00 00 00 00 mov %fs:0x0,%eax 10f: R_386_32 __stack_chk_guard
However, if a non-TLS definition of the symbol in question is visible in the same compilation unit (which amounts to the whole of vmlinux if LTO is enabled), it will drop the per-CPU prefix and emit a load from a bogus address.
Work around this by using a symbol name that never occurs in C code, and emit it as an alias in the linker script.
Fixes: 3fb0fdb3bbe7 ("x86/stackprotector/32: Make the canary into a regular percpu variable") Signed-off-by: Ard Biesheuvel ardb@kernel.org Signed-off-by: Brian Gerst brgerst@gmail.com Signed-off-by: Borislav Petkov (AMD) bp@alien8.de Reviewed-by: Nathan Chancellor nathan@kernel.org Tested-by: Nathan Chancellor nathan@kernel.org Cc: stable@vger.kernel.org Link: https://github.com/ClangBuiltLinux/linux/issues/1854 Link: https://lore.kernel.org/r/20241105155801.1779119-2-brgerst@gmail.com --- arch/x86/Makefile | 5 +++-- arch/x86/entry/entry.S | 16 ++++++++++++++++ arch/x86/include/asm/asm-prototypes.h | 3 +++ arch/x86/kernel/cpu/common.c | 2 ++ arch/x86/kernel/vmlinux.lds.S | 3 +++ 5 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile index cd75e78..5b773b3 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -142,9 +142,10 @@ ifeq ($(CONFIG_X86_32),y)
ifeq ($(CONFIG_STACKPROTECTOR),y) ifeq ($(CONFIG_SMP),y) - KBUILD_CFLAGS += -mstack-protector-guard-reg=fs -mstack-protector-guard-symbol=__stack_chk_guard + KBUILD_CFLAGS += -mstack-protector-guard-reg=fs \ + -mstack-protector-guard-symbol=__ref_stack_chk_guard else - KBUILD_CFLAGS += -mstack-protector-guard=global + KBUILD_CFLAGS += -mstack-protector-guard=global endif endif else diff --git a/arch/x86/entry/entry.S b/arch/x86/entry/entry.S index 324686b..b7ea3e8 100644 --- a/arch/x86/entry/entry.S +++ b/arch/x86/entry/entry.S @@ -51,3 +51,19 @@ EXPORT_SYMBOL_GPL(mds_verw_sel); .popsection
THUNK warn_thunk_thunk, __warn_thunk + +#ifndef CONFIG_X86_64 +/* + * Clang's implementation of TLS stack cookies requires the variable in + * question to be a TLS variable. If the variable happens to be defined as an + * ordinary variable with external linkage in the same compilation unit (which + * amounts to the whole of vmlinux with LTO enabled), Clang will drop the + * segment register prefix from the references, resulting in broken code. Work + * around this by avoiding the symbol used in -mstack-protector-guard-symbol= + * entirely in the C code, and use an alias emitted by the linker script + * instead. + */ +#ifdef CONFIG_STACKPROTECTOR +EXPORT_SYMBOL(__ref_stack_chk_guard); +#endif +#endif diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h index 25466c4..3674006 100644 --- a/arch/x86/include/asm/asm-prototypes.h +++ b/arch/x86/include/asm/asm-prototypes.h @@ -20,3 +20,6 @@ extern void cmpxchg8b_emu(void); #endif
+#if defined(__GENKSYMS__) && defined(CONFIG_STACKPROTECTOR) +extern unsigned long __ref_stack_chk_guard; +#endif diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index a5f221e..f43bb97 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -2089,8 +2089,10 @@ void syscall_init(void)
#ifdef CONFIG_STACKPROTECTOR DEFINE_PER_CPU(unsigned long, __stack_chk_guard); +#ifndef CONFIG_SMP EXPORT_PER_CPU_SYMBOL(__stack_chk_guard); #endif +#endif
#endif /* CONFIG_X86_64 */
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index b8c5741..feb8102 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -491,6 +491,9 @@ SECTIONS . = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE), "kernel image bigger than KERNEL_IMAGE_SIZE");
+/* needed for Clang - see arch/x86/entry/entry.S */ +PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); + #ifdef CONFIG_X86_64 /* * Per-cpu symbols which need to be offset from __per_cpu_load
On 11/05, Brian Gerst wrote:
--- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -468,6 +468,9 @@ SECTIONS . = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE), "kernel image bigger than KERNEL_IMAGE_SIZE");
+/* needed for Clang - see arch/x86/entry/entry.S */ +PROVIDE(__ref_stack_chk_guard = __stack_chk_guard);
Don't we need the simple fix below?
without this patch I can't build the kernel with CONFIG_STACKPROTECTOR=n.
Oleg.
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index fab3ac9a4574..2ff48645bab9 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -472,8 +472,10 @@ SECTIONS . = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE), "kernel image bigger than KERNEL_IMAGE_SIZE");
+#ifdef CONFIG_STACKPROTECTOR /* needed for Clang - see arch/x86/entry/entry.S */ PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); +#endif
#ifdef CONFIG_X86_64 /*
On Fri, Dec 6, 2024 at 6:52 AM Oleg Nesterov oleg@redhat.com wrote:
On 11/05, Brian Gerst wrote:
--- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -468,6 +468,9 @@ SECTIONS . = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE), "kernel image bigger than KERNEL_IMAGE_SIZE");
+/* needed for Clang - see arch/x86/entry/entry.S */ +PROVIDE(__ref_stack_chk_guard = __stack_chk_guard);
Don't we need the simple fix below?
without this patch I can't build the kernel with CONFIG_STACKPROTECTOR=n.
Oleg.
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index fab3ac9a4574..2ff48645bab9 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -472,8 +472,10 @@ SECTIONS . = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE), "kernel image bigger than KERNEL_IMAGE_SIZE");
+#ifdef CONFIG_STACKPROTECTOR /* needed for Clang - see arch/x86/entry/entry.S */ PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); +#endif
#ifdef CONFIG_X86_64 /*
Which compiler are you using? It builds fine with GCC 14 and clang 18.
Brian Gerst
On 12/06, Brian Gerst wrote:
On Fri, Dec 6, 2024 at 6:52 AM Oleg Nesterov oleg@redhat.com wrote:
On 11/05, Brian Gerst wrote:
--- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -468,6 +468,9 @@ SECTIONS . = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE), "kernel image bigger than KERNEL_IMAGE_SIZE");
+/* needed for Clang - see arch/x86/entry/entry.S */ +PROVIDE(__ref_stack_chk_guard = __stack_chk_guard);
Don't we need the simple fix below?
without this patch I can't build the kernel with CONFIG_STACKPROTECTOR=n.
...
Which compiler are you using? It builds fine with GCC 14 and clang 18.
gcc version 5.3.1 20160406 (Red Hat 5.3.1-6) (GCC) GNU ld version 2.25-17.fc23
See also my reply to Ard
Oleg.
Add the necessary '#ifdef CONFIG_STACKPROTECTOR' into arch/x86/kernel/vmlinux.lds.S
Fixes: 577c134d311b ("x86/stackprotector: Work around strict Clang TLS symbol requirements") Cc: stable@vger.kernel.org Signed-off-by: Oleg Nesterov oleg@redhat.com --- arch/x86/kernel/vmlinux.lds.S | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index fab3ac9a4574..2ff48645bab9 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -472,8 +472,10 @@ SECTIONS . = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE), "kernel image bigger than KERNEL_IMAGE_SIZE");
+#ifdef CONFIG_STACKPROTECTOR /* needed for Clang - see arch/x86/entry/entry.S */ PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); +#endif
#ifdef CONFIG_X86_64 /*
On Fri, 6 Dec 2024 at 13:32, Oleg Nesterov oleg@redhat.com wrote:
Add the necessary '#ifdef CONFIG_STACKPROTECTOR' into arch/x86/kernel/vmlinux.lds.S
Fixes: 577c134d311b ("x86/stackprotector: Work around strict Clang TLS symbol requirements") Cc: stable@vger.kernel.org Signed-off-by: Oleg Nesterov oleg@redhat.com
arch/x86/kernel/vmlinux.lds.S | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index fab3ac9a4574..2ff48645bab9 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -472,8 +472,10 @@ SECTIONS . = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE), "kernel image bigger than KERNEL_IMAGE_SIZE");
+#ifdef CONFIG_STACKPROTECTOR /* needed for Clang - see arch/x86/entry/entry.S */ PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); +#endif
#ifdef CONFIG_X86_64 /*
This shouldn't be necessary - PROVIDE() is only evaluated if a reference exists to the symbol it defines.
Also, I'm failing to reproduce this. Could you share your .config, please, and the error that you get during the build?
On 12/06, Ard Biesheuvel wrote:
On Fri, 6 Dec 2024 at 13:32, Oleg Nesterov oleg@redhat.com wrote:
+#ifdef CONFIG_STACKPROTECTOR /* needed for Clang - see arch/x86/entry/entry.S */ PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); +#endif
#ifdef CONFIG_X86_64 /*
This shouldn't be necessary - PROVIDE() is only evaluated if a reference exists to the symbol it defines.
Also, I'm failing to reproduce this. Could you share your .config, please, and the error that you get during the build?
Please see the attached .config
without the change above:
$ make bzImage CALL scripts/checksyscalls.sh DESCEND objtool INSTALL libsubcmd_headers UPD include/generated/utsversion.h CC init/version-timestamp.o KSYMS .tmp_vmlinux0.kallsyms.S AS .tmp_vmlinux0.kallsyms.o LD .tmp_vmlinux1 ./arch/x86/kernel/vmlinux.lds:154: undefined symbol `__stack_chk_guard' referenced in expression scripts/Makefile.vmlinux:77: recipe for target 'vmlinux' failed make[2]: *** [vmlinux] Error 1 /home/oleg/tmp/LINUX/Makefile:1225: recipe for target 'vmlinux' failed make[1]: *** [vmlinux] Error 2 Makefile:251: recipe for target '__sub-make' failed make: *** [__sub-make] Error 2
perhaps this is because my toolchain is quite old,
$ ld -v GNU ld version 2.25-17.fc23
but according to Documentation/process/changes.rst
binutils 2.25 ld -v
it is still supported.
Oleg.
On Fri, 6 Dec 2024 at 15:22, Oleg Nesterov oleg@redhat.com wrote:
On 12/06, Ard Biesheuvel wrote:
On Fri, 6 Dec 2024 at 13:32, Oleg Nesterov oleg@redhat.com wrote:
+#ifdef CONFIG_STACKPROTECTOR /* needed for Clang - see arch/x86/entry/entry.S */ PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); +#endif
#ifdef CONFIG_X86_64 /*
This shouldn't be necessary - PROVIDE() is only evaluated if a reference exists to the symbol it defines.
Also, I'm failing to reproduce this. Could you share your .config, please, and the error that you get during the build?
Please see the attached .config
without the change above:
$ make bzImage CALL scripts/checksyscalls.sh DESCEND objtool INSTALL libsubcmd_headers UPD include/generated/utsversion.h CC init/version-timestamp.o KSYMS .tmp_vmlinux0.kallsyms.S AS .tmp_vmlinux0.kallsyms.o LD .tmp_vmlinux1 ./arch/x86/kernel/vmlinux.lds:154: undefined symbol `__stack_chk_guard' referenced in expression scripts/Makefile.vmlinux:77: recipe for target 'vmlinux' failed make[2]: *** [vmlinux] Error 1 /home/oleg/tmp/LINUX/Makefile:1225: recipe for target 'vmlinux' failed make[1]: *** [vmlinux] Error 2 Makefile:251: recipe for target '__sub-make' failed make: *** [__sub-make] Error 2perhaps this is because my toolchain is quite old,
$ ld -v GNU ld version 2.25-17.fc23but according to Documentation/process/changes.rst
binutils 2.25 ld -vit is still supported.
We're about to bump the minimum toolchain requirements to GCC 8.1 (and whichever version of binutils was current at the time), so you might want to consider upgrading.
However, you are right that these are still supported today, and so we need this fix this, especially because this has been backported to older stable kernels too.
For the patch,
Acked-by: Ard Biesheuvel ardb@kernel.org
On Fri, Dec 6, 2024 at 9:37 AM Ard Biesheuvel ardb@kernel.org wrote:
On Fri, 6 Dec 2024 at 15:22, Oleg Nesterov oleg@redhat.com wrote:
On 12/06, Ard Biesheuvel wrote:
On Fri, 6 Dec 2024 at 13:32, Oleg Nesterov oleg@redhat.com wrote:
+#ifdef CONFIG_STACKPROTECTOR /* needed for Clang - see arch/x86/entry/entry.S */ PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); +#endif
#ifdef CONFIG_X86_64 /*
This shouldn't be necessary - PROVIDE() is only evaluated if a reference exists to the symbol it defines.
Also, I'm failing to reproduce this. Could you share your .config, please, and the error that you get during the build?
Please see the attached .config
without the change above:
$ make bzImage CALL scripts/checksyscalls.sh DESCEND objtool INSTALL libsubcmd_headers UPD include/generated/utsversion.h CC init/version-timestamp.o KSYMS .tmp_vmlinux0.kallsyms.S AS .tmp_vmlinux0.kallsyms.o LD .tmp_vmlinux1 ./arch/x86/kernel/vmlinux.lds:154: undefined symbol `__stack_chk_guard' referenced in expression scripts/Makefile.vmlinux:77: recipe for target 'vmlinux' failed make[2]: *** [vmlinux] Error 1 /home/oleg/tmp/LINUX/Makefile:1225: recipe for target 'vmlinux' failed make[1]: *** [vmlinux] Error 2 Makefile:251: recipe for target '__sub-make' failed make: *** [__sub-make] Error 2perhaps this is because my toolchain is quite old,
$ ld -v GNU ld version 2.25-17.fc23but according to Documentation/process/changes.rst
binutils 2.25 ld -vit is still supported.
We're about to bump the minimum toolchain requirements to GCC 8.1 (and whichever version of binutils was current at the time), so you might want to consider upgrading.
However, you are right that these are still supported today, and so we need this fix this, especially because this has been backported to older stable kernels too.
For the patch,
Acked-by: Ard Biesheuvel ardb@kernel.org
Using PROVIDES() is now unnecessary.
Brian Gerst
On Fri, 6 Dec 2024 at 16:12, Brian Gerst brgerst@gmail.com wrote:
On Fri, Dec 6, 2024 at 9:37 AM Ard Biesheuvel ardb@kernel.org wrote:
On Fri, 6 Dec 2024 at 15:22, Oleg Nesterov oleg@redhat.com wrote:
On 12/06, Ard Biesheuvel wrote:
On Fri, 6 Dec 2024 at 13:32, Oleg Nesterov oleg@redhat.com wrote:
+#ifdef CONFIG_STACKPROTECTOR /* needed for Clang - see arch/x86/entry/entry.S */ PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); +#endif
#ifdef CONFIG_X86_64 /*
This shouldn't be necessary - PROVIDE() is only evaluated if a reference exists to the symbol it defines.
Also, I'm failing to reproduce this. Could you share your .config, please, and the error that you get during the build?
Please see the attached .config
without the change above:
$ make bzImage CALL scripts/checksyscalls.sh DESCEND objtool INSTALL libsubcmd_headers UPD include/generated/utsversion.h CC init/version-timestamp.o KSYMS .tmp_vmlinux0.kallsyms.S AS .tmp_vmlinux0.kallsyms.o LD .tmp_vmlinux1 ./arch/x86/kernel/vmlinux.lds:154: undefined symbol `__stack_chk_guard' referenced in expression scripts/Makefile.vmlinux:77: recipe for target 'vmlinux' failed make[2]: *** [vmlinux] Error 1 /home/oleg/tmp/LINUX/Makefile:1225: recipe for target 'vmlinux' failed make[1]: *** [vmlinux] Error 2 Makefile:251: recipe for target '__sub-make' failed make: *** [__sub-make] Error 2perhaps this is because my toolchain is quite old,
$ ld -v GNU ld version 2.25-17.fc23but according to Documentation/process/changes.rst
binutils 2.25 ld -vit is still supported.
We're about to bump the minimum toolchain requirements to GCC 8.1 (and whichever version of binutils was current at the time), so you might want to consider upgrading.
However, you are right that these are still supported today, and so we need this fix this, especially because this has been backported to older stable kernels too.
For the patch,
Acked-by: Ard Biesheuvel ardb@kernel.org
Using PROVIDES() is now unnecessary.
At this point, the use of -mstack-protector-guard-symbol is still limited to 32-bit x86. However, if we drop PROVIDE() here, the 64-bit kernel will also gain a symbol `__ref_stack_chk_guard` in its symbol table (and /proc/kallsyms, most likely).
Not sure whether that matters or not, but I'd rather keep the PROVIDE() as it doesn't do any harm.
Just to report this, bisection tomorrow unless someone figures it out in the meantime...
This is 64-bit, allmodconfig, clang:
clang --version Ubuntu clang version 15.0.7 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin
This guy:
Ubuntu clang version 18.1.3 (1ubuntu1) Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin
on the other box builds fine.
tip/master:
commit bc6bc2e1d7fa7e950c5ffb1ddf19bbaf15ad8863 (HEAD, refs/remotes/tip/master) Merge: f00b8d0b903a 72dafb567760 Author: Ingo Molnar mingo@kernel.org Date: Mon Mar 10 21:57:15 2025 +0100
Merge branch into tip/master: 'x86/sev'
# New commits in x86/sev: 72dafb567760 ("x86/sev: Add missing RIP_REL_REF() invocations during sme_enable()")
Signed-off-by: Ingo Molnar mingo@kernel.org
vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x30: relocation to !ENDBR: .text+0x180475 Absolute reference to symbol '__ref_stack_chk_guard' not permitted in .head.text make[3]: *** [arch/x86/Makefile.postlink:28: vmlinux] Error 1 make[2]: *** [scripts/Makefile.vmlinux:77: vmlinux] Error 2 make[2]: *** Deleting file 'vmlinux' make[1]: *** [/home/amd/kernel/linux/Makefile:1234: vmlinux] Error 2 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:251: __sub-make] Error 2
On Mon, 10 Mar 2025 at 22:44, Borislav Petkov bp@alien8.de wrote:
Just to report this, bisection tomorrow unless someone figures it out in the meantime...
This is 64-bit, allmodconfig, clang:
clang --version Ubuntu clang version 15.0.7 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin
This guy:
Ubuntu clang version 18.1.3 (1ubuntu1) Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin
on the other box builds fine.
tip/master:
commit bc6bc2e1d7fa7e950c5ffb1ddf19bbaf15ad8863 (HEAD, refs/remotes/tip/master) Merge: f00b8d0b903a 72dafb567760 Author: Ingo Molnar mingo@kernel.org Date: Mon Mar 10 21:57:15 2025 +0100
Merge branch into tip/master: 'x86/sev' # New commits in x86/sev: 72dafb567760 ("x86/sev: Add missing RIP_REL_REF() invocations during sme_enable()") Signed-off-by: Ingo Molnar <mingo@kernel.org>vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x30: relocation to !ENDBR: .text+0x180475 Absolute reference to symbol '__ref_stack_chk_guard' not permitted in .head.text make[3]: *** [arch/x86/Makefile.postlink:28: vmlinux] Error 1 make[2]: *** [scripts/Makefile.vmlinux:77: vmlinux] Error 2 make[2]: *** Deleting file 'vmlinux' make[1]: *** [/home/amd/kernel/linux/Makefile:1234: vmlinux] Error 2 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:251: __sub-make] Error 2
I tried building allmodconfig from the same commit using
$ clang-15 -v Debian clang version 15.0.7 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin
but it does not reproduce for me.
$ make LLVM=-15 bzImage -j100 -s drivers/spi/spi-amd.o: warning: objtool: amd_set_spi_freq() falls through to next function amd_spi_busy_wait() vmlinux.o: warning: objtool: screen_info_fixup_lfb+0x562: stack state mismatch: reg1[12]=-2-48 reg2[12]=-1+0 vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x30: relocation to !ENDBR: .text+0x17f535
and no error.
Could you capture the output of
objdump -dr .tmp_vmlinux2 --section .head.text
and share it somewhere please?
On Mon, Mar 10, 2025 at 11:19:03PM +0100, Ard Biesheuvel wrote:
and no error.
Oh fun.
Could you capture the output of
objdump -dr .tmp_vmlinux2 --section .head.text
and share it somewhere please?
See attached.
Now lemme try to bisect it, see what this machine says since it is magically toolchain or whatnot-specific. :-\
On Tue, 11 Mar 2025 at 11:24, Borislav Petkov bp@alien8.de wrote:
On Mon, Mar 10, 2025 at 11:19:03PM +0100, Ard Biesheuvel wrote:
and no error.
Oh fun.
Could you capture the output of
objdump -dr .tmp_vmlinux2 --section .head.text
and share it somewhere please?
See attached.
Now lemme try to bisect it, see what this machine says since it is magically toolchain or whatnot-specific. :-\
There are many occurrences of
ffffffff8373cb87: 49 c7 c6 20 c0 55 86 mov $0xffffffff8655c020,%r14 ffffffff8373cb8a: R_X86_64_32S __ref_stack_chk_guard
whereas the ordinary Clang uses R_X86_64_REX_GOTPCRELX here, which are relaxed by the linker.
I suspect that Ubuntu's Clang 15 has some additional patches that trigger this behavior.
We could add __no_stack_protector to __head to work around this.
On Tue, Mar 11, 2025 at 11:37:59AM +0100, Ard Biesheuvel wrote:
There are many occurrences of
ffffffff8373cb87: 49 c7 c6 20 c0 55 86 mov $0xffffffff8655c020,%r14 ffffffff8373cb8a: R_X86_64_32S __ref_stack_chk_guard
whereas the ordinary Clang uses R_X86_64_REX_GOTPCRELX here, which are relaxed by the linker.
I suspect that Ubuntu's Clang 15 has some additional patches that trigger this behavior.
... and then we don't know what else out there does other "additional" patches
;-\
We could add __no_stack_protector to __head to work around this.
Yap, that fixes the build:
diff --git a/arch/x86/include/asm/init.h b/arch/x86/include/asm/init.h index 0e82ebc5d1e1..6cf4ea847dc3 100644 --- a/arch/x86/include/asm/init.h +++ b/arch/x86/include/asm/init.h @@ -2,7 +2,7 @@ #ifndef _ASM_X86_INIT_H #define _ASM_X86_INIT_H
-#define __head __section(".head.text") __no_sanitize_undefined +#define __head __section(".head.text") __no_sanitize_undefined __no_stack_protector
struct x86_mapping_info { void *(*alloc_pgt_page)(void *); /* allocate buf for page table */
Thx.
On Tue, Mar 11, 2025 at 12:21:12PM +0100, Borislav Petkov wrote:
Yap, that fixes the build:
Lemme run randbuilds with that one, see what else breaks with it.
On 03/11, Borislav Petkov wrote:
On Tue, Mar 11, 2025 at 12:21:12PM +0100, Borislav Petkov wrote:
Yap, that fixes the build:
Lemme run randbuilds with that one, see what else breaks with it.
sorry for the off-topic noise, but what about the
[PATCH] x86/stackprotector: fix build failure with CONFIG_STACKPROTECTOR=n https://lore.kernel.org/all/20241206123207.GA2091@redhat.com/
fix for the older binutils? It was acked by Ard.
Should I resend it?
Oleg.
On Tue, Mar 11, 2025 at 03:37:25PM +0100, Oleg Nesterov wrote:
sorry for the off-topic noise, but what about the
[PATCH] x86/stackprotector: fix build failure with CONFIG_STACKPROTECTOR=n https://lore.kernel.org/all/20241206123207.GA2091@redhat.com/
fix for the older binutils? It was acked by Ard.
Should I resend it?
Can you pls explain how you trigger this?
I just did a
# CONFIG_STACKPROTECTOR is not set
build here and it was fine.
So there's something else I'm missing.
Thx.
On 03/11, Borislav Petkov wrote:
On Tue, Mar 11, 2025 at 03:37:25PM +0100, Oleg Nesterov wrote:
sorry for the off-topic noise, but what about the
[PATCH] x86/stackprotector: fix build failure with CONFIG_STACKPROTECTOR=n https://lore.kernel.org/all/20241206123207.GA2091@redhat.com/
fix for the older binutils? It was acked by Ard.
Should I resend it?
Can you pls explain how you trigger this?
I just did a
# CONFIG_STACKPROTECTOR is not set
build here and it was fine.
So there's something else I'm missing.
See the "older binutils?" above ;)
my toolchain is quite old,
$ ld -v GNU ld version 2.25-17.fc23
but according to Documentation/process/changes.rst
binutils 2.25 ld -v
it should be still supported.
Oleg.
On Tue, Mar 11, 2025 at 07:10:57PM +0100, Oleg Nesterov wrote:
See the "older binutils?" above ;)
my toolchain is quite old,
$ ld -v GNU ld version 2.25-17.fc23
but according to Documentation/process/changes.rst
binutils 2.25 ld -v
it should be still supported.
So your issue happens because of older binutils? Any other ingredient?
I'd like for the commit message to contain *exactly* what we're fixing here so that anyone who reads this, can know whether this fix is needed on her/his kernel or not...
Thx.
On 03/11, Borislav Petkov wrote:
On Tue, Mar 11, 2025 at 07:10:57PM +0100, Oleg Nesterov wrote:
See the "older binutils?" above ;)
my toolchain is quite old,
$ ld -v GNU ld version 2.25-17.fc23
but according to Documentation/process/changes.rst
binutils 2.25 ld -v
it should be still supported.
So your issue happens because of older binutils? Any other ingredient?
Yes, I think so.
I'd like for the commit message to contain *exactly* what we're fixing here so that anyone who reads this, can know whether this fix is needed on her/his kernel or not...
OK. I'll update the subject/changelog to explain that this is only needed for the older binutils and send V2.
Oleg.
On Tue, Mar 11, 2025 at 3:24 PM Oleg Nesterov oleg@redhat.com wrote:
On 03/11, Borislav Petkov wrote:
On Tue, Mar 11, 2025 at 07:10:57PM +0100, Oleg Nesterov wrote:
See the "older binutils?" above ;)
my toolchain is quite old,
$ ld -v GNU ld version 2.25-17.fc23but according to Documentation/process/changes.rst
binutils 2.25 ld -vit should be still supported.
So your issue happens because of older binutils? Any other ingredient?
Yes, I think so.
I'd like for the commit message to contain *exactly* what we're fixing here so that anyone who reads this, can know whether this fix is needed on her/his kernel or not...
OK. I'll update the subject/changelog to explain that this is only needed for the older binutils and send V2.
With it conditional on CONFIG_STACKPROTECTOR, you can also drop PROVIDES().
Brian Gerst
On 03/11, Brian Gerst wrote:
On Tue, Mar 11, 2025 at 3:24 PM Oleg Nesterov oleg@redhat.com wrote:
OK. I'll update the subject/changelog to explain that this is only needed for the older binutils and send V2.
With it conditional on CONFIG_STACKPROTECTOR, you can also drop PROVIDES().
Sorry Brian, I don't understand this magic even remotely...
Do you mean
-/* needed for Clang - see arch/x86/entry/entry.S */ -PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); +#ifdef CONFIG_STACKPROTECTOR +__ref_stack_chk_guard = __stack_chk_guard; +#endif
?
Oleg.
On Tue, Mar 11, 2025 at 5:42 PM Oleg Nesterov oleg@redhat.com wrote:
On 03/11, Brian Gerst wrote:
On Tue, Mar 11, 2025 at 3:24 PM Oleg Nesterov oleg@redhat.com wrote:
OK. I'll update the subject/changelog to explain that this is only needed for the older binutils and send V2.
With it conditional on CONFIG_STACKPROTECTOR, you can also drop PROVIDES().
Sorry Brian, I don't understand this magic even remotely...
Do you mean
-/* needed for Clang - see arch/x86/entry/entry.S */ -PROVIDE(__ref_stack_chk_guard = __stack_chk_guard); +#ifdef CONFIG_STACKPROTECTOR +__ref_stack_chk_guard = __stack_chk_guard; +#endif?
Oleg.
Yes. Also keep the comment about Clang.
Brian Gerst
On Tue, Mar 11, 2025 at 02:13:56PM +0100, Borislav Petkov wrote:
On Tue, Mar 11, 2025 at 12:21:12PM +0100, Borislav Petkov wrote:
Yap, that fixes the build:
Lemme run randbuilds with that one, see what else breaks with it.
Yeah, looks good.
Pls send a proper patch.
Thx.
linux-stable-mirror@lists.linaro.org
-
Ard Biesheuvel -
Borislav Petkov -
Brian Gerst -
Nathan Chancellor -
Oleg Nesterov -
tip-bot2 for Ard Biesheuvel