linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] powerpc/32: fix handling of stack protector with recent GCC
@ 2017-01-16  9:00 Christophe Leroy
  2017-01-19  3:36 ` Christian Kujau
  2017-01-31  0:12 ` Christian Kujau
  0 siblings, 2 replies; 5+ messages in thread
From: Christophe Leroy @ 2017-01-16  9:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Scott Wood, Christian Kujau
  Cc: linux-kernel, linuxppc-dev

Since 2005, powerpc GCC doesn't manage anymore __stack_chk_guard as
a global variable but as some value located at -0x7008(r2)

In the Linux kernel, r2 is used as a pointer to current task struct.

This patch changes the meaning of r2 when stack protector
is activated:
- current is taken from thread_info and not kept in r2 anymore
- r2 is set to current + offset of stack canary + 0x7008 so
that -0x7008(r2) directly points to current->stack_canary

current could have been more efficiently calculated from r2
but some circular inclusion prevent inserting struct task_struct
into arch/powerpc/include/asm/current.h so it is not possible
to get offset of stack_canary within current task_struct from there.

fixes: 6533b7c16ee57 ("powerpc: Initial stack protector
(-fstack-protector) support")
Reported-by: Christian Kujau <lists@nerdbynature.de>

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 Christian, can you test it ?

 arch/powerpc/include/asm/current.h        | 12 +++++++++++-
 arch/powerpc/include/asm/stackprotector.h | 13 +++++++++----
 arch/powerpc/kernel/entry_32.S            | 19 +++++++++++++++----
 arch/powerpc/kernel/head_32.S             |  7 +++++++
 arch/powerpc/kernel/head_40x.S            |  4 ++++
 arch/powerpc/kernel/head_44x.S            |  4 ++++
 arch/powerpc/kernel/head_8xx.S            |  4 ++++
 arch/powerpc/kernel/head_fsl_booke.S      |  7 +++++++
 arch/powerpc/kernel/process.c             |  6 ------
 9 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/current.h b/arch/powerpc/include/asm/current.h
index e2c7f06..2f67f02 100644
--- a/arch/powerpc/include/asm/current.h
+++ b/arch/powerpc/include/asm/current.h
@@ -27,8 +27,16 @@ static inline struct task_struct *get_current(void)
 }
 #define current	get_current()
 
-#else
+#else /* __powerpc64__ */
+#if defined(CONFIG_CC_STACKPROTECTOR)
+#include <linux/thread_info.h>
 
+static inline struct task_struct *get_current(void)
+{
+	return current_thread_info()->task;
+}
+#define current	get_current()
+#else
 /*
  * We keep `current' in r2 for speed.
  */
@@ -36,5 +44,7 @@ register struct task_struct *current asm ("r2");
 
 #endif
 
+#endif /* __powerpc64__ */
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_CURRENT_H */
diff --git a/arch/powerpc/include/asm/stackprotector.h b/arch/powerpc/include/asm/stackprotector.h
index 6720190..bf30509 100644
--- a/arch/powerpc/include/asm/stackprotector.h
+++ b/arch/powerpc/include/asm/stackprotector.h
@@ -12,12 +12,18 @@
 #ifndef _ASM_STACKPROTECTOR_H
 #define _ASM_STACKPROTECTOR_H
 
+#ifdef CONFIG_PPC64
+#define SSP_OFFSET	0x7010
+#else
+#define SSP_OFFSET	0x7008
+#endif
+
+#ifndef __ASSEMBLY__
+
 #include <linux/random.h>
 #include <linux/version.h>
 #include <asm/reg.h>
 
-extern unsigned long __stack_chk_guard;
-
 /*
  * Initialize the stackprotector canary value.
  *
@@ -34,7 +40,6 @@ static __always_inline void boot_init_stack_canary(void)
 	canary ^= LINUX_VERSION_CODE;
 
 	current->stack_canary = canary;
-	__stack_chk_guard = current->stack_canary;
 }
-
+#endif /* __ASSEMBLY__ */
 #endif	/* _ASM_STACKPROTECTOR_H */
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 5742dbd..b3a363c 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -34,6 +34,7 @@
 #include <asm/ftrace.h>
 #include <asm/ptrace.h>
 #include <asm/export.h>
