All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 1/2] powerpc/32: add stack protector support
@ 2018-09-27  7:05 Christophe Leroy
  2018-09-27  7:05 ` [PATCH v5 2/2] powerpc/64: " Christophe Leroy
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Christophe Leroy @ 2018-09-27  7:05 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev

This functionality was tentatively added in the past
(commit 6533b7c16ee5 ("powerpc: Initial stack protector
(-fstack-protector) support")) but had to be reverted
(commit f2574030b0e3 ("powerpc: Revert the initial stack
protector support") because of GCC implementing it differently
whether it had been built with libc support or not.

Now, GCC offers the possibility to manually set the
stack-protector mode (global or tls) regardless of libc support.

This time, the patch selects HAVE_STACKPROTECTOR only if
-mstack-protector-guard=tls is supported by GCC.

On PPC32, as register r2 points to current task_struct at
all time, the stack_canary located inside task_struct can be
used directly by using the following GCC options:
-mstack-protector-guard=tls
-mstack-protector-guard-reg=r2
-mstack-protector-guard-offset=offsetof(struct task_struct, stack_canary))

The protector is disabled for prom_init and bootx_init as
it is too early to handle it properly.

 $ echo CORRUPT_STACK > /sys/kernel/debug/provoke-crash/DIRECT
[  134.943666] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: lkdtm_CORRUPT_STACK+0x64/0x64
[  134.943666]
[  134.955414] CPU: 0 PID: 283 Comm: sh Not tainted 4.18.0-s3k-dev-12143-ga3272be41209 #835
[  134.963380] Call Trace:
[  134.965860] [c6615d60] [c001f76c] panic+0x118/0x260 (unreliable)
[  134.971775] [c6615dc0] [c001f654] panic+0x0/0x260
[  134.976435] [c6615dd0] [c032c368] lkdtm_CORRUPT_STACK_STRONG+0x0/0x64
[  134.982769] [c6615e00] [ffffffff] 0xffffffff

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 v5: Using get_random_canary() and masking canary with CANARY_MASK

 v4: disable stack protector in bootx_init

 v3: the offset is now defined by a rule in the Makefile. No need anymore
     to take stack_canary out of the randomised area of task_struct

 arch/powerpc/Kconfig                      |  1 +
 arch/powerpc/Makefile                     | 10 +++++++++
 arch/powerpc/include/asm/stackprotector.h | 34 +++++++++++++++++++++++++++++++
 arch/powerpc/kernel/Makefile              |  2 ++
 arch/powerpc/kernel/asm-offsets.c         |  3 +++
 arch/powerpc/platforms/powermac/Makefile  |  1 +
 6 files changed, 51 insertions(+)
 create mode 100644 arch/powerpc/include/asm/stackprotector.h

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a80669209155..3bcb05929931 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -180,6 +180,7 @@ config PPC
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_CBPF_JIT			if !PPC64
+	select HAVE_STACKPROTECTOR		if $(cc-option,-mstack-protector-guard=tls) && PPC32
 	select HAVE_CONTEXT_TRACKING		if PPC64
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DEBUG_STACKOVERFLOW
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 07d9dce7eda6..45b8eb4d8fe7 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -112,6 +112,9 @@ KBUILD_LDFLAGS	+= -m elf$(BITS)$(LDEMULATION)
 KBUILD_ARFLAGS	+= --target=elf$(BITS)-$(GNUTARGET)
 endif
 
+cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard=tls
+cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r2
+
 LDFLAGS_vmlinux-y := -Bstatic
 LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
 LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
@@ -404,6 +407,13 @@ archclean:
 
 archprepare: checkbin
 
+ifdef CONFIG_STACKPROTECTOR
+prepare: stack_protector_prepare
+
+stack_protector_prepare: prepare0
+	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
+endif
+
 # Use the file '.tmp_gas_check' for binutils tests, as gas won't output
 # to stdout and these checks are run even on install targets.
 TOUT	:= .tmp_gas_check
diff --git a/arch/powerpc/include/asm/stackprotector.h b/arch/powerpc/include/asm/stackprotector.h
new file mode 100644
index 000000000000..d05d969c98c2
--- /dev/null
+++ b/arch/powerpc/include/asm/stackprotector.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * GCC stack protector support.
+ *
+ */
+
+#ifndef _ASM_STACKPROTECTOR_H
+#define _ASM_STACKPROTECTOR_H
+
+#include <linux/random.h>
+#include <linux/version.h>
+#include <asm/reg.h>
+#include <asm/current.h>
+
+/*
+ * 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. */
+	canary = get_random_canary();
+	canary ^= mftb();
+	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
+
+	current->stack_canary = canary;
+}
+
+#endif	/* _ASM_STACKPROTECTOR_H */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 1e64cfe22a83..85ffa488dfb5 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -20,6 +20,8 @@ CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 
+CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)
+
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
 CFLAGS_REMOVE_cputable.o = $(CC_FLAGS_FTRACE)
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index ba9d0fc98730..a992f7a53cf3 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -79,6 +79,9 @@ int main(void)
 {
 	OFFSET(THREAD, task_struct, thread);
 	OFFSET(MM, task_struct, mm);
+#ifdef CONFIG_STACKPROTECTOR
+	OFFSET(TASK_CANARY, task_struct, stack_canary);
+#endif
 	OFFSET(MMCONTEXTID, mm_struct, context.id);
 #ifdef CONFIG_PPC64
 	DEFINE(SIGSEGV, SIGSEGV);
diff --git a/arch/powerpc/platforms/powermac/Makefile b/arch/powerpc/platforms/powermac/Makefile
index 561a67d65e4d..923bfb340433 100644
--- a/arch/powerpc/platforms/powermac/Makefile
+++ b/arch/powerpc/platforms/powermac/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 CFLAGS_bootx_init.o  		+= -fPIC
+CFLAGS_bootx_init.o  		+= $(call cc-option, -fno-stack-protector)
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
-- 
2.13.3


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

* [PATCH v5 2/2] powerpc/64: add stack protector support
  2018-09-27  7:05 [PATCH v5 1/2] powerpc/32: add stack protector support Christophe Leroy
@ 2018-09-27  7:05 ` Christophe Leroy
  2018-10-04  6:14 ` [v5,1/2] powerpc/32: " Michael Ellerman
  2019-01-12 23:32   ` Samuel Holland
  2 siblings, 0 replies; 7+ messages in thread
