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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/c3ff2a5193fa61b1b284cfb1d79628
cheers
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 <[email protected]>
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
Samuel Holland <[email protected]> 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