All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] powerpc: stack protector (-fstack-protector) support
@ 2016-09-30 14:26 Christophe Leroy
  2016-09-30 14:26 ` [PATCH 1/2] powerpc: initial " Christophe Leroy
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Christophe Leroy @ 2016-09-30 14:26 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Scott Wood
  Cc: linux-kernel, linuxppc-dev, Nicolas Pitre

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

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

* [PATCH 1/2] powerpc: initial stack protector (-fstack-protector) support
  2016-09-30 14:26 [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Christophe Leroy
@ 2016-09-30 14:26 ` Christophe Leroy
  2016-11-17 11:05   ` Michael Ellerman
  2016-09-30 14:27 ` [PATCH 2/2] powerpc/32: stack protector: change the canary value per task Christophe Leroy
  2016-09-30 16:26 ` [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Denis Kirjanov
  2 siblings, 1 reply; 10+ messages in thread
From: Christophe Leroy @ 2016-09-30 14:26 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Scott Wood
  Cc: linux-kernel, linuxppc-dev, Nicolas Pitre

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

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

* [PATCH 2/2] powerpc/32: stack protector: change the canary value per task
  2016-09-30 14:26 [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Christophe Leroy
  2016-09-30 14:26 ` [PATCH 1/2] powerpc: initial " Christophe Leroy
