2021-11-30 20:58:06

by Brian Gerst

[permalink] [raw]
Subject: [PATCH v2 0/6] x86-64: Stack protector and percpu improvements

Currently, x86-64 uses an unusual per-cpu layout, where the percpu section
is linked at absolute address 0. The reason behind this is that older GCC
versions placed the stack protector (if enabled) at a fixed offset from the
GS segment base. Since the GS segement is also used for percpu variables,
this forced the current layout.

GCC since version 8.1 supports a configurable location for the stack
protector value, which allows removal of the restriction on how the percpu
section is linked.

Changes from v1:
- Remove fixed location stack protector support

Brian Gerst (6):
x86: Remove stack protector test scripts
x86-64: Convert stack protector to normal percpu variable
x86-64: Use relative per-cpu offsets
x86-64: Remove inverse relocations
kallsyms: Remove KALLSYMS_ABSOLUTE_PERCPU
percpu: Remove PER_CPU_FIRST_SECTION

arch/x86/Kconfig | 7 +-
arch/x86/Makefile | 19 +--
arch/x86/boot/compressed/misc.c | 14 +--
arch/x86/entry/entry_64.S | 2 +-
arch/x86/include/asm/percpu.h | 22 ----
arch/x86/include/asm/processor.h | 25 +---
arch/x86/include/asm/stackprotector.h | 36 ++----
arch/x86/kernel/asm-offsets_64.c | 6 -
arch/x86/kernel/cpu/common.c | 8 +-
arch/x86/kernel/head_64.S | 10 +-
arch/x86/kernel/irq_64.c | 1 -
arch/x86/kernel/setup_percpu.c | 12 +-
arch/x86/kernel/vmlinux.lds.S | 33 -----
arch/x86/tools/relocs.c | 145 +---------------------
arch/x86/xen/xen-head.S | 10 +-
include/asm-generic/vmlinux.lds.h | 1 -
include/linux/percpu-defs.h | 12 --
init/Kconfig | 11 +-
kernel/kallsyms.c | 12 +-
scripts/gcc-x86_32-has-stack-protector.sh | 8 --
scripts/gcc-x86_64-has-stack-protector.sh | 4 -
scripts/kallsyms.c | 68 ++--------
scripts/link-vmlinux.sh | 4 -
23 files changed, 56 insertions(+), 414 deletions(-)
delete mode 100755 scripts/gcc-x86_32-has-stack-protector.sh
delete mode 100755 scripts/gcc-x86_64-has-stack-protector.sh

--
2.31.1



2021-11-30 20:58:12

by Brian Gerst

[permalink] [raw]
Subject: [PATCH v2 4/6] x86-64: Remove inverse relocations

Now that the percpu section is not at a fixed address, inverse
relocations (which were needed to offset the effects of relocation on
RIP-relative percpu references) are no longer used.

