[Skiboot] [RFC PATCH] Test new gcc stack protector option

Benjamin Herrenschmidt benh at kernel.crashing.org
Wed Nov 22 10:27:35 AEDT 2017


This tentative patch exploits the new gcc parametrized stack
protector options, courtesy of Segher, currently available
in gcc 7.x.

Tested in simics.

Not-yet-signed-off-by...
---

diff --git a/Makefile.main b/Makefile.main
index 73c91962..83576db1 100644
--- a/Makefile.main
+++ b/Makefile.main
@@ -19,6 +19,7 @@ try = $(shell set -e; if ($(1)) >/dev/null 2>&1; \
 	else echo "$(3)"; fi )
 
 try-cflag = $(call try,$(1) $(2) -x c -c /dev/null -o /dev/null,$(2))
+test_cflag = $(call try,$(1) $(2) -x c -c /dev/null -o /dev/null,1,0)
 
 # Base warnings
 CWARNS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
@@ -84,20 +85,33 @@ ifeq ($(SKIBOOT_GCOV),1)
 CFLAGS += -fprofile-arcs -ftest-coverage -DSKIBOOT_GCOV=1
 endif
 
+# Check if the new parametrized stack protector option is supported
+# by gcc, otherwise disable stack protector
+STACK_PROT_CFLAGS := -mstack-protector-guard=tls -mstack-protector-guard-reg=%r13
+STACK_PROT_CFLAGS += -mstack-protector-guard-offset=0
+HAS_STACK_PROT := $(call test_cflag,$(CC),$(STACK_PROT_CFLAGS))
+
 # Stack protector disabled for now. gcc tends to use the TLS to
 # access the canary (depending on how gcc was built), and this won't
 # work for us.
 #
 ifeq ($(STACK_CHECK),1)
-#CFLAGS += -fstack-protector-all -pg
-CFLAGS += -fno-stack-protector -pg
+STACK_PROT_CFLAGS += -fstack-protector-all
 CPPFLAGS += -DSTACK_CHECK_ENABLED
+CFLAGS += -pg
+else
+STACK_PROT_CFLAGS += -fstack-protector
+STACK_PROT_CFLAGS += $(call try-cflag,$(CC),-fstack-protector-strong)
+endif
+
+ifeq ($(HAS_STACK_PROT),1)
+CPPFLAGS += -DHAS_STACK_PROT
+CFLAGS += $(STACK_PROT_CFLAGS)
 else
 CFLAGS += -fno-stack-protector
-#CFLAGS += -fstack-protector
-#CFLAGS += $(call try-cflag,$(CC),-fstack-protector-strong)
 endif
 
+
 CFLAGS += $(call try-cflag,$(CC),-Wjump-misses-init) \
 	  $(call try-cflag,$(CC),-Wsuggest-attribute=const) \
 	  $(call try-cflag,$(CC),-Wsuggest-attribute=noreturn) \
diff --git a/asm/head.S b/asm/head.S
index ccf09482..444907a3 100644
--- a/asm/head.S
+++ b/asm/head.S
@@ -389,6 +389,11 @@ boot_entry:
 	li	%r0,0
 	std	%r0,CPUTHREAD_STACK_BOT_MARK(%r13)
 #endif
+	/* Initialize the stack guard */
+	LOAD_IMM64(%r3,STACK_CHECK_GUARD_BASE);
+	xor	%r3,%r3,%r31
+	std	%r3,0(%r13)
+
 	/* Jump to C */
 	mr	%r3,%r27
 	bl	main_cpu_entry
diff --git a/core/cpu.c b/core/cpu.c
index 27e0d6cf..cf14d267 100644
--- a/core/cpu.c
+++ b/core/cpu.c
@@ -794,6 +794,7 @@ static void init_cpu_thread(struct cpu_thread *t,
 	init_lock(&t->dctl_lock);
 	init_lock(&t->job_lock);
 	list_head_init(&t->job_queue);
+	t->stack_guard = STACK_CHECK_GUARD_BASE ^ pir;
 	t->state = state;
 	t->pir = pir;
 #ifdef STACK_CHECK_ENABLED
@@ -837,7 +838,8 @@ void __nomcount pre_init_boot_cpu(void)
 {
 	struct cpu_thread *cpu = this_cpu();
 
-	memset(cpu, 0, sizeof(struct cpu_thread));
+	/* We skip the stack guard ! */
+	memset(((void *)cpu) + 8, 0, sizeof(struct cpu_thread) - 8);
 }
 
 void init_boot_cpu(void)
@@ -915,8 +917,14 @@ void init_boot_cpu(void)
 	top_of_ram += (cpu_max_pir + 1) * STACK_SIZE;
 
 	/* Clear the CPU structs */
-	for (i = 0; i <= cpu_max_pir; i++)
+	for (i = 0; i <= cpu_max_pir; i++) {
+		/* boot CPU already cleared and we don't want to clobber
+		 * its stack guard value.
+		 */
+		if (i == pir)
+			continue;
 		memset(&cpu_stacks[i].cpu, 0, sizeof(struct cpu_thread));
+	}
 
 	/* Setup boot CPU state */
 	boot_cpu = &cpu_stacks[pir].cpu;
diff --git a/core/utils.c b/core/utils.c
index d21881ef..9e2cb37c 100644
--- a/core/utils.c
+++ b/core/utils.c
@@ -22,9 +22,6 @@
 #include <cpu.h>
 #include <stack.h>
 
-extern unsigned long __stack_chk_guard;
-unsigned long __stack_chk_guard = 0xdeadf00dbaad300dULL;
-
 void __noreturn assert_fail(const char *msg)
 {
 	/**
diff --git a/include/cpu.h b/include/cpu.h
index 168fa994..0b3c37e8 100644
--- a/include/cpu.h
+++ b/include/cpu.h
@@ -44,6 +44,11 @@ struct cpu_job;
 struct xive_cpu_state;
 
 struct cpu_thread {
+	/*
+	 * "stack_guard" must be at offset 0 to match the
+	 * -mstack-protector-guard-offset=0 statement in the Makefile
+	 */
+	uint64_t			stack_guard;
 	uint32_t			pir;
 	uint32_t			server_no;
 	uint32_t			chip_id;
diff --git a/include/stack.h b/include/stack.h
index 3c9799b1..e50b34f0 100644
--- a/include/stack.h
+++ b/include/stack.h
@@ -44,6 +44,8 @@
  */
 #define STACK_WARNING_GAP	2048
 
+#define STACK_CHECK_GUARD_BASE	0xdeadf00dbaad300
+
 #ifndef __ASSEMBLY__
 
 #include <stdint.h>


More information about the Skiboot mailing list