From: Christophe Leroy @ 2018-09-27  7:05 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev

On PPC64, as register r13 points to the paca_struct at all time,
this patch adds a copy of the canary there, which is copied at
task_switch.
That new canary is then used by using the following GCC options:
-mstack-protector-guard=tls
-mstack-protector-guard-reg=r13
-mstack-protector-guard-offset=offsetof(struct paca_struct, canary))

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/Kconfig                      | 2 +-
 arch/powerpc/Makefile                     | 8 ++++++++
 arch/powerpc/include/asm/paca.h           | 3 +++
 arch/powerpc/include/asm/stackprotector.h | 4 ++++
 arch/powerpc/kernel/asm-offsets.c         | 3 +++
 arch/powerpc/kernel/entry_64.S            | 4 ++++
 6 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 3bcb05929931..602eea723624 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -180,7 +180,7 @@ config PPC
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_CBPF_JIT			if !PPC64
-	select HAVE_STACKPROTECTOR		if $(cc-option,-mstack-protector-guard=tls) && PPC32
+	select HAVE_STACKPROTECTOR		if $(cc-option,-mstack-protector-guard=tls)
 	select HAVE_CONTEXT_TRACKING		if PPC64
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DEBUG_STACKOVERFLOW
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 45b8eb4d8fe7..81552c7b46eb 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -113,7 +113,11 @@ KBUILD_ARFLAGS	+= --target=elf$(BITS)-$(GNUTARGET)
 endif
 
 cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard=tls