Signed-off-by: Brian Gerst <[email protected]>
---
arch/x86/boot/compressed/misc.c | 14 +-------------
arch/x86/tools/relocs.c | 9 ---------
2 files changed, 1 insertion(+), 22 deletions(-)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index a4339cb2d247..5d0ffab408cd 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -215,7 +215,7 @@ static void handle_relocations(void *output, unsigned long output_len,

/*
* Process relocations: 32 bit relocations first then 64 bit after.
- * Three sets of binary relocations are added to the end of the kernel
+ * Two sets of binary relocations are added to the end of the kernel
* before compression. Each relocation table entry is the kernel
* address of the location which needs to be updated stored as a
* 32-bit value which is sign extended to 64 bits.
@@ -225,8 +225,6 @@ static void handle_relocations(void *output, unsigned long output_len,
* kernel bits...
* 0 - zero terminator for 64 bit relocations
* 64 bit relocation repeated
- * 0 - zero terminator for inverse 32 bit relocations
- * 32 bit inverse relocation repeated
* 0 - zero terminator for 32 bit relocations
* 32 bit relocation repeated
*
@@ -243,16 +241,6 @@ static void handle_relocations(void *output, unsigned long output_len,
*(uint32_t *)ptr += delta;
}
#ifdef CONFIG_X86_64
- while (*--reloc) {
- long extended = *reloc;
- extended += map;
-
- ptr = (unsigned long)extended;
- if (ptr < min_addr || ptr > max_addr)
- error("inverse 32-bit relocation outside of kernel!\n");
-
- *(int32_t *)ptr -= delta;
- }
for (reloc--; *reloc; reloc--) {
long extended = *reloc;
extended += map;
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index b27980291ff8..e452b54ddaa1 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -28,7 +28,6 @@ struct relocs {
static struct relocs relocs16;
static struct relocs relocs32;
#if ELF_BITS == 64
-static struct relocs relocs32neg;
static struct relocs relocs64;
#define FMT PRIu64
#else
@@ -959,7 +958,6 @@ static void emit_relocs(int as_text, int use_real_mode)
/* Order the relocations for more efficient processing */
sort_relocs(&relocs32);
#if ELF_BITS == 64
- sort_relocs(&relocs32neg);
sort_relocs(&relocs64);
#else
sort_relocs(&relocs16);
@@ -991,13 +989,6 @@ static void emit_relocs(int as_text, int use_real_mode)
/* Now print each relocation */
for (i = 0; i < relocs64.count; i++)
write_reloc(relocs64.offset[i], stdout);
-
- /* Print a stop */
- write_reloc(0, stdout);
-
- /* Now print each inverse 32-bit relocation */
- for (i = 0; i < relocs32neg.count; i++)
- write_reloc(relocs32neg.offset[i], stdout);
#endif

/* Print a stop */
--
2.31.1


2021-11-30 20:58:32

by Brian Gerst

[permalink] [raw]
Subject: [PATCH v2 6/6] percpu: Remove PER_CPU_FIRST_SECTION

x86-64 was the only user.

Signed-off-by: Brian Gerst <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 1 -
include/linux/percpu-defs.h | 12 ------------
2 files changed, 13 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 42f3866bca69..a7fc94e1b985 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -1056,7 +1056,6 @@
*/
#define PERCPU_INPUT(cacheline) \
__per_cpu_start = .; \
- *(.data..percpu..first) \
. = ALIGN(PAGE_SIZE); \
*(.data..percpu..page_aligned) \
. = ALIGN(cacheline); \
diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
index af1071535de8..819d1b9ff8fe 100644
--- a/include/linux/percpu-defs.h
+++ b/include/linux/percpu-defs.h
@@ -26,13 +26,11 @@
#define PER_CPU_SHARED_ALIGNED_SECTION "..shared_aligned"
#define PER_CPU_ALIGNED_SECTION "..shared_aligned"
#endif
-#define PER_CPU_FIRST_SECTION "..first"

#else

#define PER_CPU_SHARED_ALIGNED_SECTION ""
#define PER_CPU_ALIGNED_SECTION "..shared_aligned"
-#define PER_CPU_FIRST_SECTION ""

#endif

@@ -114,16 +112,6 @@
#define DEFINE_PER_CPU(type, name) \
DEFINE_PER_CPU_SECTION(type, name, "")

-/*
- * Declaration/definition used for per-CPU variables that must come first in
- * the set of variables.
- */
-#define DECLARE_PER_CPU_FIRST(type, name) \
- DECLARE_PER_CPU_SECTION(type, name, PER_CPU_FIRST_SECTION)
-
-#define DEFINE_PER_CPU_FIRST(type, name) \
- DEFINE_PER_CPU_SECTION(type, name, PER_CPU_FIRST_SECTION)
-
/*
* Declaration/definition used for per-CPU variables that must be cacheline
* aligned under SMP conditions so that, whilst a particular instance of the
--
2.31.1


2021-11-30 20:59:03

by Brian Gerst

[permalink] [raw]
Subject: [PATCH v2 5/6] kallsyms: Remove KALLSYMS_ABSOLUTE_PERCPU

x86-64 was the only user.

Signed-off-by: Brian Gerst <[email protected]>
---
init/Kconfig | 10 +-----
kernel/kallsyms.c | 12 ++------
scripts/kallsyms.c | 68 ++++++-----------------------------------
scripts/link-vmlinux.sh | 4 ---
4 files changed, 12 insertions(+), 82 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 943552dc9c19..981621f658a9 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1711,10 +1711,6 @@ config KALLSYMS_ALL

Say N unless you really need all symbols.

-config KALLSYMS_ABSOLUTE_PERCPU
- bool
- depends on KALLSYMS
-
config KALLSYMS_BASE_RELATIVE
bool
depends on KALLSYMS
@@ -1722,11 +1718,7 @@ config KALLSYMS_BASE_RELATIVE
help
Instead of emitting them as absolute values in the native word size,
emit the symbol references in the kallsyms table as 32-bit entries,
- each containing a relative value in the range [base, base + U32_MAX]
- or, when KALLSYMS_ABSOLUTE_PERCPU is in effect, each containing either
- an absolute value in the range [0, S32_MAX] or a relative value in the
- range [base, base + S32_MAX], where base is the lowest relative symbol
- address encountered in the image.
+ each containing a relative value in the range [base, base + U32_MAX].

On 64-bit builds, this reduces the size of the address table by 50%,
but more importantly, it results in entries whose values are build
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 3011bc33a5ba..bea1cfc5cacd 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -152,16 +152,8 @@ static unsigned long kallsyms_sym_address(int idx)
if (!IS_ENABLED(CONFIG_KALLSYMS_BASE_RELATIVE))
return kallsyms_addresses[idx];

- /* values are unsigned offsets if --absolute-percpu is not in effect */
- if (!IS_ENABLED(CONFIG_KALLSYMS_ABSOLUTE_PERCPU))
- return kallsyms_relative_base + (u32)kallsyms_offsets[idx];
-
- /* ...otherwise, positive offsets are absolute values */
- if (kallsyms_offsets[idx] >= 0)
- return kallsyms_offsets[idx];
-
- /* ...and negative offsets are relative to kallsyms_relative_base - 1 */
- return kallsyms_relative_base - 1 - kallsyms_offsets[idx];
+ /* values are unsigned offsets */
+ return kallsyms_relative_base + (u32)kallsyms_offsets[idx];
}

static bool cleanup_symbol_name(char *s)
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 54ad86d13784..f6419efc2c54 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -33,7 +33,6 @@ struct sym_entry {
unsigned long long addr;
unsigned int len;
unsigned int start_pos;
- unsigned int percpu_absolute;
unsigned char sym[];
};