+#include <asm/stackprotector.h>
 
 /*
  * MSR_KERNEL is > 0x10000 on 4xx/Book-E since it include MSR_CE.
@@ -149,6 +150,9 @@ transfer_to_handler:
 	mfspr	r12,SPRN_SPRG_THREAD
 	addi	r2,r12,-THREAD
 	tovirt(r2,r2)			/* set r2 to current */
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 	beq	2f			/* if from user, fix up THREAD.regs */
 	addi	r11,r1,STACK_FRAME_OVERHEAD
 	stw	r11,PT_REGS(r12)
@@ -385,6 +389,9 @@ syscall_exit_cont:
 	lwz	r3,GPR3(r1)
 1:
 #endif /* CONFIG_TRACE_IRQFLAGS */
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	subi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
 	/* If the process has its own DBCR0 value, load it up.  The internal
 	   debug mode bit tells us that dbcr0 should be loaded. */
@@ -617,6 +624,9 @@ _GLOBAL(_switch)
 	stwu	r1,-INT_FRAME_SIZE(r1)
 	mflr	r0
 	stw	r0,INT_FRAME_SIZE+4(r1)
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	subi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 	/* r3-r12 are caller saved -- Cort */
 	SAVE_NVGPRS(r1)
 	stw	r0,_NIP(r1)	/* Return to switch caller */
@@ -674,10 +684,8 @@ 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)
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
 #endif
 	lwz	r0,_CCR(r1)
 	mtcrf	0xFF,r0
@@ -779,6 +787,9 @@ user_exc_return:		/* r10 contains MSR_KERNEL here */
 	bne	do_work
 
 restore_user:
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	subi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
 	/* Check whether this process has its own DBCR0 value.  The internal
 	   debug mode bit tells us that dbcr0 should be loaded. */
diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 9d96354..bae9b83 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -35,6 +35,7 @@
 #include <asm/bug.h>
 #include <asm/kvm_book3s_asm.h>
 #include <asm/export.h>
+#include <asm/stackprotector.h>
 
 /* 601 only have IBAT; cr0.eq is set on 601 when using this macro */
 #define LOAD_BAT(n, reg, RA, RB)	\
@@ -864,6 +865,9 @@ __secondary_start:
 	tophys(r4,r2)
 	addi	r4,r4,THREAD	/* phys address of our thread_struct */
 	mtspr	SPRN_SPRG_THREAD,r4
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 	li	r3,0
 	mtspr	SPRN_SPRG_RTAS,r3	/* 0 => not in RTAS */
 
@@ -950,6 +954,9 @@ start_here:
 	tophys(r4,r2)
 	addi	r4,r4,THREAD	/* init task's THREAD */
 	mtspr	SPRN_SPRG_THREAD,r4
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 	li	r3,0
 	mtspr	SPRN_SPRG_RTAS,r3	/* 0 => not in RTAS */
 
diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index 41374a4..20b1c31 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -42,6 +42,7 @@
 #include <asm/asm-offsets.h>
 #include <asm/ptrace.h>
 #include <asm/export.h>
+#include <asm/stackprotector.h>
 
 /* As with the other PowerPC ports, it is expected that when code
  * execution begins here, the following registers contain valid, yet
@@ -835,6 +836,9 @@ start_here:
 	tophys(r4,r2)
 	addi	r4,r4,THREAD	/* init task's THREAD */
 	mtspr	SPRN_SPRG_THREAD,r4
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 
 	/* stack */
 	lis	r1,init_thread_union@ha
diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
index 37e4a7c..753e2bf 100644
--- a/arch/powerpc/kernel/head_44x.S
+++ b/arch/powerpc/kernel/head_44x.S
@@ -40,6 +40,7 @@
 #include <asm/ptrace.h>
 #include <asm/synch.h>
 #include <asm/export.h>