+ifdef CONFIG_PPC64
+cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r13
+else
 cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r2
+endif
 
 LDFLAGS_vmlinux-y := -Bstatic
 LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
@@ -411,8 +415,12 @@ ifdef CONFIG_STACKPROTECTOR
 prepare: stack_protector_prepare
 
 stack_protector_prepare: prepare0
+ifdef CONFIG_PPC64
+	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "PACA_CANARY") print $$3;}' include/generated/asm-offsets.h))
+else
 	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
 endif
+endif
 
 # Use the file '.tmp_gas_check' for binutils tests, as gas won't output
 # to stdout and these checks are run even on install targets.
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 6d6b3706232c..98d883e58945 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -246,6 +246,9 @@ struct paca_struct {
 	struct slb_entry *mce_faulty_slbs;
 	u16 slb_save_cache_ptr;
 #endif /* CONFIG_PPC_BOOK3S_64 */
+#ifdef CONFIG_STACKPROTECTOR
+	unsigned long canary;
+#endif
 } ____cacheline_aligned;
 
 extern struct paca_struct **paca_ptrs;
diff --git a/arch/powerpc/include/asm/stackprotector.h b/arch/powerpc/include/asm/stackprotector.h
index d05d969c98c2..1c8460e23583 100644
--- a/arch/powerpc/include/asm/stackprotector.h
+++ b/arch/powerpc/include/asm/stackprotector.h
@@ -11,6 +11,7 @@
 #include <linux/version.h>
 #include <asm/reg.h>
 #include <asm/current.h>
+#include <asm/paca.h>
 
 /*
  * Initialize the stackprotector canary value.
@@ -29,6 +30,9 @@ static __always_inline void boot_init_stack_canary(void)
 	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
+#ifdef CONFIG_PPC64
+	get_paca()->canary = canary;
+#endif
 }
 
 #endif	/* _ASM_STACKPROTECTOR_H */
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index a992f7a53cf3..773dee55b3f6 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -81,6 +81,9 @@ int main(void)
 	OFFSET(MM, task_struct, mm);
 #ifdef CONFIG_STACKPROTECTOR
 	OFFSET(TASK_CANARY, task_struct, stack_canary);
+#ifdef CONFIG_PPC64
+	OFFSET(PACA_CANARY, paca_struct, canary);
+#endif
 #endif
 	OFFSET(MMCONTEXTID, mm_struct, context.id);
 #ifdef CONFIG_PPC64
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 77a888bfcb53..573fa879d785 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -624,6 +624,10 @@ _GLOBAL(_switch)
 
 	addi	r6,r4,-THREAD	/* Convert THREAD to 'current' */
 	std	r6,PACACURRENT(r13)	/* Set new 'current' */
+#if defined(CONFIG_STACKPROTECTOR)
+	ld	r6, TASK_CANARY(r6)
+	std	r6, PACA_CANARY(r13)
+#endif
 
 	ld	r8,KSP(r4)	/* new stack pointer */
 #ifdef CONFIG_PPC_BOOK3S_64
-- 
2.13.3


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

