x86/stackprotector/64: Convert to normal per-CPU variable

Older versions of GCC fixed the location of the stack protector canary
at %gs:40.  This constraint forced the percpu section to be linked at
absolute address 0 so that the canary could be the first data object in
the percpu section.  Supporting the zero-based percpu section requires
additional code to handle relocations for RIP-relative references to
percpu data, extra complexity to kallsyms, and workarounds for linker
bugs due to the use of absolute symbols.

GCC 8.1 supports redefining where the canary is located, allowing it to
become a normal percpu variable instead of at a fixed location.  This
removes the constraint that the percpu section must be zero-based.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/20250123190747.745588-8-brgerst@gmail.com

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 5b773b34768d1282ab4b6e0cb56f2b1a851cc251..88a1705366f918c15406ed220c073e6c46e2a81c 100644 (file)
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -140,14 +140,7 @@ ifeq ($(CONFIG_X86_32),y)
         # temporary until string.h is fixed
         KBUILD_CFLAGS += -ffreestanding
 
-    ifeq ($(CONFIG_STACKPROTECTOR),y)
-        ifeq ($(CONFIG_SMP),y)
-            KBUILD_CFLAGS += -mstack-protector-guard-reg=fs \
-                             -mstack-protector-guard-symbol=__ref_stack_chk_guard
-        else
-            KBUILD_CFLAGS += -mstack-protector-guard=global
-        endif
-    endif
+        percpu_seg := fs
 else
         BITS := 64
         UTS_MACHINE := x86_64
@@ -197,6 +190,17 @@ else
         KBUILD_CFLAGS += -mcmodel=kernel
         KBUILD_RUSTFLAGS += -Cno-redzone=y
         KBUILD_RUSTFLAGS += -Ccode-model=kernel
+
+        percpu_seg := gs
+endif
+
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+    ifeq ($(CONFIG_SMP),y)
+       KBUILD_CFLAGS += -mstack-protector-guard-reg=$(percpu_seg)
+       KBUILD_CFLAGS += -mstack-protector-guard-symbol=__ref_stack_chk_guard
+    else
+       KBUILD_CFLAGS += -mstack-protector-guard=global
+    endif
 endif
 
 #

diff --git a/arch/x86/entry/entry.S b/arch/x86/entry/entry.S
index b7ea3e8e9eccd540a2ff55eda7a05266a48a4f49..fe5344a249a1ba1842021663cf23c7d454f96e70 100644 (file)
--- a/arch/x86/entry/entry.S
+++ b/arch/x86/entry/entry.S
@@ -52,7 +52,6 @@ EXPORT_SYMBOL_GPL(mds_verw_sel);
 
 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
@@ -66,4 +65,3 @@ THUNK warn_thunk_thunk, __warn_thunk
 #ifdef CONFIG_STACKPROTECTOR
 EXPORT_SYMBOL(__ref_stack_chk_guard);
 #endif
-#endif

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index f52dbe0ad93cd1e261f605e7cbd6209517859a07..33a955aa01d895c1be8457ee66bf3b524f169eb1 100644 (file)
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -192,7 +192,7 @@ SYM_FUNC_START(__switch_to_asm)
 
 #ifdef CONFIG_STACKPROTECTOR
        movq    TASK_stack_canary(%rsi), %rbx
-       movq    %rbx, PER_CPU_VAR(fixed_percpu_data + FIXED_stack_canary)
+       movq    %rbx, PER_CPU_VAR(__stack_chk_guard)
 #endif
 
        /*

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index c0cd10182e90c12eed155cbdf4ec1d72b76409ab..a4687122951f75e8d24b84649b484943189aad13 100644 (file)
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -422,16 +422,8 @@ struct irq_stack {
 
 #ifdef CONFIG_X86_64
 struct fixed_percpu_data {
-       /*
-        * GCC hardcodes the stack canary as %gs:40.  Since the
-        * irq_stack is the object at %gs:0, we reserve the bottom
-        * 48 bytes of the irq stack for the canary.
-        *
-        * Once we are willing to require -mstack-protector-guard-symbol=
-        * support for x86_64 stackprotector, we can get rid of this.
-        */
        char            gs_base[40];
-       unsigned long   stack_canary;
+       unsigned long   reserved;
 };
 
 DECLARE_PER_CPU_FIRST(struct fixed_percpu_data, fixed_percpu_data) __visible;
@@ -446,11 +438,7 @@ extern asmlinkage void entry_SYSCALL32_ignore(void);
 
 /* Save actual FS/GS selectors and bases to current->thread */
 void current_save_fsgs(void);
-#else  /* X86_64 */
-#ifdef CONFIG_STACKPROTECTOR
-DECLARE_PER_CPU(unsigned long, __stack_chk_guard);
-#endif
-#endif /* !X86_64 */
+#endif /* X86_64 */
 
 struct perf_event;
 

diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
index 00473a650f512b2e47110e21726756bb2d018cd3..d43fb589fcf6639f44438ce2fbc99f84d64c6565 100644 (file)
--- a/arch/x86/include/asm/stackprotector.h
+++ b/arch/x86/include/asm/stackprotector.h
@@ -2,26 +2,10 @@
 /*
  * GCC stack protector support.
  *
- * Stack protector works by putting predefined pattern at the start of
+ * Stack protector works by putting a predefined pattern at the start of
  * the stack frame and verifying that it hasn't been overwritten when
- * returning from the function.  The pattern is called stack canary
- * and unfortunately gcc historically required it to be at a fixed offset
- * from the percpu segment base.  On x86_64, the offset is 40 bytes.
- *
- * The same segment is shared by percpu area and stack canary.  On
- * x86_64, percpu symbols are zero based and %gs (64-bit) points to the
- * base of percpu area.  The first occupant of the percpu area is always
- * fixed_percpu_data which contains stack_canary at the appropriate
- * offset.  On x86_32, the stack canary is just a regular percpu
- * variable.
- *
- * Putting percpu data in %fs on 32-bit is a minor optimization compared to
- * using %gs.  Since 32-bit userspace normally has %fs == 0, we are likely
- * to load 0 into %fs on exit to usermode, whereas with percpu data in
- * %gs, we are likely to load a non-null %gs on return to user mode.
- *
- * Once we are willing to require GCC 8.1 or better for 64-bit stackprotector
- * support, we can remove some of this complexity.
+ * returning from the function.  The pattern is called the stack canary
+ * and is a unique value for each task.
  */
 
 #ifndef _ASM_STACKPROTECTOR_H
@@ -36,6 +20,8 @@
 
 #include <linux/sched.h>
 
+DECLARE_PER_CPU(unsigned long, __stack_chk_guard);
+
 /*
  * Initialize the stackprotector canary value.
  *
@@ -51,25 +37,13 @@ static __always_inline void boot_init_stack_canary(void)
 {
        unsigned long canary = get_random_canary();
 
-#ifdef CONFIG_X86_64
-       BUILD_BUG_ON(offsetof(struct fixed_percpu_data, stack_canary) != 40);
-#endif
-
        current->stack_canary = canary;
-#ifdef CONFIG_X86_64
-       this_cpu_write(fixed_percpu_data.stack_canary, canary);
-#else
        this_cpu_write(__stack_chk_guard, canary);
-#endif
 }
 
 static inline void cpu_init_stack_canary(int cpu, struct task_struct *idle)
 {
-#ifdef CONFIG_X86_64
-       per_cpu(fixed_percpu_data.stack_canary, cpu) = idle->stack_canary;
-#else
        per_cpu(__stack_chk_guard, cpu) = idle->stack_canary;
-#endif
 }
 
 #else  /* STACKPROTECTOR */

diff --git a/arch/x86/kernel/asm-offsets_64.c b/arch/x86/kernel/asm-offsets_64.c
index bb65371ea9df521e37cf6d0c27688a9fbf47af19..590b6cd0eac0e45b9fef4ec7390db620d1d9dc0c 100644 (file)
--- a/arch/x86/kernel/asm-offsets_64.c
+++ b/arch/x86/kernel/asm-offsets_64.c
@@ -54,11 +54,5 @@ int main(void)
        BLANK();
 #undef ENTRY
 
-       BLANK();
-
-#ifdef CONFIG_STACKPROTECTOR
-       OFFSET(FIXED_stack_canary, fixed_percpu_data, stack_canary);
-       BLANK();
-#endif
        return 0;
 }

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 7cce91b19fb2c5bf59fb09dd392e6f79c8a55a14..b71178f0ed6c65b3e582beaacf9c68688a15afbb 100644 (file)
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2089,8 +2089,7 @@ void syscall_init(void)
        if (!cpu_feature_enabled(X86_FEATURE_FRED))
                idt_syscall_init();
 }
-
-#else  /* CONFIG_X86_64 */
+#endif /* CONFIG_X86_64 */
 
 #ifdef CONFIG_STACKPROTECTOR
 DEFINE_PER_CPU(unsigned long, __stack_chk_guard);
@@ -2099,8 +2098,6 @@ EXPORT_PER_CPU_SYMBOL(__stack_chk_guard);
 #endif
 #endif
 
-#endif /* CONFIG_X86_64 */
-
 /*
  * Clear all 6 debug registers:
  */

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 31345e0ba0064e23056a659ded3c3259a736412d..c3d73c04603f410bc430465c355a3c7a2b5221f3 100644 (file)
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -361,8 +361,7 @@ SYM_INNER_LABEL(common_startup_64, SYM_L_LOCAL)
 
        /* Set up %gs.
         *
-        * The base of %gs always points to fixed_percpu_data. If the
-        * stack protector canary is enabled, it is located at %gs:40.
+        * The base of %gs always points to fixed_percpu_data.
         * Note that, on SMP, the boot cpu uses init data section until
         * the per cpu areas are set up.
         */

diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index 894edf8d6d62f8a7db7a86b1b5765b0a24e0a3fb..a31b057e641e44ac73db89ace5fbf54cd9027cf4 100644 (file)
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -33,8 +33,7 @@ SYM_CODE_START(startup_xen)
 
        /* Set up %gs.
         *
-        * The base of %gs always points to fixed_percpu_data.  If the
-        * stack protector canary is enabled, it is located at %gs:40.
+        * The base of %gs always points to fixed_percpu_data.
         * Note that, on SMP, the boot cpu uses init data section until
         * the per cpu areas are set up.
         */