+#include <asm/stackprotector.h>
 #include "head_booke.h"
 
 
@@ -107,6 +108,9 @@ _ENTRY(_start);
 	/* ptr to current thread */
 	addi	r4,r2,THREAD	/* init task's THREAD */
 	mtspr	SPRN_SPRG_THREAD,r4
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 
 	/* stack */
 	lis	r1,init_thread_union@h
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 1a9c99d..49f9ce1 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -32,6 +32,7 @@
 #include <asm/ptrace.h>
 #include <asm/fixmap.h>
 #include <asm/export.h>
+#include <asm/stackprotector.h>
 
 /* Macro to make the code more readable. */
 #ifdef CONFIG_8xx_CPU6
@@ -820,6 +821,9 @@ start_here:
 	tophys(r4,r2)
 	addi	r4,r4,THREAD	/* init task's THREAD */
 	mtspr	SPRN_SPRG_THREAD,r4
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 
 	/* stack */
 	lis	r1,init_thread_union@ha
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index bf4c602..9634ac7 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -43,6 +43,7 @@
 #include <asm/cache.h>
 #include <asm/ptrace.h>
 #include <asm/export.h>
+#include <asm/stackprotector.h>
 #include "head_booke.h"
 
 /* As with the other PowerPC ports, it is expected that when code
@@ -235,6 +236,9 @@ set_ivor:
 	/* ptr to current thread */
 	addi	r4,r2,THREAD	/* init task's THREAD */
 	mtspr	SPRN_SPRG_THREAD,r4
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 
 	/* stack */
 	lis	r1,init_thread_union@h
@@ -1086,6 +1090,9 @@ __secondary_start:
 	/* ptr to current thread */
 	addi	r4,r2,THREAD	/* address of our thread_struct */
 	mtspr	SPRN_SPRG_THREAD,r4
+#if defined(CONFIG_CC_STACKPROTECTOR)
+	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
+#endif
 
 	/* Setup the defaults for TLB entries */
 	li	r4,(MAS4_TSIZED(BOOK3E_PAGESZ_4K))@l
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 04885ce..5dd056d 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -64,12 +64,6 @@
 #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.10.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC] powerpc/32: fix handling of stack protector with recent GCC
  2017-01-16  9:00 [PATCH RFC] powerpc/32: fix handling of stack protector with recent GCC Christophe Leroy
@ 2017-01-19  3:36 ` Christian Kujau
  2017-01-31  0:12 ` Christian Kujau
  1 sibling, 0 replies; 5+ messages in thread
From: Christian Kujau @ 2017-01-19  3:36 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Scott Wood, linux-kernel, linuxppc-dev

On Mon, 16 Jan 2017, Christophe Leroy wrote:
>  Christian, can you test it ?

OK, so with that applied to v4.10-rc4, compilation still fails with GCC 
4.9.2 and CC_STACKPROTECTOR_STRONG=y, see below. But it compiles just fine 
with CC_STACKPROTECTOR_REGULAR=y and boots to!

Cross-compiling the same with GCC 5.2.0 works, even for 
CC_STACKPROTECTOR_STRONG=y and the system boots just fine.

So, with that limitation, feel free to add:

 Tested-by: Christian Kujau <lists@nerdbynature.de>


Thanks for the fix!
Christian.


----
$ gcc --version | head -1
gcc-4.9.real (Debian 4.9.2-10) 4.9.2

$ grep CC_STACKPROTECTOR_STRONG $DIR/.config
CONFIG_CC_STACKPROTECTOR_STRONG=y