* Re: [v5,1/2] powerpc/32: add stack protector support
  2018-09-27  7:05 [PATCH v5 1/2] powerpc/32: add stack protector support Christophe Leroy
  2018-09-27  7:05 ` [PATCH v5 2/2] powerpc/64: " Christophe Leroy
@ 2018-10-04  6:14 ` Michael Ellerman
  2019-01-12 23:32   ` Samuel Holland
  2 siblings, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2018-10-04  6:14 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linuxppc-dev, linux-kernel

On Thu, 2018-09-27 at 07:05:53 UTC, Christophe Leroy wrote:
> This functionality was tentatively added in the past
> (commit 6533b7c16ee5 ("powerpc: Initial stack protector
> (-fstack-protector) support")) but had to be reverted
> (commit f2574030b0e3 ("powerpc: Revert the initial stack
> protector support") because of GCC implementing it differently
> whether it had been built with libc support or not.
> 
> Now, GCC offers the possibility to manually set the
> stack-protector mode (global or tls) regardless of libc support.
> 
> This time, the patch selects HAVE_STACKPROTECTOR only if
> -mstack-protector-guard=tls is supported by GCC.
> 
> On PPC32, as register r2 points to current task_struct at
> all time, the stack_canary located inside task_struct can be
> used directly by using the following GCC options:
> -mstack-protector-guard=tls
> -mstack-protector-guard-reg=r2
> -mstack-protector-guard-offset=offsetof(struct task_struct, stack_canary))
> 
> The protector is disabled for prom_init and bootx_init as
> it is too early to handle it properly.
> 
>  $ echo CORRUPT_STACK > /sys/kernel/debug/provoke-crash/DIRECT
> [  134.943666] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: lkdtm_CORRUPT_STACK+0x64/0x64
> [  134.943666]
> [  134.955414] CPU: 0 PID: 283 Comm: sh Not tainted 4.18.0-s3k-dev-12143-ga3272be41209 #835
> [  134.963380] Call Trace:
> [  134.965860] [c6615d60] [c001f76c] panic+0x118/0x260 (unreliable)
> [  134.971775] [c6615dc0] [c001f654] panic+0x0/0x260
> [  134.976435] [c6615dd0] [c032c368] lkdtm_CORRUPT_STACK_STRONG+0x0/0x64
> [  134.982769] [c6615e00] [ffffffff] 0xffffffff
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/c3ff2a5193fa61b1b284cfb1d79628

cheers

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

* Re: [PATCH v5 1/2] powerpc/32: add stack protector support
  2018-09-27  7:05 [PATCH v5 1/2] powerpc/32: add stack protector support Christophe Leroy
@ 2019-01-12 23:32   ` Samuel Holland
  2018-10-04  6:14 ` [v5,1/2] powerpc/32: " Michael Ellerman
  2019-01-12 23:32   ` Samuel Holland
  2 siblings, 0 replies; 7+ messages in thread
From: Samuel Holland @ 2019-01-12 23:32 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Masahiro Yamada
  Cc: linux-kernel, linuxppc-dev, Jason A. Donenfeld

Hello all,

On 09/27/18 02:05, Christophe Leroy wrote:
[..snip..]
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 07d9dce7eda6..45b8eb4d8fe7 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -112,6 +112,9 @@ KBUILD_LDFLAGS	+= -m elf$(BITS)$(LDEMULATION)
>  KBUILD_ARFLAGS	+= --target=elf$(BITS)-$(GNUTARGET)
>  endif
>  
> +cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard=tls
> +cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r2
> +
>  LDFLAGS_vmlinux-y := -Bstatic
>  LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
>  LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
> @@ -404,6 +407,13 @@ archclean:
>  
>  archprepare: checkbin
>  
> +ifdef CONFIG_STACKPROTECTOR
> +prepare: stack_protector_prepare
> +
> +stack_protector_prepare: prepare0
> +	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
> +endif
> +

This breaks when building out-of-tree kernel modules. GCC is not getting passed
the -mstack-protector-guard-offset argument, so the default offset is used. The
kernel then panics the first time a function with stack protector is called.

I'm seeing this on powerpc64. It looks like it was reported for powerpc on
kernel bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201891

Linux 4.20 does not have a "prepare" target when KBUILD_EXTMOD is set. One is
added by:

  commit e07db28eea38ed4e332b3a89f3995c86b713cb5b
  Author: Masahiro Yamada <yamada.masahiro@socionext.com>
  Date:   Thu Nov 22 08:11:54 2018 +0900

      kbuild: fix single target build for external module

However, after cherry-picking that patch, the build fails because it's missing
prepare0. I applied the patch below and I successfully built an out-of-tree
module with CONFIG_STACKPROTECTOR=y.