@@ -51,14 +50,9 @@ static struct addr_range text_ranges[] = {
#define text_range_text (&text_ranges[0])
#define text_range_inittext (&text_ranges[1])

-static struct addr_range percpu_range = {
- "__per_cpu_start", "__per_cpu_end", -1ULL, 0
-};
-
static struct sym_entry **table;
static unsigned int table_size, table_cnt;
static int all_symbols;
-static int absolute_percpu;
static int base_relative;

static int token_profit[0x10000];
@@ -224,7 +218,6 @@ static struct sym_entry *read_symbol(FILE *in)
return NULL;

check_symbol_range(name, addr, text_ranges, ARRAY_SIZE(text_ranges));
- check_symbol_range(name, addr, &percpu_range, 1);

/* include the type field in the symbol name, so that it gets
* compressed together */
@@ -241,7 +234,6 @@ static struct sym_entry *read_symbol(FILE *in)
sym->len = len;
sym->sym[0] = type;
strcpy(sym_name(sym), name);
- sym->percpu_absolute = 0;

return sym;
}
@@ -379,11 +371,6 @@ static int expand_symbol(const unsigned char *data, int len, char *result)
return total;
}

-static int symbol_absolute(const struct sym_entry *s)
-{
- return s->percpu_absolute;
-}
-
static void write_src(void)
{
unsigned int i, k, off;
@@ -419,28 +406,17 @@ static void write_src(void)
long long offset;
int overflow;

- if (!absolute_percpu) {
- offset = table[i]->addr - relative_base;
- overflow = (offset < 0 || offset > UINT_MAX);
- } else if (symbol_absolute(table[i])) {
- offset = table[i]->addr;
- overflow = (offset < 0 || offset > INT_MAX);
- } else {
- offset = relative_base - table[i]->addr - 1;
- overflow = (offset < INT_MIN || offset >= 0);
- }
+ offset = table[i]->addr - relative_base;
+ overflow = (offset < 0 || offset > UINT_MAX);
if (overflow) {
fprintf(stderr, "kallsyms failure: "
- "%s symbol value %#llx out of range in relative mode\n",
- symbol_absolute(table[i]) ? "absolute" : "relative",
+ "symbol value %#llx out of range in relative mode\n",
table[i]->addr);
exit(EXIT_FAILURE);
}
printf("\t.long\t%#x\n", (int)offset);
- } else if (!symbol_absolute(table[i])) {
- output_address(table[i]->addr);
} else {
- printf("\tPTR\t%#llx\n", table[i]->addr);
+ output_address(table[i]->addr);
}
}
printf("\n");
@@ -730,36 +706,14 @@ static void sort_symbols(void)
qsort(table, table_cnt, sizeof(table[0]), compare_symbols);
}

-static void make_percpus_absolute(void)
-{
- unsigned int i;
-
- for (i = 0; i < table_cnt; i++)
- if (symbol_in_range(table[i], &percpu_range, 1)) {
- /*
- * Keep the 'A' override for percpu symbols to
- * ensure consistent behavior compared to older
- * versions of this tool.
- */
- table[i]->sym[0] = 'A';
- table[i]->percpu_absolute = 1;
- }
-}
-
/* find the minimum non-absolute symbol address */
static void record_relative_base(void)
{
- unsigned int i;
-
- for (i = 0; i < table_cnt; i++)
- if (!symbol_absolute(table[i])) {
- /*
- * The table is sorted by address.
- * Take the first non-absolute symbol value.
- */
- relative_base = table[i]->addr;
- return;
- }
+ /*
+ * The table is sorted by address.
+ * Take the first symbol value.
+ */
+ relative_base = table[0]->addr;
}