$ make O=$DIR V=1 bindeb-pkg
[...]
+ ld -EB -m elf32ppc -Bstatic --build-id -X -o .tmp_vmlinux1 -T 
./arch/powerpc/kernel/vmlinux.lds arch/powerpc/kernel/head_32.o 
arch/powerpc/kernel/fpu.o arch/powerpc/kernel/vector.o 
arch/powerpc/kernel/prom_init.o init/built-in.o --start-group 
usr/built-in.o arch/powerpc/kernel/built-in.o arch/powerpc/mm/built-in.o 
arch/powerpc/lib/built-in.o arch/powerpc/sysdev/built-in.o 
arch/powerpc/platforms/built-in.o arch/powerpc/math-emu/built-in.o 
arch/powerpc/crypto/built-in.o arch/powerpc/net/built-in.o 
kernel/built-in.o certs/built-in.o mm/built-in.o fs/built-in.o 
ipc/built-in.o security/built-in.o crypto/built-in.o block/built-in.o 
lib/lib.a lib/built-in.o drivers/built-in.o sound/built-in.o 
firmware/built-in.o net/built-in.o virt/built-in.o --end-group
arch/powerpc/platforms/built-in.o: In function `bootx_printf':
/usr/local/src/linux-git/arch/powerpc/platforms/powermac/bootx_init.c:88: undefined reference to `__stack_chk_fail_local'
arch/powerpc/platforms/built-in.o: In function `bootx_add_display_props':
/usr/local/src/linux-git/arch/powerpc/platforms/powermac/bootx_init.c:211: undefined reference to `__stack_chk_fail_local' 
arch/powerpc/platforms/built-in.o: In function `bootx_scan_dt_build_struct':
/usr/local/src/linux-git/arch/powerpc/platforms/powermac/bootx_init.c:350: undefined reference to `__stack_chk_fail_local'
arch/powerpc/platforms/built-in.o: In function `bootx_init':
/usr/local/src/linux-git/arch/powerpc/platforms/powermac/bootx_init.c:596: undefined reference to `__stack_chk_fail_local'
/usr/bin/ld.bfd.real: .tmp_vmlinux1: hidden symbol `__stack_chk_fail_local' isn't defined
/usr/bin/ld.bfd.real: final link failed: Bad value

-- 
BOFH excuse #66:

bit bucket overflow

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC] powerpc/32: fix handling of stack protector with recent GCC
  2017-01-16  9:00 [PATCH RFC] powerpc/32: fix handling of stack protector with recent GCC Christophe Leroy
  2017-01-19  3:36 ` Christian Kujau