diff --git a/Makefile b/Makefile
index 826826553085..f0a93e1ba1b6 100644
--- a/Makefile
+++ b/Makefile
@@ -1596,9 +1596,10 @@ help:
        @echo  ''

 # Dummies...
-PHONY += prepare scripts
-prepare:
+PHONY += prepare prepare0 scripts
+prepare: prepare0
        $(cmd_crmodverdir)
+prepare0: ;
 scripts: ;
 endif # KBUILD_EXTMOD

The context has been changed some in later patches, but I think a change like
this one should go into 5.0, and it e07db28eea38 should go into 4.20.y.

Thanks,
Samuel

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

* Re: [PATCH v5 1/2] powerpc/32: add stack protector support
@ 2019-01-12 23:32   ` Samuel Holland
  0 siblings, 0 replies; 7+ messages in thread
From: Samuel Holland @ 2019-01-12 23:32 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Masahiro Yamada
  Cc: Jason A. Donenfeld, linuxppc-dev, linux-kernel

Hello all,

On 09/27/18 02:05, Christophe Leroy wrote:
[..snip..]
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 07d9dce7eda6..45b8eb4d8fe7 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -112,6 +112,9 @@ KBUILD_LDFLAGS	+= -m elf$(BITS)$(LDEMULATION)
>  KBUILD_ARFLAGS	+= --target=elf$(BITS)-$(GNUTARGET)
>  endif
>  
> +cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard=tls
> +cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r2
> +
>  LDFLAGS_vmlinux-y := -Bstatic
>  LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
>  LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
> @@ -404,6 +407,13 @@ archclean:
>  
>  archprepare: checkbin
>  
> +ifdef CONFIG_STACKPROTECTOR
> +prepare: stack_protector_prepare
> +
> +stack_protector_prepare: prepare0
> +	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
> +endif
> +

This breaks when building out-of-tree kernel modules. GCC is not getting passed
the -mstack-protector-guard-offset argument, so the default offset is used. The
kernel then panics the first time a function with stack protector is called.

I'm seeing this on powerpc64. It looks like it was reported for powerpc on
kernel bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201891

Linux 4.20 does not have a "prepare" target when KBUILD_EXTMOD is set. One is
added by:

  commit e07db28eea38ed4e332b3a89f3995c86b713cb5b
  Author: Masahiro Yamada <yamada.masahiro@socionext.com>
  Date:   Thu Nov 22 08:11:54 2018 +0900

      kbuild: fix single target build for external module

However, after cherry-picking that patch, the build fails because it's missing
prepare0. I applied the patch below and I successfully built an out-of-tree
module with CONFIG_STACKPROTECTOR=y.

diff --git a/Makefile b/Makefile
index 826826553085..f0a93e1ba1b6 100644
--- a/Makefile
+++ b/Makefile
@@ -1596,9 +1596,10 @@ help:
        @echo  ''

 # Dummies...
-PHONY += prepare scripts
-prepare:
+PHONY += prepare prepare0 scripts
+prepare: prepare0
        $(cmd_crmodverdir)
+prepare0: ;
 scripts: ;
 endif # KBUILD_EXTMOD

The context has been changed some in later patches, but I think a change like
this one should go into 5.0, and it e07db28eea38 should go into 4.20.y.

Thanks,
Samuel

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

* Re: [PATCH v5 1/2] powerpc/32: add stack protector support
  2019-01-12 23:32   ` Samuel Holland