int main(int argc, char **argv)
@@ -769,8 +723,6 @@ int main(int argc, char **argv)
for (i = 1; i < argc; i++) {
if(strcmp(argv[i], "--all-symbols") == 0)
all_symbols = 1;
- else if (strcmp(argv[i], "--absolute-percpu") == 0)
- absolute_percpu = 1;
else if (strcmp(argv[i], "--base-relative") == 0)
base_relative = 1;
else
@@ -781,8 +733,6 @@ int main(int argc, char **argv)

read_map(stdin);
shrink_table();
- if (absolute_percpu)
- make_percpus_absolute();
sort_symbols();
if (base_relative)
record_relative_base();
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 5cdd9bc5c385..797b177cc181 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -243,10 +243,6 @@ kallsyms()
kallsymopt="${kallsymopt} --all-symbols"
fi

- if [ -n "${CONFIG_KALLSYMS_ABSOLUTE_PERCPU}" ]; then
- kallsymopt="${kallsymopt} --absolute-percpu"
- fi
-
if [ -n "${CONFIG_KALLSYMS_BASE_RELATIVE}" ]; then
kallsymopt="${kallsymopt} --base-relative"
fi
--
2.31.1


2021-11-30 20:59:37

by Brian Gerst

[permalink] [raw]
Subject: [PATCH v2 3/6] x86-64: Use relative per-cpu offsets

The per-cpu section is currently linked at virtual address 0, because
older compilers hardcoded the stack protector canary value at a fixed
offset from the start of the GS segment. Now that the canary is a
normal per-cpu variable, the special handling of the per-cpu section
can be removed.

Signed-off-by: Brian Gerst <[email protected]>
---
arch/x86/include/asm/percpu.h | 22 ------
arch/x86/kernel/head_64.S | 6 +-
arch/x86/kernel/irq_64.c | 1 -
arch/x86/kernel/setup_percpu.c | 12 +--
arch/x86/kernel/vmlinux.lds.S | 27 -------
arch/x86/tools/relocs.c | 135 +--------------------------------
init/Kconfig | 1 -
7 files changed, 6 insertions(+), 198 deletions(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index a3c33b79fb86..5418f0a4d073 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -16,12 +16,6 @@
#define PER_CPU_VAR(var) var
#endif /* SMP */

-#ifdef CONFIG_X86_64_SMP
-#define INIT_PER_CPU_VAR(var) init_per_cpu__##var
-#else
-#define INIT_PER_CPU_VAR(var) var
-#endif
-
#else /* ...!ASSEMBLY */

#include <linux/kernel.h>
@@ -49,22 +43,6 @@

#define __percpu_arg(x) __percpu_prefix "%" #x

-/*
- * Initialized pointers to per-cpu variables needed for the boot
- * processor need to use these macros to get the proper address
- * offset from __per_cpu_load on SMP.
- *
- * There also must be an entry in vmlinux_64.lds.S
- */
-#define DECLARE_INIT_PER_CPU(var) \
- extern typeof(var) init_per_cpu_var(var)
-
-#ifdef CONFIG_X86_64_SMP
-#define init_per_cpu_var(var) init_per_cpu__##var
-#else
-#define init_per_cpu_var(var) var
-#endif
-
/* For arch-specific code, we can use direct single-insn ops (they
* don't give an lvalue though). */

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 833f747e74ee..1b8c9919afdb 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -335,11 +335,7 @@ SYM_CODE_END(vc_boot_ghcb)
__REFDATA
.balign 8
SYM_DATA(initial_code, .quad x86_64_start_kernel)
-#ifdef CONFIG_SMP
-SYM_DATA(initial_gs, .quad __per_cpu_load)
-#else
SYM_DATA(initial_gs, .quad 0)
-#endif
#ifdef CONFIG_AMD_MEM_ENCRYPT
SYM_DATA(initial_vc_handler, .quad handle_vc_boot_ghcb)
#endif
@@ -572,7 +568,7 @@ SYM_DATA_END(level1_fixmap_pgt)
.align 16

SYM_DATA(early_gdt_descr, .word GDT_ENTRIES*8-1)
-SYM_DATA_LOCAL(early_gdt_descr_base, .quad INIT_PER_CPU_VAR(gdt_page))
+SYM_DATA_LOCAL(early_gdt_descr_base, .quad gdt_page)

.align 16
/* This must match the first entry in level2_kernel_pgt */
diff --git a/arch/x86/kernel/irq_64.c b/arch/x86/kernel/irq_64.c
index 1c0fb96b9e39..df537518983e 100644
--- a/arch/x86/kernel/irq_64.c
+++ b/arch/x86/kernel/irq_64.c
@@ -26,7 +26,6 @@
#include <asm/apic.h>

DEFINE_PER_CPU_PAGE_ALIGNED(struct irq_stack, irq_stack_backing_store) __visible;
-DECLARE_INIT_PER_CPU(irq_stack_backing_store);

#ifdef CONFIG_VMAP_STACK
/*
diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
index 7b65275544b2..a2489b31564e 100644
--- a/arch/x86/kernel/setup_percpu.c
+++ b/arch/x86/kernel/setup_percpu.c
@@ -26,18 +26,10 @@
DEFINE_PER_CPU_READ_MOSTLY(int, cpu_number);
EXPORT_PER_CPU_SYMBOL(cpu_number);

-#ifdef CONFIG_X86_64
-#define BOOT_PERCPU_OFFSET ((unsigned long)__per_cpu_load)
-#else
-#define BOOT_PERCPU_OFFSET 0
-#endif
-
-DEFINE_PER_CPU_READ_MOSTLY(unsigned long, this_cpu_off) = BOOT_PERCPU_OFFSET;
+DEFINE_PER_CPU_READ_MOSTLY(unsigned long, this_cpu_off);
EXPORT_PER_CPU_SYMBOL(this_cpu_off);

-unsigned long __per_cpu_offset[NR_CPUS] __ro_after_init = {
- [0 ... NR_CPUS-1] = BOOT_PERCPU_OFFSET,
-};
+unsigned long __per_cpu_offset[NR_CPUS] __ro_after_init;
EXPORT_SYMBOL(__per_cpu_offset);

/*
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 18a01ece43ee..a72288fa4691 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -103,9 +103,6 @@ PHDRS {
text PT_LOAD FLAGS(5); /* R_E */
data PT_LOAD FLAGS(6); /* RW_ */
#ifdef CONFIG_X86_64
-#ifdef CONFIG_SMP
- percpu PT_LOAD FLAGS(6); /* RW_ */
-#endif
init PT_LOAD FLAGS(7); /* RWE */
#endif
note PT_NOTE FLAGS(0); /* ___ */
@@ -215,17 +212,6 @@ SECTIONS
__init_begin = .; /* paired with __init_end */
}

-#if defined(CONFIG_X86_64) && defined(CONFIG_SMP)
- /*
- * percpu offsets are zero-based on SMP. PERCPU_VADDR() changes the
- * output PHDR, so the next output section - .init.text - should
- * start another segment - init.
- */
- PERCPU_VADDR(INTERNODE_CACHE_BYTES, 0, :percpu)
- ASSERT(SIZEOF(.data..percpu) < CONFIG_PHYSICAL_START,
- "per-CPU data too large - increase CONFIG_PHYSICAL_START")
-#endif
-
INIT_TEXT_SECTION(PAGE_SIZE)
#ifdef CONFIG_X86_64
:init
@@ -339,9 +325,7 @@ SECTIONS
EXIT_DATA
}