@ 2016-09-30 14:27 ` Christophe Leroy
  2016-09-30 16:26 ` [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Denis Kirjanov
  2 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2016-09-30 14:27 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Scott Wood
  Cc: linux-kernel, linuxppc-dev, Nicolas Pitre

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

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

* Re: [PATCH 0/2] powerpc: stack protector (-fstack-protector) support
  2016-09-30 14:26 [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Christophe Leroy
  2016-09-30 14:26 ` [PATCH 1/2] powerpc: initial " Christophe Leroy
  2016-09-30 14:27 ` [PATCH 2/2] powerpc/32: stack protector: change the canary value per task Christophe Leroy
@ 2016-09-30 16:26 ` Denis Kirjanov
  2016-09-30 16:38   ` Christophe LEROY
  2016-11-17 11:27   ` Michael Ellerman
  2 siblings, 2 replies; 10+ messages in thread
From: Denis Kirjanov @ 2016-09-30 16:26 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Scott Wood, Nicolas Pitre, linuxppc-dev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 924 bytes --]

On Friday, September 30, 2016, Christophe Leroy <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


>
> 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
>
>

[-- Attachment #2: Type: text/html, Size: 1405 bytes --]

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

* Re: [PATCH 0/2] powerpc: stack protector (-fstack-protector) support
  2016-09-30 16:26 ` [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Denis Kirjanov
@ 2016-09-30 16:38   ` Christophe LEROY
  2016-09-30 23:17     ` Benjamin Herrenschmidt
  2016-11-17 11:27   ` Michael Ellerman
  1 sibling, 1 reply; 10+ messages in thread
From: Christophe LEROY @ 2016-09-30 16:38 UTC (permalink / raw)
  To: Denis Kirjanov
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Scott Wood, Nicolas Pitre, linuxppc-dev, linux-kernel



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
>

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

* Re: [PATCH 0/2] powerpc: stack protector (-fstack-protector) support
  2016-09-30 16:38   ` Christophe LEROY
@ 2016-09-30 23:17     ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2016-09-30 23:17 UTC (permalink / raw)
  To: Christophe LEROY, Denis Kirjanov
  Cc: Paul Mackerras, Michael Ellerman, Scott Wood, Nicolas Pitre,
	linuxppc-dev, linux-kernel

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.

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

* Re: [PATCH 1/2] powerpc: initial stack protector (-fstack-protector) support
  2016-09-30 14:26 ` [PATCH 1/2] powerpc: initial " Christophe Leroy
@ 2016-11-17 11:05   ` Michael Ellerman
  2016-11-22 10:51     ` Christophe LEROY
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2016-11-17 11:05 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, Scott Wood
  Cc: linux-kernel, linuxppc-dev, Nicolas Pitre

Christophe Leroy <christophe.leroy@c-s.fr> writes:

> 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
                             ^
                             PPC
                             
> + * we cannot have a different canary value per task.
> + */
> +
> +#ifndef _ASM_STACKPROTECTOR_H
> +#define _ASM_STACKPROTECTOR_H 1

We usually just define it, not define it to 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;

What about mixing in an mftb() as well ?

> 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)

We've already assigned to CFLAGS_prom_init.o so I think you should be
using += not := shouldn't you?

Also it could just be a single line:

CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)


cheers

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

* Re: [PATCH 0/2] powerpc: stack protector (-fstack-protector) support
  2016-09-30 16:26 ` [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Denis Kirjanov
  2016-09-30 16:38   ` Christophe LEROY
@ 2016-11-17 11:27   ` Michael Ellerman
  2016-11-18 12:43     ` Denis Kirjanov
  1 sibling, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2016-11-17 11:27 UTC (permalink / raw)
  To: Denis Kirjanov, Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Scott Wood,
	Nicolas Pitre, linuxppc-dev, linux-kernel

Denis Kirjanov <kda@linux-powerpc.org> writes:

> On Friday, September 30, 2016, Christophe Leroy <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

Did you get around to testing it?

It seems to be working here:

  root@mpe-ubuntu-le:/sys/kernel/debug/provoke-crash# echo CORRUPT_STACK > DIRECT
  Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: c000000000671ed8
  
  CPU: 1 PID: 3835 Comm: bash Not tainted 4.9.0-rc5-compiler_gcc-6.2.0-00075-gf41db6c7a3c8-dirty #464
  Call Trace:
  [c0000000dbf73a30] [c000000000541728] dump_stack+0xb8/0x100 (unreliable)
  
  [c0000000dbf73a70] [c000000000219ba8] panic+0x14c/0x320
  [c0000000dbf73b10] [c0000000000c27ec] __stack_chk_fail+0x2c/0x30
  [c0000000dbf73b70] [c000000000671ed8] lkdtm_CORRUPT_STACK+0x88/0x90
  [c0000000dbf73bf0] [6161616161616161] 0x6161616161616161
  Rebooting in 10 seconds..


cheers

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

* Re: [PATCH 0/2] powerpc: stack protector (-fstack-protector) support
  2016-11-17 11:27   ` Michael Ellerman
@ 2016-11-18 12:43     ` Denis Kirjanov
  0 siblings, 0 replies; 10+ messages in thread
From: Denis Kirjanov @ 2016-11-18 12:43 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Scott Wood, Nicolas Pitre, linuxppc-dev, linux-kernel

On 11/17/16, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Denis Kirjanov <kda@linux-powerpc.org> writes:
>
>> On Friday, September 30, 2016, Christophe Leroy <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

Not yet, I've broken my test machine :/ I need some time to recover

>
> Did you get around to testing it?
>
> It seems to be working here:
>
>   root@mpe-ubuntu-le:/sys/kernel/debug/provoke-crash# echo CORRUPT_STACK >
> DIRECT
>   Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in:
> c000000000671ed8
>
>   CPU: 1 PID: 3835 Comm: bash Not tainted
> 4.9.0-rc5-compiler_gcc-6.2.0-00075-gf41db6c7a3c8-dirty #464
>   Call Trace:
>   [c0000000dbf73a30] [c000000000541728] dump_stack+0xb8/0x100 (unreliable)
>
>   [c0000000dbf73a70] [c000000000219ba8] panic+0x14c/0x320
>   [c0000000dbf73b10] [c0000000000c27ec] __stack_chk_fail+0x2c/0x30
>   [c0000000dbf73b70] [c000000000671ed8] lkdtm_CORRUPT_STACK+0x88/0x90
>   [c0000000dbf73bf0] [6161616161616161] 0x6161616161616161
>   Rebooting in 10 seconds..
>
>
> cheers
>

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

* Re: [PATCH 1/2] powerpc: initial stack protector (-fstack-protector) support
  2016-11-17 11:05   ` Michael Ellerman
@ 2016-11-22 10:51     ` Christophe LEROY
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe LEROY @ 2016-11-22 10:51 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Scott Wood
  Cc: linux-kernel, linuxppc-dev, Nicolas Pitre



Le 17/11/2016 à 12:05, Michael Ellerman a écrit :

Hi Michael,

I took your comments into account in v2. Shame on me, I forgot to add 
the list of changes from v1 to v2 in the commit log.

Christophe

> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>
>> 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
>                              ^
>                              PPC
>
>> + * we cannot have a different canary value per task.
>> + */
>> +
>> +#ifndef _ASM_STACKPROTECTOR_H
>> +#define _ASM_STACKPROTECTOR_H 1
>
> We usually just define it, not define it to 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;
>
> What about mixing in an mftb() as well ?
>
>> 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)
>
> We've already assigned to CFLAGS_prom_init.o so I think you should be
> using += not := shouldn't you?
>
> Also it could just be a single line:
>
> CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)
>
>
> cheers
>

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

end of thread, other threads:[~2016-11-22 11:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-30 14:26 [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Christophe Leroy
2016-09-30 14:26 ` [PATCH 1/2] powerpc: initial " Christophe Leroy
2016-11-17 11:05   ` Michael Ellerman
2016-11-22 10:51     ` Christophe LEROY
2016-09-30 14:27 ` [PATCH 2/2] powerpc/32: stack protector: change the canary value per task Christophe Leroy
2016-09-30 16:26 ` [PATCH 0/2] powerpc: stack protector (-fstack-protector) support Denis Kirjanov
2016-09-30 16:38   ` Christophe LEROY
2016-09-30 23:17     ` Benjamin Herrenschmidt
2016-11-17 11:27   ` Michael Ellerman
2016-11-18 12:43     ` Denis Kirjanov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.