@ 2019-01-14  0:46     ` Michael Ellerman
  -1 siblings, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2019-01-14  0:46 UTC (permalink / raw)
  To: Samuel Holland, Christophe Leroy, Benjamin Herrenschmidt,
	Paul Mackerras, Masahiro Yamada
  Cc: linux-kernel, linuxppc-dev, Jason A. Donenfeld, Alexey Kardashevskiy

Samuel Holland <samuel@sholland.org> writes:
> Hello all,
>
> On 09/27/18 02:05, Christophe Leroy wrote:
> [..snip..]
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index 07d9dce7eda6..45b8eb4d8fe7 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -112,6 +112,9 @@ KBUILD_LDFLAGS	+= -m elf$(BITS)$(LDEMULATION)
>>  KBUILD_ARFLAGS	+= --target=elf$(BITS)-$(GNUTARGET)
>>  endif
>>  
>> +cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard=tls
>> +cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r2
>> +
>>  LDFLAGS_vmlinux-y := -Bstatic
>>  LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
>>  LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
>> @@ -404,6 +407,13 @@ archclean:
>>  
>>  archprepare: checkbin
>>  
>> +ifdef CONFIG_STACKPROTECTOR
>> +prepare: stack_protector_prepare
>> +
>> +stack_protector_prepare: prepare0
>> +	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
>> +endif
>> +
>
> This breaks when building out-of-tree kernel modules. GCC is not getting passed
> the -mstack-protector-guard-offset argument, so the default offset is used. The
> kernel then panics the first time a function with stack protector is called.
>
> I'm seeing this on powerpc64. It looks like it was reported for powerpc on
> kernel bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201891

Thanks for the bug report.

Alexey also hit this and sent an RFC fix.

  https://patchwork.ozlabs.org/patch/1022751/

But Masahiro thought we could do something less hacky, and hopefully he
can help us come up with something better.

cheers

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

* Re: [PATCH v5 1/2] powerpc/32: add stack protector support
@ 2019-01-14  0:46     ` Michael Ellerman
  0 siblings, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2019-01-14  0:46 UTC (permalink / raw)
  To: Samuel Holland, Christophe Leroy, Benjamin Herrenschmidt,
	Paul Mackerras, Masahiro Yamada
  Cc: Alexey Kardashevskiy, Jason A. Donenfeld, linuxppc-dev, linux-kernel

Samuel Holland <samuel@sholland.org> writes:
> Hello all,
>
> On 09/27/18 02:05, Christophe Leroy wrote:
> [..snip..]
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index 07d9dce7eda6..45b8eb4d8fe7 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -112,6 +112,9 @@ KBUILD_LDFLAGS	+= -m elf$(BITS)$(LDEMULATION)
>>  KBUILD_ARFLAGS	+= --target=elf$(BITS)-$(GNUTARGET)
>>  endif
>>  
>> +cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard=tls
>> +cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r2
>> +
>>  LDFLAGS_vmlinux-y := -Bstatic
>>  LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
>>  LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
>> @@ -404,6 +407,13 @@ archclean:
>>  
>>  archprepare: checkbin
>>  
>> +ifdef CONFIG_STACKPROTECTOR
>> +prepare: stack_protector_prepare
>> +
>> +stack_protector_prepare: prepare0
>> +	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
>> +endif
>> +
>
> This breaks when building out-of-tree kernel modules. GCC is not getting passed
> the -mstack-protector-guard-offset argument, so the default offset is used. The
> kernel then panics the first time a function with stack protector is called.
>
> I'm seeing this on powerpc64. It looks like it was reported for powerpc on
> kernel bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201891

Thanks for the bug report.

Alexey also hit this and sent an RFC fix.

  https://patchwork.ozlabs.org/patch/1022751/

But Masahiro thought we could do something less hacky, and hopefully he
can help us come up with something better.

cheers

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

end of thread, other threads:[~2019-01-14  0:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-27  7:05 [PATCH v5 1/2] powerpc/32: add stack protector support Christophe Leroy
2018-09-27  7:05 ` [PATCH v5 2/2] powerpc/64: " Christophe Leroy
2018-10-04  6:14 ` [v5,1/2] powerpc/32: " Michael Ellerman
2019-01-12 23:32 ` [PATCH v5 1/2] " Samuel Holland
2019-01-12 23:32   ` Samuel Holland
2019-01-14  0:46   ` Michael Ellerman
2019-01-14  0:46     ` Michael Ellerman

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.