-#if !defined(CONFIG_X86_64) || !defined(CONFIG_SMP)
PERCPU_SECTION(INTERNODE_CACHE_BYTES)
-#endif

. = ALIGN(PAGE_SIZE);

@@ -474,17 +458,6 @@ SECTIONS
. = ASSERT((_end - LOAD_OFFSET <= KERNEL_IMAGE_SIZE),
"kernel image bigger than KERNEL_IMAGE_SIZE");

-#ifdef CONFIG_X86_64
-/*
- * Per-cpu symbols which need to be offset from __per_cpu_load
- * for the boot processor.
- */
-#define INIT_PER_CPU(x) init_per_cpu__##x = ABSOLUTE(x) + __per_cpu_load
-INIT_PER_CPU(gdt_page);
-INIT_PER_CPU(irq_stack_backing_store);
-
-#endif /* CONFIG_X86_64 */
-
#ifdef CONFIG_KEXEC_CORE
#include <asm/kexec.h>

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 9b3d51cb2cd1..b27980291ff8 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -83,8 +83,6 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
"__initramfs_start|"
"(jiffies|jiffies_64)|"
#if ELF_BITS == 64
- "__per_cpu_load|"
- "init_per_cpu__.*|"
"__end_rodata_hpage_align|"
#endif
"__vvar_page|"
@@ -280,33 +278,6 @@ static const char *sym_name(const char *sym_strtab, Elf_Sym *sym)
return name;
}

-static Elf_Sym *sym_lookup(const char *symname)
-{
- int i;
- for (i = 0; i < shnum; i++) {
- struct section *sec = &secs[i];
- long nsyms;
- char *strtab;
- Elf_Sym *symtab;
- Elf_Sym *sym;
-
- if (sec->shdr.sh_type != SHT_SYMTAB)
- continue;
-
- nsyms = sec->shdr.sh_size/sizeof(Elf_Sym);
- symtab = sec->symtab;
- strtab = sec->link->strtab;
-
- for (sym = symtab; --nsyms >= 0; sym++) {
- if (!sym->st_name)
- continue;
- if (strcmp(symname, strtab + sym->st_name) == 0)
- return sym;
- }
- }
- return 0;
-}
-
#if BYTE_ORDER == LITTLE_ENDIAN
#define le16_to_cpu(val) (val)
#define le32_to_cpu(val) (val)
@@ -749,80 +720,8 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel,
}
}

-/*
- * The .data..percpu section is a special case for x86_64 SMP kernels.
- * It is used to initialize the actual per_cpu areas and to provide
- * definitions for the per_cpu variables that correspond to their offsets
- * within the percpu area. Since the values of all of the symbols need
- * to be offsets from the start of the per_cpu area the virtual address
- * (sh_addr) of .data..percpu is 0 in SMP kernels.
- *
- * This means that:
- *
- * Relocations that reference symbols in the per_cpu area do not
- * need further relocation (since the value is an offset relative
- * to the start of the per_cpu area that does not change).
- *
- * Relocations that apply to the per_cpu area need to have their
- * offset adjusted by by the value of __per_cpu_load to make them
- * point to the correct place in the loaded image (because the
- * virtual address of .data..percpu is 0).
- *
- * For non SMP kernels .data..percpu is linked as part of the normal
- * kernel data and does not require special treatment.
- *
- */
-static int per_cpu_shndx = -1;
-static Elf_Addr per_cpu_load_addr;
-
-static void percpu_init(void)
-{
- int i;
- for (i = 0; i < shnum; i++) {
- ElfW(Sym) *sym;
- if (strcmp(sec_name(i), ".data..percpu"))
- continue;
-
- if (secs[i].shdr.sh_addr != 0) /* non SMP kernel */
- return;
-
- sym = sym_lookup("__per_cpu_load");
- if (!sym)
- die("can't find __per_cpu_load\n");
-
- per_cpu_shndx = i;
- per_cpu_load_addr = sym->st_value;
- return;
- }
-}
-
#if ELF_BITS == 64

