Skip to content

Commit 060de9c

Browse files
ardbiesheuvelsmb49
authored andcommitted
x86/stackprotector: Work around strict Clang TLS symbol requirements
BugLink: https://bugs.launchpad.net/bugs/2101042 commit 577c134d311b9b94598d7a0c86be1f431f823003 upstream. 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: 3fb0fdb ("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: ClangBuiltLinux/linux#1854 Link: https://lore.kernel.org/r/20241105155801.1779119-2-brgerst@gmail.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Manuel Diewald <manuel.diewald@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
1 parent 1f6b7fe commit 060de9c

File tree

5 files changed

+26
-2
lines changed

5 files changed

+26
-2
lines changed

arch/x86/Makefile

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,10 @@ ifeq ($(CONFIG_X86_32),y)
114114

115115
ifeq ($(CONFIG_STACKPROTECTOR),y)
116116
ifeq ($(CONFIG_SMP),y)
117-
KBUILD_CFLAGS += -mstack-protector-guard-reg=fs -mstack-protector-guard-symbol=__stack_chk_guard
117+
KBUILD_CFLAGS += -mstack-protector-guard-reg=fs \
118+
-mstack-protector-guard-symbol=__ref_stack_chk_guard
118119
else
119-
KBUILD_CFLAGS += -mstack-protector-guard=global
120+
KBUILD_CFLAGS += -mstack-protector-guard=global
120121
endif
121122
endif
122123
else

arch/x86/entry/entry.S

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,18 @@ EXPORT_SYMBOL_GPL(mds_verw_sel);
4848

4949
.popsection
5050

51+
#ifndef CONFIG_X86_64
52+
/*
53+
* Clang's implementation of TLS stack cookies requires the variable in
54+
* question to be a TLS variable. If the variable happens to be defined as an
55+
* ordinary variable with external linkage in the same compilation unit (which
56+
* amounts to the whole of vmlinux with LTO enabled), Clang will drop the
57+
* segment register prefix from the references, resulting in broken code. Work
58+
* around this by avoiding the symbol used in -mstack-protector-guard-symbol=
59+
* entirely in the C code, and use an alias emitted by the linker script
60+
* instead.
61+
*/
62+
#ifdef CONFIG_STACKPROTECTOR
63+
EXPORT_SYMBOL(__ref_stack_chk_guard);
64+
#endif
65+
#endif

arch/x86/include/asm/asm-prototypes.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,6 @@
1919
extern void cmpxchg8b_emu(void);
2020
#endif
2121

22+
#if defined(__GENKSYMS__) && defined(CONFIG_STACKPROTECTOR)
23+
extern unsigned long __ref_stack_chk_guard;
24+
#endif

arch/x86/kernel/cpu/common.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2153,8 +2153,10 @@ void syscall_init(void)
21532153

21542154
#ifdef CONFIG_STACKPROTECTOR
21552155
DEFINE_PER_CPU(unsigned long, __stack_chk_guard);
2156+
#ifndef CONFIG_SMP
21562157
EXPORT_PER_CPU_SYMBOL(__stack_chk_guard);
21572158
#endif
2159+
#endif
21582160

21592161
#endif /* CONFIG_X86_64 */
21602162

arch/x86/kernel/vmlinux.lds.S

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,9 @@ SECTIONS
489489
. = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE),
490490
"kernel image bigger than KERNEL_IMAGE_SIZE");
491491

492+
/* needed for Clang - see arch/x86/entry/entry.S */
493+
PROVIDE(__ref_stack_chk_guard = __stack_chk_guard);
494+
492495
#ifdef CONFIG_X86_64
493496
/*
494497
* Per-cpu symbols which need to be offset from __per_cpu_load

0 commit comments

Comments
 (0)