Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1494129 > unrolled thread

[PATCH 0/2] powerpc: stack protector (-fstack-protector) support

Started byChristophe Leroy <christophe.leroy@c-s.fr>
First post2016-09-30 16:30 +0200
Last post2016-10-01 01:20 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Christophe Leroy <christophe.leroy@c-s.fr> - 2016-09-30 16:30 +0200
    [PATCH 2/2] powerpc/32: stack protector: change the canary value per  task Christophe Leroy <christophe.leroy@c-s.fr> - 2016-09-30 16:30 +0200
    [PATCH 1/2] powerpc: initial stack protector (-fstack-protector)  support Christophe Leroy <christophe.leroy@c-s.fr> - 2016-09-30 16:30 +0200
    Re: [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Christophe LEROY <christophe.leroy@c-s.fr> - 2016-09-30 18:40 +0200
      Re: [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2016-10-01 01:20 +0200

#1494129 — [PATCH 0/2] powerpc: stack protector (-fstack-protector) support

FromChristophe Leroy <christophe.leroy@c-s.fr>
Date2016-09-30 16:30 +0200
Subject[PATCH 0/2] powerpc: stack protector (-fstack-protector) support
Message-ID<sn6Z4-2UA-19@gated-at.bofh.it>
Add HAVE_CC_STACKPROTECTOR to powerpc. This is copied from ARM.

Not tested on PPC64, compile ok with ppc64_defconfig

Christophe Leroy (2):
  powerpc: initial stack protector (-fstack-protector) support
  powerpc/32: stack protector: change the canary value per task

 arch/powerpc/Kconfig                      |  1 +
 arch/powerpc/include/asm/stackprotector.h | 38 +++++++++++++++++++++++++++++++
 arch/powerpc/kernel/Makefile              |  5 ++++
 arch/powerpc/kernel/asm-offsets.c         |  3 +++
 arch/powerpc/kernel/entry_32.S            |  6 ++++-
 arch/powerpc/kernel/process.c             |  6 +++++
 6 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/include/asm/stackprotector.h

-- 
2.1.0

[toc] | [next] | [standalone]


#1494132 — [PATCH 2/2] powerpc/32: stack protector: change the canary value per task

FromChristophe Leroy <christophe.leroy@c-s.fr>
Date2016-09-30 16:30 +0200
Subject[PATCH 2/2] powerpc/32: stack protector: change the canary value per task
Message-ID<sn6Z4-2UA-27@gated-at.bofh.it>
In reply to#1494129
Partially copied from commit df0698be14c66 ("ARM: stack protector:
change the canary value per task")

A new random value for the canary is stored in the task struct whenever
a new task is forked.  This is meant to allow for different canary values
per task.  On powerpc, GCC expects the canary value to be found in a global
variable called __stack_chk_guard.  So this variable has to be updated
with the value stored in the task struct whenever a task switch occurs.

Because the variable GCC expects is global, this cannot work on SMP
unfortunately.  So, on SMP, the same initial canary value is kept
throughout, making this feature a bit less effective although it is still
useful.

Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/asm-offsets.c | 3 +++
 arch/powerpc/kernel/entry_32.S    | 6 +++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index a51ae9b..ede2fc4 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -91,6 +91,9 @@ int main(void)
 	DEFINE(TI_livepatch_sp, offsetof(struct thread_info, livepatch_sp));
 #endif
 
+#ifdef CONFIG_CC_STACKPROTECTOR
+	DEFINE(TSK_STACK_CANARY, offsetof(struct task_struct, stack_canary));
+#endif
 	DEFINE(KSP, offsetof(struct thread_struct, ksp));
 	DEFINE(PT_REGS, offsetof(struct thread_struct, regs));
 #ifdef CONFIG_BOOKE
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 3841d74..5742dbd 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -674,7 +674,11 @@ BEGIN_FTR_SECTION
 	mtspr	SPRN_SPEFSCR,r0		/* restore SPEFSCR reg */
 END_FTR_SECTION_IFSET(CPU_FTR_SPE)
 #endif /* CONFIG_SPE */
-
+#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
+	lwz	r0,TSK_STACK_CANARY(r2)
+	lis	r4,__stack_chk_guard@ha
+	stw	r0,__stack_chk_guard@l(r4)
+#endif
 	lwz	r0,_CCR(r1)
 	mtcrf	0xFF,r0
 	/* r3-r12 are destroyed -- Cort */
-- 
2.1.0

[toc] | [prev] | [next] | [standalone]


#1494137 — [PATCH 1/2] powerpc: initial stack protector (-fstack-protector) support

FromChristophe Leroy <christophe.leroy@c-s.fr>
Date2016-09-30 16:30 +0200
Subject[PATCH 1/2] powerpc: initial stack protector (-fstack-protector) support
Message-ID<sn6Z4-2UA-35@gated-at.bofh.it>
In reply to#1494129
Partialy copied from commit c743f38013aef ("ARM: initial stack protector
(-fstack-protector) support")

This is the very basic stuff without the changing canary upon
task switch yet.  Just the Kconfig option and a constant canary
value initialized at boot time.

Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/Kconfig                      |  1 +
 arch/powerpc/include/asm/stackprotector.h | 38 +++++++++++++++++++++++++++++++
 arch/powerpc/kernel/Makefile              |  5 ++++
 arch/powerpc/kernel/process.c             |  6 +++++
 4 files changed, 50 insertions(+)
 create mode 100644 arch/powerpc/include/asm/stackprotector.h

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 59e53f4..2947dab 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -162,6 +162,7 @@ config PPC
 	select HAVE_VIRT_CPU_ACCOUNTING
 	select HAVE_ARCH_HARDENED_USERCOPY
 	select HAVE_KERNEL_GZIP
+	select HAVE_CC_STACKPROTECTOR
 
 config GENERIC_CSUM
 	def_bool CPU_LITTLE_ENDIAN
diff --git a/arch/powerpc/include/asm/stackprotector.h b/arch/powerpc/include/asm/stackprotector.h
new file mode 100644
index 0000000..de00332
--- /dev/null
+++ b/arch/powerpc/include/asm/stackprotector.h
@@ -0,0 +1,38 @@
+/*
+ * GCC stack protector support.
+ *
+ * Stack protector works by putting 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 gcc expects it to be defined by a global variable called
+ * "__stack_chk_guard" on ARM.  This unfortunately means that on SMP
+ * we cannot have a different canary value per task.
+ */
+
+#ifndef _ASM_STACKPROTECTOR_H
+#define _ASM_STACKPROTECTOR_H 1
+
+#include <linux/random.h>
+#include <linux/version.h>
+
+extern unsigned long __stack_chk_guard;
+
+/*
+ * Initialize the stackprotector canary value.
+ *
+ * NOTE: this must only be called from functions that never return,
+ * and it must always be inlined.
+ */
+static __always_inline void boot_init_stack_canary(void)
+{
+	unsigned long canary;
+
+	/* Try to get a semi random initial value. */
+	get_random_bytes(&canary, sizeof(canary));
+	canary ^= LINUX_VERSION_CODE;
+
+	current->stack_canary = canary;
+	__stack_chk_guard = current->stack_canary;
+}
+
+#endif	/* _ASM_STACKPROTECTOR_H */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index e59ed6a..4a62179 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -19,6 +19,11 @@ CFLAGS_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 
+# -fstack-protector triggers protection checks in this code,
+# but it is being used too early to link to meaningful stack_chk logic.
+nossp_flags := $(call cc-option, -fno-stack-protector)
+CFLAGS_prom_init.o := $(nossp_flags)
+
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
 CFLAGS_REMOVE_cputable.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index ce8a26a..ba8f32a 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -64,6 +64,12 @@
 #include <linux/kprobes.h>
 #include <linux/kdebug.h>
 
+#ifdef CONFIG_CC_STACKPROTECTOR
+#include <linux/stackprotector.h>
+unsigned long __stack_chk_guard __read_mostly;
+EXPORT_SYMBOL(__stack_chk_guard);
+#endif
+
 /* Transactional Memory debug */
 #ifdef TM_DEBUG_SW
 #define TM_DEBUG(x...) printk(KERN_INFO x)
-- 
2.1.0

[toc] | [prev] | [next] | [standalone]


#1494195

FromChristophe LEROY <christophe.leroy@c-s.fr>
Date2016-09-30 18:40 +0200
Message-ID<sn90R-4l6-3@gated-at.bofh.it>
In reply to#1494129

Le 30/09/2016 à 18:26, Denis Kirjanov a écrit :
>
>
> On Friday, September 30, 2016, Christophe Leroy <christophe.leroy@c-s.fr
> <mailto:christophe.leroy@c-s.fr>> wrote:
>
>     Add HAVE_CC_STACKPROTECTOR to powerpc. This is copied from ARM.
>
>     Not tested on PPC64, compile ok with ppc64_
>
>
> Hi Christophe,
>
> are you going to test it on ppc64? If not, I can take it

Thanks Denis, you are welcome to test it.

I don't have any ppc64 target, I only have mpc8xx and mpc83xx which both 
are ppc32

Christophe

>
>
>
>     Christophe Leroy (2):
>       powerpc: initial stack protector (-fstack-protector) support
>       powerpc/32: stack protector: change the canary value per task
>
>      arch/powerpc/Kconfig                      |  1 +
>      arch/powerpc/include/asm/stackprotector.h | 38
>     +++++++++++++++++++++++++++++++
>      arch/powerpc/kernel/Makefile              |  5 ++++
>      arch/powerpc/kernel/asm-offsets.c         |  3 +++
>      arch/powerpc/kernel/entry_32.S            |  6 ++++-
>      arch/powerpc/kernel/process.c             |  6 +++++
>      6 files changed, 58 insertions(+), 1 deletion(-)
>      create mode 100644 arch/powerpc/include/asm/stackprotector.h
>
>     --
>     2.1.0
>

[toc] | [prev] | [next] | [standalone]


#1494323

FromBenjamin Herrenschmidt <benh@kernel.crashing.org>
Date2016-10-01 01:20 +0200
Message-ID<snffY-aa-17@gated-at.bofh.it>
In reply to#1494195
On Fri, 2016-09-30 at 18:38 +0200, Christophe LEROY wrote:
> I don't have any ppc64 target, I only have mpc8xx and mpc83xx which
> both 
> are ppc32

That's what qemu is for :-)

Cheers,
Ben.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web