-/*
- * Check to see if a symbol lies in the .data..percpu section.
- *
- * The linker incorrectly associates some symbols with the
- * .data..percpu section so we also need to check the symbol
- * name to make sure that we classify the symbol correctly.
- *
- * The GNU linker incorrectly associates:
- * __init_begin
- * __per_cpu_load
- *
- * The "gold" linker incorrectly associates:
- * init_per_cpu__gdt_page
- */
-static int is_percpu_sym(ElfW(Sym) *sym, const char *symname)
-{
- int shndx = sym_index(sym);
-
- return (shndx == per_cpu_shndx) &&
- strcmp(symname, "__init_begin") &&
- strcmp(symname, "__per_cpu_load") &&
- strncmp(symname, "init_per_cpu_", 13);
-}
-
-
static int do_reloc64(struct section *sec, Elf_Rel *rel, ElfW(Sym) *sym,
const char *symname)
{
@@ -833,47 +732,21 @@ static int do_reloc64(struct section *sec, Elf_Rel *rel, ElfW(Sym) *sym,
if (sym->st_shndx == SHN_UNDEF)
return 0;

- /*
- * Adjust the offset if this reloc applies to the percpu section.
- */
- if (sec->shdr.sh_info == per_cpu_shndx)
- offset += per_cpu_load_addr;
-
switch (r_type) {
case R_X86_64_NONE:
- /* NONE can be ignored. */
- break;
-
case R_X86_64_PC32:
case R_X86_64_PLT32:
- /*
- * PC relative relocations don't need to be adjusted unless
- * referencing a percpu symbol.
- *
- * NB: R_X86_64_PLT32 can be treated as R_X86_64_PC32.
- */
- if (is_percpu_sym(sym, symname))
- add_reloc(&relocs32neg, offset);
- break;
-
case R_X86_64_PC64:
/*
- * Only used by jump labels
+ * NONE can be ignored and PC relative relocations don't need
+ * to be adjusted. Because sym must be defined, R_X86_64_PLT32 can
+ * be treated the same way as R_X86_64_PC32.
*/
- if (is_percpu_sym(sym, symname))
- die("Invalid R_X86_64_PC64 relocation against per-CPU symbol %s\n",
- symname);
break;

case R_X86_64_32:
case R_X86_64_32S:
case R_X86_64_64:
- /*
- * References to the percpu area don't need to be adjusted.
- */
- if (is_percpu_sym(sym, symname))
- break;
-
if (shn_abs) {
/*
* Whitelisted absolute symbols do not require
@@ -1175,8 +1048,6 @@ void process(FILE *fp, int use_real_mode, int as_text,
read_strtabs(fp);
read_symtabs(fp);
read_relocs(fp);
- if (ELF_BITS == 64)
- percpu_init();
if (show_absolute_syms) {
print_absolute_symbols();
return;
diff --git a/init/Kconfig b/init/Kconfig
index 41a728debdbd..943552dc9c19 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1714,7 +1714,6 @@ config KALLSYMS_ALL
config KALLSYMS_ABSOLUTE_PERCPU
bool
depends on KALLSYMS
- default X86_64 && SMP

config KALLSYMS_BASE_RELATIVE
bool
--
2.31.1


2021-11-30 20:59:40

by Brian Gerst

[permalink] [raw]
Subject: [PATCH v2 1/6] x86: Remove stack protector test scripts

For 64-bit, this test for the stack protector was added in 2006 to
make sure the compiler had the PR28281 patch applied. With GCC 5.1
being the minimum supported compiler now, this test is no longer
necessary.

For 32-bit, testing for compiler option support can be done directly
in Kconfig.

Signed-off-by: Brian Gerst <[email protected]>
---
arch/x86/Kconfig | 7 +++----
scripts/gcc-x86_32-has-stack-protector.sh | 8 --------
scripts/gcc-x86_64-has-stack-protector.sh | 4 ----
3 files changed, 3 insertions(+), 16 deletions(-)
delete mode 100755 scripts/gcc-x86_32-has-stack-protector.sh
delete mode 100755 scripts/gcc-x86_64-has-stack-protector.sh

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 8ef4c24cc569..82ed7fdddc75 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -392,12 +392,11 @@ config PGTABLE_LEVELS

config CC_HAS_SANE_STACKPROTECTOR
bool
- default $(success,$(srctree)/scripts/gcc-x86_64-has-stack-protector.sh $(CC)) if 64BIT
- default $(success,$(srctree)/scripts/gcc-x86_32-has-stack-protector.sh $(CC))
+ default y if 64BIT
+ default $(cc-option,-mstack-protector-guard-reg=fs -mstack-protector-guard-symbol=__stack_chk_guard)
help
We have to make sure stack protector is unconditionally disabled if
- the compiler produces broken code or if it does not let us control
- the segment on 32-bit kernels.
+ the compiler does not let us control the segment and symbol.

menu "Processor type and features"

diff --git a/scripts/gcc-x86_32-has-stack-protector.sh b/scripts/gcc-x86_32-has-stack-protector.sh
deleted file mode 100755
index 825c75c5b715..000000000000
--- a/scripts/gcc-x86_32-has-stack-protector.sh
+++ /dev/null
@@ -1,8 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-
-# This requires GCC 8.1 or better. Specifically, we require
-# -mstack-protector-guard-reg, added by
-# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81708
-
-echo "int foo(void) { char X[200]; return 3; }" | $* -S -x c -c -m32 -O0 -fstack-protector -mstack-protector-guard-reg=fs -mstack-protector-guard-symbol=__stack_chk_guard - -o - 2> /dev/null | grep -q "%fs"
diff --git a/scripts/gcc-x86_64-has-stack-protector.sh b/scripts/gcc-x86_64-has-stack-protector.sh
deleted file mode 100755
index 75e4e22b986a..000000000000
--- a/scripts/gcc-x86_64-has-stack-protector.sh
+++ /dev/null
@@ -1,4 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-
-echo "int foo(void) { char X[200]; return 3; }" | $* -S -x c -c -m64 -O0 -mcmodel=kernel -fno-PIE -fstack-protector - -o - 2> /dev/null | grep -q "%gs"
--
2.31.1


2021-11-30 21:00:06

by Brian Gerst

[permalink] [raw]
Subject: [PATCH v2 2/6] x86-64: Convert stack protector to normal percpu 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
virtual 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.

Since version 8.1, GCC has options to configure the location of the
canary value. This allows the canary to be turned into a normal
percpu variable and removes the constraint that the percpu section
be zero-based.

Signed-off-by: Brian Gerst <[email protected]>
---
arch/x86/Kconfig | 2 +-
arch/x86/Makefile | 19 ++++++++------
arch/x86/entry/entry_64.S | 2 +-
arch/x86/include/asm/processor.h | 25 ++++---------------
arch/x86/include/asm/stackprotector.h | 36 ++++++---------------------
arch/x86/kernel/asm-offsets_64.c | 6 -----
arch/x86/kernel/cpu/common.c | 8 ++----
arch/x86/kernel/head_64.S | 12 +++++----
arch/x86/kernel/vmlinux.lds.S | 6 -----
arch/x86/tools/relocs.c | 1 -
arch/x86/xen/xen-head.S | 10 +++-----
11 files changed, 39 insertions(+), 88 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 82ed7fdddc75..a8177ece6a99 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -392,7 +392,7 @@ config PGTABLE_LEVELS

config CC_HAS_SANE_STACKPROTECTOR
bool
- default y if 64BIT
+ default $(cc-option,-mstack-protector-guard-reg=gs -mstack-protector-guard-symbol=__stack_chk_guard) if 64BIT
default $(cc-option,-mstack-protector-guard-reg=fs -mstack-protector-guard-symbol=__stack_chk_guard)
help
We have to make sure stack protector is unconditionally disabled if
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 42243869216d..f25ba649abf8 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -79,13 +79,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=__stack_chk_guard
- else
- KBUILD_CFLAGS += -mstack-protector-guard=global
- endif
- endif
+ percpu_seg := fs
else
BITS := 64
UTS_MACHINE := x86_64
@@ -126,6 +120,17 @@ else

KBUILD_CFLAGS += -mno-red-zone
KBUILD_CFLAGS += -mcmodel=kernel
+
+ percpu_seg := gs
+endif
+
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+ ifeq ($(CONFIG_SMP),y)
+ KBUILD_CFLAGS += -mstack-protector-guard-reg=$(percpu_seg) \
+ -mstack-protector-guard-symbol=__stack_chk_guard
+ else
+ KBUILD_CFLAGS += -mstack-protector-guard=global
+ endif
endif

ifdef CONFIG_X86_X32
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index e38a4cf795d9..7bfa3b99b5d9 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -240,7 +240,7 @@ SYM_FUNC_START(__switch_to_asm)

#ifdef CONFIG_STACKPROTECTOR
movq TASK_stack_canary(%rsi), %rbx
- movq %rbx, PER_CPU_VAR(fixed_percpu_data) + stack_canary_offset
+ movq %rbx, PER_CPU_VAR(__stack_chk_guard)
#endif

#ifdef CONFIG_RETPOLINE
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 2c5f12ae7d04..a63cb7680bcb 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -427,25 +427,13 @@ struct irq_stack {
DECLARE_PER_CPU(unsigned long, cpu_current_top_of_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;
-};
-
-DECLARE_PER_CPU_FIRST(struct fixed_percpu_data, fixed_percpu_data) __visible;
-DECLARE_INIT_PER_CPU(fixed_percpu_data);
-
static inline unsigned long cpu_kernelmode_gs_base(int cpu)
{
- return (unsigned long)per_cpu(fixed_percpu_data.gs_base, cpu);
+#ifdef CONFIG_SMP
+ return per_cpu_offset(cpu);
+#else
+ return 0;
+#endif
}

DECLARE_PER_CPU(void *, hardirq_stack_ptr);
@@ -455,9 +443,6 @@ extern asmlinkage void ignore_sysret(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
DECLARE_PER_CPU(struct irq_stack *, hardirq_stack_ptr);
DECLARE_PER_CPU(struct irq_stack *, softirq_stack_ptr);
#endif /* !X86_64 */
diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
index 24a8d6c4fb18..6a755a5c4fec 100644
--- a/arch/x86/include/asm/stackprotector.h
+++ b/arch/x86/include/asm/stackprotector.h
@@ -2,26 +2,13 @@
/*
* 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.
+ * returning from the function. The pattern is called the stack canary
+ * and is a unique value for each task.
*
- * 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.
+ * GCC is configured to read the stack canary value from the __stack_chk_guard
+ * per-cpu variable, which is changed on task switch.
*/

#ifndef _ASM_STACKPROTECTOR_H
@@ -37,6 +24,8 @@
#include <linux/random.h>
#include <linux/sched.h>

+DECLARE_PER_CPU(unsigned long, __stack_chk_guard);
+
/*
* Initialize the stackprotector canary value.
*
@@ -53,9 +42,6 @@ static __always_inline void boot_init_stack_canary(void)
u64 canary;
u64 tsc;

-#ifdef CONFIG_X86_64
- BUILD_BUG_ON(offsetof(struct fixed_percpu_data, stack_canary) != 40);
-#endif
/*
* We both use the random pool and the current TSC as a source
* of randomness. The TSC only matters for very early init,
@@ -68,20 +54,12 @@ static __always_inline void boot_init_stack_canary(void)
canary &= CANARY_MASK;

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 b14533af7676..985dc3455993 100644
--- 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
- DEFINE(stack_canary_offset, offsetof(struct 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 0663642d6199..0462de9b9921 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1767,10 +1767,6 @@ static __init int setup_clearcpuid(char *arg)
__setup("clearcpuid=", setup_clearcpuid);

#ifdef CONFIG_X86_64
-DEFINE_PER_CPU_FIRST(struct fixed_percpu_data,
- fixed_percpu_data) __aligned(PAGE_SIZE) __visible;
-EXPORT_PER_CPU_SYMBOL_GPL(fixed_percpu_data);
-
/*
* The following percpu variables are hot. Align current_task to
* cacheline size such that they fall in the same cacheline.
@@ -1851,13 +1847,13 @@ DEFINE_PER_CPU(unsigned long, cpu_current_top_of_stack) =
(unsigned long)&init_thread_union + THREAD_SIZE;
EXPORT_PER_CPU_SYMBOL(cpu_current_top_of_stack);

+#endif /* CONFIG_X86_64 */
+
#ifdef CONFIG_STACKPROTECTOR
DEFINE_PER_CPU(unsigned long, __stack_chk_guard);
EXPORT_PER_CPU_SYMBOL(__stack_chk_guard);
#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 d8b3ebd2bb85..833f747e74ee 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -198,10 +198,8 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
movl %eax,%fs
movl %eax,%gs

- /* 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.
+ /*
+ * Set up GS base.
* Note that, on SMP, the boot cpu uses init data section until
* the per cpu areas are set up.
*/
@@ -337,7 +335,11 @@ SYM_CODE_END(vc_boot_ghcb)
__REFDATA
.balign 8
SYM_DATA(initial_code, .quad x86_64_start_kernel)
-SYM_DATA(initial_gs, .quad INIT_PER_CPU_VAR(fixed_percpu_data))
+#ifdef CONFIG_SMP
+SYM_DATA(initial_gs, .quad __per_cpu_load)
+#else
+SYM_DATA(initial_gs, .quad 0)
+#endif
#ifdef CONFIG_AMD_MEM_ENCRYPT
SYM_DATA(initial_vc_handler, .quad handle_vc_boot_ghcb)
#endif
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 3d6dc12d198f..18a01ece43ee 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -481,14 +481,8 @@ SECTIONS
*/
#define INIT_PER_CPU(x) init_per_cpu__##x = ABSOLUTE(x) + __per_cpu_load
INIT_PER_CPU(gdt_page);
-INIT_PER_CPU(fixed_percpu_data);
INIT_PER_CPU(irq_stack_backing_store);

-#ifdef CONFIG_SMP
-. = ASSERT((fixed_percpu_data == 0),
- "fixed_percpu_data is not at start of per-cpu area");
-#endif
-
#endif /* CONFIG_X86_64 */

#ifdef CONFIG_KEXEC_CORE
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index c736cf2ac76b..9b3d51cb2cd1 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -810,7 +810,6 @@ static void percpu_init(void)
* __per_cpu_load
*
* The "gold" linker incorrectly associates:
- * init_per_cpu__fixed_percpu_data
* init_per_cpu__gdt_page
*/
static int is_percpu_sym(ElfW(Sym) *sym, const char *symname)
diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index 6a64496edefb..ba738835db11 100644
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -54,16 +54,14 @@ SYM_CODE_START(startup_xen)
mov %rsi, xen_start_info
mov initial_stack(%rip), %rsp

- /* 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.
+ /*
+ * Set up GS base.
* Note that, on SMP, the boot cpu uses init data section until
* the per cpu areas are set up.
*/
movl $MSR_GS_BASE,%ecx
- movq $INIT_PER_CPU_VAR(fixed_percpu_data),%rax
- cdq
+ movl initial_gs(%rip),%eax
+ movl initial_gs+4(%rip),%edx
wrmsr

call xen_start_kernel
--
2.31.1


2021-12-01 09:51:19

by David Laight

[permalink] [raw]
Subject: RE: [PATCH v2 2/6] x86-64: Convert stack protector to normal percpu variable

From: Brian Gerst
> Sent: 30 November 2021 20:56
>
> 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
> virtual 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.
>
> Since version 8.1, GCC has options to configure the location of the
> canary value. This allows the canary to be turned into a normal
> percpu variable and removes the constraint that the percpu section
> be zero-based.

I didn't think the minimum gcc version has been raised as far as 8.1?

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


2021-12-01 14:21:41

by Brian Gerst

[permalink] [raw]
Subject: Re: [PATCH v2 2/6] x86-64: Convert stack protector to normal percpu variable

On Wed, Dec 1, 2021 at 4:50 AM David Laight <[email protected]> wrote:
>
> From: Brian Gerst
> > Sent: 30 November 2021 20:56
> >
> > 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
> > virtual 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.
> >
> > Since version 8.1, GCC has options to configure the location of the
> > canary value. This allows the canary to be turned into a normal
> > percpu variable and removes the constraint that the percpu section
> > be zero-based.
>
> I didn't think the minimum gcc version has been raised as far as 8.1?

The first version of this patchset retained compatibility with older
compilers, but there were objections to the increased code size to
support both versions. x86-32 already dropped support for stack
protector on older compilers, so v2 of this patchset does the same.
The benefit of removing the fixed location version is all of the
supporting code that can be removed when the stack protector becomes a
standard percpu variable.

--
Brian Gerst

2021-12-02 22:51:24

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v2 2/6] x86-64: Convert stack protector to normal percpu variable

On Wed, Dec 01, 2021 at 09:50:57AM +0000, David Laight wrote:
> From: Brian Gerst
> > Sent: 30 November 2021 20:56
> >
> > 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
> > virtual 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.
> >
> > Since version 8.1, GCC has options to configure the location of the
> > canary value. This allows the canary to be turned into a normal
> > percpu variable and removes the constraint that the percpu section
> > be zero-based.
>
> I didn't think the minimum gcc version has been raised as far as 8.1?

Older GCC can still build a kernel, just not with stack protector on.
And 8.1 is already 3 years old. If you run ancient distros, you can run
ancient kernels too.