@ 2017-01-31  0:12 ` Christian Kujau
  2017-01-31  1:22   ` Segher Boessenkool
  2017-01-31  3:37   ` Michael Ellerman
  1 sibling, 2 replies; 5+ messages in thread
From: Christian Kujau @ 2017-01-31  0:12 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Scott Wood, linux-kernel, linuxppc-dev

On Mon, 16 Jan 2017, Christophe Leroy wrote:
> Since 2005, powerpc GCC doesn't manage anymore __stack_chk_guard as
> a global variable but as some value located at -0x7008(r2)

Is this still an "RFC" or is there a chance that this will land in 4.10?

Thanks,
Christian.

> In the Linux kernel, r2 is used as a pointer to current task struct.
> 
> This patch changes the meaning of r2 when stack protector
> is activated:
> - current is taken from thread_info and not kept in r2 anymore
> - r2 is set to current + offset of stack canary + 0x7008 so
> that -0x7008(r2) directly points to current->stack_canary
> 
> current could have been more efficiently calculated from r2
> but some circular inclusion prevent inserting struct task_struct
> into arch/powerpc/include/asm/current.h so it is not possible
> to get offset of stack_canary within current task_struct from there.
> 
> fixes: 6533b7c16ee57 ("powerpc: Initial stack protector
> (-fstack-protector) support")
> Reported-by: Christian Kujau <lists@nerdbynature.de>
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  Christian, can you test it ?
> 
>  arch/powerpc/include/asm/current.h        | 12 +++++++++++-
>  arch/powerpc/include/asm/stackprotector.h | 13 +++++++++----
>  arch/powerpc/kernel/entry_32.S            | 19 +++++++++++++++----
>  arch/powerpc/kernel/head_32.S             |  7 +++++++
>  arch/powerpc/kernel/head_40x.S            |  4 ++++
>  arch/powerpc/kernel/head_44x.S            |  4 ++++
>  arch/powerpc/kernel/head_8xx.S            |  4 ++++
>  arch/powerpc/kernel/head_fsl_booke.S      |  7 +++++++
>  arch/powerpc/kernel/process.c             |  6 ------
>  9 files changed, 61 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/current.h b/arch/powerpc/include/asm/current.h
> index e2c7f06..2f67f02 100644
> --- a/arch/powerpc/include/asm/current.h
> +++ b/arch/powerpc/include/asm/current.h
> @@ -27,8 +27,16 @@ static inline struct task_struct *get_current(void)
>  }
>  #define current	get_current()
>  
> -#else
> +#else /* __powerpc64__ */
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +#include <linux/thread_info.h>
>  
> +static inline struct task_struct *get_current(void)
> +{
> +	return current_thread_info()->task;
> +}
> +#define current	get_current()
> +#else
>  /*
>   * We keep `current' in r2 for speed.
>   */
> @@ -36,5 +44,7 @@ register struct task_struct *current asm ("r2");
>  
>  #endif
>  
> +#endif /* __powerpc64__ */
> +
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_CURRENT_H */
> diff --git a/arch/powerpc/include/asm/stackprotector.h b/arch/powerpc/include/asm/stackprotector.h
> index 6720190..bf30509 100644
> --- a/arch/powerpc/include/asm/stackprotector.h
> +++ b/arch/powerpc/include/asm/stackprotector.h
> @@ -12,12 +12,18 @@
>  #ifndef _ASM_STACKPROTECTOR_H
>  #define _ASM_STACKPROTECTOR_H
>  
> +#ifdef CONFIG_PPC64
> +#define SSP_OFFSET	0x7010
> +#else
> +#define SSP_OFFSET	0x7008
> +#endif
> +
> +#ifndef __ASSEMBLY__
> +
>  #include <linux/random.h>
>  #include <linux/version.h>
>  #include <asm/reg.h>
>  
> -extern unsigned long __stack_chk_guard;
> -
>  /*
>   * Initialize the stackprotector canary value.
>   *
> @@ -34,7 +40,6 @@ static __always_inline void boot_init_stack_canary(void)
>  	canary ^= LINUX_VERSION_CODE;
>  
>  	current->stack_canary = canary;
> -	__stack_chk_guard = current->stack_canary;
>  }
> -
> +#endif /* __ASSEMBLY__ */
>  #endif	/* _ASM_STACKPROTECTOR_H */
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 5742dbd..b3a363c 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -34,6 +34,7 @@
>  #include <asm/ftrace.h>
>  #include <asm/ptrace.h>
>  #include <asm/export.h>
> +#include <asm/stackprotector.h>
>  
>  /*
>   * MSR_KERNEL is > 0x10000 on 4xx/Book-E since it include MSR_CE.
> @@ -149,6 +150,9 @@ transfer_to_handler:
>  	mfspr	r12,SPRN_SPRG_THREAD
>  	addi	r2,r12,-THREAD
>  	tovirt(r2,r2)			/* set r2 to current */
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  	beq	2f			/* if from user, fix up THREAD.regs */
>  	addi	r11,r1,STACK_FRAME_OVERHEAD
>  	stw	r11,PT_REGS(r12)
> @@ -385,6 +389,9 @@ syscall_exit_cont:
>  	lwz	r3,GPR3(r1)
>  1:
>  #endif /* CONFIG_TRACE_IRQFLAGS */
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	subi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
>  	/* If the process has its own DBCR0 value, load it up.  The internal
>  	   debug mode bit tells us that dbcr0 should be loaded. */
> @@ -617,6 +624,9 @@ _GLOBAL(_switch)
>  	stwu	r1,-INT_FRAME_SIZE(r1)
>  	mflr	r0
>  	stw	r0,INT_FRAME_SIZE+4(r1)
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	subi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  	/* r3-r12 are caller saved -- Cort */
>  	SAVE_NVGPRS(r1)
>  	stw	r0,_NIP(r1)	/* Return to switch caller */
> @@ -674,10 +684,8 @@ 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)
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
>  #endif
>  	lwz	r0,_CCR(r1)
>  	mtcrf	0xFF,r0
> @@ -779,6 +787,9 @@ user_exc_return:		/* r10 contains MSR_KERNEL here */
>  	bne	do_work
>  
>  restore_user:
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	subi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
>  	/* Check whether this process has its own DBCR0 value.  The internal
>  	   debug mode bit tells us that dbcr0 should be loaded. */
> diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
> index 9d96354..bae9b83 100644
> --- a/arch/powerpc/kernel/head_32.S
> +++ b/arch/powerpc/kernel/head_32.S
> @@ -35,6 +35,7 @@
>  #include <asm/bug.h>
>  #include <asm/kvm_book3s_asm.h>
>  #include <asm/export.h>
> +#include <asm/stackprotector.h>
>  
>  /* 601 only have IBAT; cr0.eq is set on 601 when using this macro */
>  #define LOAD_BAT(n, reg, RA, RB)	\
> @@ -864,6 +865,9 @@ __secondary_start:
>  	tophys(r4,r2)
>  	addi	r4,r4,THREAD	/* phys address of our thread_struct */
>  	mtspr	SPRN_SPRG_THREAD,r4
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  	li	r3,0
>  	mtspr	SPRN_SPRG_RTAS,r3	/* 0 => not in RTAS */
>  
> @@ -950,6 +954,9 @@ start_here:
>  	tophys(r4,r2)
>  	addi	r4,r4,THREAD	/* init task's THREAD */
>  	mtspr	SPRN_SPRG_THREAD,r4
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  	li	r3,0
>  	mtspr	SPRN_SPRG_RTAS,r3	/* 0 => not in RTAS */
>  
> diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
> index 41374a4..20b1c31 100644
> --- a/arch/powerpc/kernel/head_40x.S
> +++ b/arch/powerpc/kernel/head_40x.S
> @@ -42,6 +42,7 @@
>  #include <asm/asm-offsets.h>
>  #include <asm/ptrace.h>
>  #include <asm/export.h>
> +#include <asm/stackprotector.h>
>  
>  /* As with the other PowerPC ports, it is expected that when code
>   * execution begins here, the following registers contain valid, yet
> @@ -835,6 +836,9 @@ start_here:
>  	tophys(r4,r2)
>  	addi	r4,r4,THREAD	/* init task's THREAD */
>  	mtspr	SPRN_SPRG_THREAD,r4
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  
>  	/* stack */
>  	lis	r1,init_thread_union@ha
> diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
> index 37e4a7c..753e2bf 100644
> --- a/arch/powerpc/kernel/head_44x.S
> +++ b/arch/powerpc/kernel/head_44x.S
> @@ -40,6 +40,7 @@
>  #include <asm/ptrace.h>
>  #include <asm/synch.h>
>  #include <asm/export.h>
> +#include <asm/stackprotector.h>
>  #include "head_booke.h"
>  
>  
> @@ -107,6 +108,9 @@ _ENTRY(_start);
>  	/* ptr to current thread */
>  	addi	r4,r2,THREAD	/* init task's THREAD */
>  	mtspr	SPRN_SPRG_THREAD,r4
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  
>  	/* stack */
>  	lis	r1,init_thread_union@h
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
> index 1a9c99d..49f9ce1 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -32,6 +32,7 @@
>  #include <asm/ptrace.h>
>  #include <asm/fixmap.h>
>  #include <asm/export.h>
> +#include <asm/stackprotector.h>
>  
>  /* Macro to make the code more readable. */
>  #ifdef CONFIG_8xx_CPU6
> @@ -820,6 +821,9 @@ start_here:
>  	tophys(r4,r2)
>  	addi	r4,r4,THREAD	/* init task's THREAD */
>  	mtspr	SPRN_SPRG_THREAD,r4
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  
>  	/* stack */
>  	lis	r1,init_thread_union@ha
> diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
> index bf4c602..9634ac7 100644
> --- a/arch/powerpc/kernel/head_fsl_booke.S
> +++ b/arch/powerpc/kernel/head_fsl_booke.S
> @@ -43,6 +43,7 @@
>  #include <asm/cache.h>
>  #include <asm/ptrace.h>
>  #include <asm/export.h>
> +#include <asm/stackprotector.h>
>  #include "head_booke.h"
>  
>  /* As with the other PowerPC ports, it is expected that when code
> @@ -235,6 +236,9 @@ set_ivor:
>  	/* ptr to current thread */
>  	addi	r4,r2,THREAD	/* init task's THREAD */
>  	mtspr	SPRN_SPRG_THREAD,r4
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  
>  	/* stack */
>  	lis	r1,init_thread_union@h
> @@ -1086,6 +1090,9 @@ __secondary_start:
>  	/* ptr to current thread */
>  	addi	r4,r2,THREAD	/* address of our thread_struct */
>  	mtspr	SPRN_SPRG_THREAD,r4
> +#if defined(CONFIG_CC_STACKPROTECTOR)
> +	addi	r2,r2,TSK_STACK_CANARY+SSP_OFFSET
> +#endif
>  
>  	/* Setup the defaults for TLB entries */
>  	li	r4,(MAS4_TSIZED(BOOK3E_PAGESZ_4K))@l
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 04885ce..5dd056d 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -64,12 +64,6 @@
>  #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.10.1
> 
> 

-- 
BOFH excuse #443:

Zombie processes detected, machine is haunted.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC] powerpc/32: fix handling of stack protector with recent GCC
  2017-01-31  0:12 ` Christian Kujau
@ 2017-01-31  1:22   ` Segher Boessenkool
  2017-01-31  3:37   ` Michael Ellerman
  1 sibling, 0 replies; 5+ messages in thread
From: Segher Boessenkool @ 2017-01-31  1:22 UTC (permalink / raw)
  To: Christian Kujau
  Cc: Christophe Leroy, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

On Mon, Jan 30, 2017 at 04:12:53PM -0800, Christian Kujau wrote:
> On Mon, 16 Jan 2017, Christophe Leroy wrote:
> > Since 2005, powerpc GCC doesn't manage anymore __stack_chk_guard as
> > a global variable but as some value located at -0x7008(r2)
> 
> Is this still an "RFC" or is there a chance that this will land in 4.10?

Older GCC (i.e. not ancient, but < 7 currently; the new options will be
backported to 5 and 6) doesn't always use TLS stack canaries either: it
depends on how your GCC is configured.  The kernel will have to detect
if the GCC it uses knows the new options, and if not, if it still wants
to use SSP it has to detect what GCC uses to get at the canary.

This patch as-is won't work.


Segher

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC] powerpc/32: fix handling of stack protector with recent GCC
  2017-01-31  0:12 ` Christian Kujau
  2017-01-31  1:22   ` Segher Boessenkool
@ 2017-01-31  3:37   ` Michael Ellerman
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2017-01-31  3:37 UTC (permalink / raw)
  To: Christian Kujau, Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Scott Wood, linux-kernel,
	linuxppc-dev

Christian Kujau <lists@nerdbynature.de> writes:

> On Mon, 16 Jan 2017, Christophe Leroy wrote:
>> Since 2005, powerpc GCC doesn't manage anymore __stack_chk_guard as
>> a global variable but as some value located at -0x7008(r2)
>
> Is this still an "RFC" or is there a chance that this will land in 4.10?

No. I've reverted the stack protector support in my fixes branch (which
is in linux-next), and it will go into 4.10 this week.

cheers

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-01-31  3:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-16  9:00 [PATCH RFC] powerpc/32: fix handling of stack protector with recent GCC Christophe Leroy
2017-01-19  3:36 ` Christian Kujau
2017-01-31  0:12 ` Christian Kujau
2017-01-31  1:22   ` Segher Boessenkool
2017-01-31  3:37   ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).