* [PATCH v2] arm64: enable per-task stack canaries
@ 2018-12-03 17:03 Ard Biesheuvel
2018-12-03 20:53 ` Kees Cook
2018-12-07 15:31 ` Will Deacon
0 siblings, 2 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2018-12-03 17:03 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, linux-hardened, keescook, arnd, Ard Biesheuvel,
will.deacon, Ramana.Radhakrishnan, labbott
This enables the use of per-task stack canary values if GCC has
support for emitting the stack canary reference relative to the
value of sp_el0, which holds the task struct pointer in the arm64
kernel.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Note that the cc-option invocation below relies on the fact that Ramana's
current implementation of the GCC support permits -mstack-protector-guard=sysreg
to appear without defining the register name or offset.
The $(eval) extends KBUILD_CFLAGS at the moment the make rule is applied,
which means asm-offsets.o (which we rely on for the offset value) is built
without the arguments, and everything built afterwards has the options set.
arch/arm64/Kconfig | 5 +++++
arch/arm64/Makefile | 10 ++++++++++
arch/arm64/include/asm/stackprotector.h | 3 ++-
arch/arm64/kernel/asm-offsets.c | 3 +++
arch/arm64/kernel/process.c | 2 +-
5 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index ea2ab0330e3a..0d7fb47fe5e1 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1272,6 +1272,11 @@ config RANDOMIZE_MODULE_REGION_FULL
a limited range that contains the [_stext, _etext] interval of the
core kernel, so branch relocations are always in range.
+config STACKPROTECTOR_PER_TASK
+ def_bool y
+ depends on STACKPROTECTOR
+ depends on $(cc-option,-mstack-protector-guard=sysreg)
+
endmenu
menu "Boot options"
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 6cb9fc7e9382..79d927542322 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -56,6 +56,16 @@ KBUILD_AFLAGS += $(lseinstr) $(brokengasinst)
KBUILD_CFLAGS += $(call cc-option,-mabi=lp64)
KBUILD_AFLAGS += $(call cc-option,-mabi=lp64)
+ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y)
+prepare: stack_protector_prepare
+stack_protector_prepare: prepare0
+ $(eval KBUILD_CFLAGS += -mstack-protector-guard=sysreg \
+ -mstack-protector-guard-reg=sp_el0 \
+ -mstack-protector-guard-offset=$(shell \
+ awk '{if ($$2 == "TSK_STACK_CANARY") print $$3;}' \
+ include/generated/asm-offsets.h))
+endif
+
ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
KBUILD_CPPFLAGS += -mbig-endian
CHECKFLAGS += -D__AARCH64EB__
diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
index 58d15be11c4d..5884a2b02827 100644
--- a/arch/arm64/include/asm/stackprotector.h
+++ b/arch/arm64/include/asm/stackprotector.h
@@ -34,7 +34,8 @@ static __always_inline void boot_init_stack_canary(void)
canary &= CANARY_MASK;
current->stack_canary = canary;
- __stack_chk_guard = current->stack_canary;
+ if (!IS_ENABLED(CONFIG_STACKPROTECTOR_PER_TASK))
+ __stack_chk_guard = current->stack_canary;
}
#endif /* _ASM_STACKPROTECTOR_H */
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 323aeb5f2fe6..65b8afc84466 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -46,6 +46,9 @@ int main(void)
DEFINE(TSK_TI_TTBR0, offsetof(struct task_struct, thread_info.ttbr0));
#endif
DEFINE(TSK_STACK, offsetof(struct task_struct, stack));
+#ifdef CONFIG_STACKPROTECTOR
+ DEFINE(TSK_STACK_CANARY, offsetof(struct task_struct, stack_canary));
+#endif
BLANK();
DEFINE(THREAD_CPU_CONTEXT, offsetof(struct task_struct, thread.cpu_context));
BLANK();
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index d9a4c2d6dd8b..8a2d68f04e0d 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -59,7 +59,7 @@
#include <asm/processor.h>
#include <asm/stacktrace.h>
-#ifdef CONFIG_STACKPROTECTOR
+#if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
#include <linux/stackprotector.h>
unsigned long __stack_chk_guard __read_mostly;
EXPORT_SYMBOL(__stack_chk_guard);
--
2.19.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] arm64: enable per-task stack canaries
2018-12-03 17:03 [PATCH v2] arm64: enable per-task stack canaries Ard Biesheuvel
@ 2018-12-03 20:53 ` Kees Cook
2018-12-03 20:56 ` Ard Biesheuvel
2018-12-07 15:31 ` Will Deacon
1 sibling, 1 reply; 5+ messages in thread
From: Kees Cook @ 2018-12-03 20:53 UTC (permalink / raw)
To: Ard Biesheuvel, Masahiro Yamada
Cc: Mark Rutland, linux-hardened, Arnd Bergmann, Will Deacon,
Ramana Radhakrishnan, linux-arm-kernel, Laura Abbott
On Mon, Dec 3, 2018 at 9:04 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> This enables the use of per-task stack canary values if GCC has
> support for emitting the stack canary reference relative to the
> value of sp_el0, which holds the task struct pointer in the arm64
> kernel.
Yay!
(Should we include the gcc plugin version of this too, since it may be
a while before gcc with this support is available widely?)
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Note that the cc-option invocation below relies on the fact that Ramana's
> current implementation of the GCC support permits -mstack-protector-guard=sysreg
> to appear without defining the register name or offset.
>
> The $(eval) extends KBUILD_CFLAGS at the moment the make rule is applied,
> which means asm-offsets.o (which we rely on for the offset value) is built
> without the arguments, and everything built afterwards has the options set.
Can we detect this from Kconfig instead and then remove the flag for
the asm-offsets.o build step?
-Kees
>
> arch/arm64/Kconfig | 5 +++++
> arch/arm64/Makefile | 10 ++++++++++
> arch/arm64/include/asm/stackprotector.h | 3 ++-
> arch/arm64/kernel/asm-offsets.c | 3 +++
> arch/arm64/kernel/process.c | 2 +-
> 5 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index ea2ab0330e3a..0d7fb47fe5e1 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1272,6 +1272,11 @@ config RANDOMIZE_MODULE_REGION_FULL
> a limited range that contains the [_stext, _etext] interval of the
> core kernel, so branch relocations are always in range.
>
> +config STACKPROTECTOR_PER_TASK
> + def_bool y
> + depends on STACKPROTECTOR
> + depends on $(cc-option,-mstack-protector-guard=sysreg)
> +
> endmenu
>
> menu "Boot options"
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 6cb9fc7e9382..79d927542322 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -56,6 +56,16 @@ KBUILD_AFLAGS += $(lseinstr) $(brokengasinst)
> KBUILD_CFLAGS += $(call cc-option,-mabi=lp64)
> KBUILD_AFLAGS += $(call cc-option,-mabi=lp64)
>
> +ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y)
> +prepare: stack_protector_prepare
> +stack_protector_prepare: prepare0
> + $(eval KBUILD_CFLAGS += -mstack-protector-guard=sysreg \
> + -mstack-protector-guard-reg=sp_el0 \
> + -mstack-protector-guard-offset=$(shell \
> + awk '{if ($$2 == "TSK_STACK_CANARY") print $$3;}' \
> + include/generated/asm-offsets.h))
> +endif
> +
> ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
> KBUILD_CPPFLAGS += -mbig-endian
> CHECKFLAGS += -D__AARCH64EB__
> diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
> index 58d15be11c4d..5884a2b02827 100644
> --- a/arch/arm64/include/asm/stackprotector.h
> +++ b/arch/arm64/include/asm/stackprotector.h
> @@ -34,7 +34,8 @@ static __always_inline void boot_init_stack_canary(void)
> canary &= CANARY_MASK;
>
> current->stack_canary = canary;
> - __stack_chk_guard = current->stack_canary;
> + if (!IS_ENABLED(CONFIG_STACKPROTECTOR_PER_TASK))
> + __stack_chk_guard = current->stack_canary;
> }
>
> #endif /* _ASM_STACKPROTECTOR_H */
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index 323aeb5f2fe6..65b8afc84466 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -46,6 +46,9 @@ int main(void)
> DEFINE(TSK_TI_TTBR0, offsetof(struct task_struct, thread_info.ttbr0));
> #endif
> DEFINE(TSK_STACK, offsetof(struct task_struct, stack));
> +#ifdef CONFIG_STACKPROTECTOR
> + DEFINE(TSK_STACK_CANARY, offsetof(struct task_struct, stack_canary));
> +#endif
> BLANK();
> DEFINE(THREAD_CPU_CONTEXT, offsetof(struct task_struct, thread.cpu_context));
> BLANK();
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index d9a4c2d6dd8b..8a2d68f04e0d 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -59,7 +59,7 @@
> #include <asm/processor.h>
> #include <asm/stacktrace.h>
>
> -#ifdef CONFIG_STACKPROTECTOR
> +#if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
> #include <linux/stackprotector.h>
> unsigned long __stack_chk_guard __read_mostly;
> EXPORT_SYMBOL(__stack_chk_guard);
> --
> 2.19.2
>
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] arm64: enable per-task stack canaries
2018-12-03 20:53 ` Kees Cook
@ 2018-12-03 20:56 ` Ard Biesheuvel
2018-12-03 20:58 ` Ard Biesheuvel
0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2018-12-03 20:56 UTC (permalink / raw)
To: Kees Cook
Cc: Mark Rutland, linux-hardened, Arnd Bergmann, Will Deacon,
Masahiro Yamada, Ramana Radhakrishnan, linux-arm-kernel,
Laura Abbott
On Mon, 3 Dec 2018 at 21:54, Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Dec 3, 2018 at 9:04 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > This enables the use of per-task stack canary values if GCC has
> > support for emitting the stack canary reference relative to the
> > value of sp_el0, which holds the task struct pointer in the arm64
> > kernel.
>
> Yay!
>
> (Should we include the gcc plugin version of this too, since it may be
> a while before gcc with this support is available widely?)
>
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > Note that the cc-option invocation below relies on the fact that Ramana's
> > current implementation of the GCC support permits -mstack-protector-guard=sysreg
> > to appear without defining the register name or offset.
> >
> > The $(eval) extends KBUILD_CFLAGS at the moment the make rule is applied,
> > which means asm-offsets.o (which we rely on for the offset value) is built
> > without the arguments, and everything built afterwards has the options set.
>
> Can we detect this from Kconfig instead and then remove the flag for
> the asm-offsets.o build step?
>
We need to know offsetof(struct task_struct, stack_canary) at Kconfig
time, which could be affected by struct randomization as well, I
suppose. So it is not clear to me how we could ever drop this step.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] arm64: enable per-task stack canaries
2018-12-03 20:56 ` Ard Biesheuvel
@ 2018-12-03 20:58 ` Ard Biesheuvel
0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2018-12-03 20:58 UTC (permalink / raw)
To: Kees Cook
Cc: Mark Rutland, linux-hardened, Arnd Bergmann, Will Deacon,
Masahiro Yamada, Ramana Radhakrishnan, linux-arm-kernel,
Laura Abbott
On Mon, 3 Dec 2018 at 21:56, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Mon, 3 Dec 2018 at 21:54, Kees Cook <keescook@chromium.org> wrote:
> >
> > On Mon, Dec 3, 2018 at 9:04 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > >
> > > This enables the use of per-task stack canary values if GCC has
> > > support for emitting the stack canary reference relative to the
> > > value of sp_el0, which holds the task struct pointer in the arm64
> > > kernel.
> >
> > Yay!
> >
> > (Should we include the gcc plugin version of this too, since it may be
> > a while before gcc with this support is available widely?)
> >
> > >
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > ---
> > > Note that the cc-option invocation below relies on the fact that Ramana's
> > > current implementation of the GCC support permits -mstack-protector-guard=sysreg
> > > to appear without defining the register name or offset.
> > >
> > > The $(eval) extends KBUILD_CFLAGS at the moment the make rule is applied,
> > > which means asm-offsets.o (which we rely on for the offset value) is built
> > > without the arguments, and everything built afterwards has the options set.
> >
> > Can we detect this from Kconfig instead and then remove the flag for
> > the asm-offsets.o build step?
> >
>
> We need to know offsetof(struct task_struct, stack_canary) at Kconfig
> time, which could be affected by struct randomization as well, I
> suppose. So it is not clear to me how we could ever drop this step.
Ah never mind. I see what you mean now.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] arm64: enable per-task stack canaries
2018-12-03 17:03 [PATCH v2] arm64: enable per-task stack canaries Ard Biesheuvel
2018-12-03 20:53 ` Kees Cook
@ 2018-12-07 15:31 ` Will Deacon
1 sibling, 0 replies; 5+ messages in thread
From: Will Deacon @ 2018-12-07 15:31 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: mark.rutland, linux-hardened, keescook, arnd,
Ramana.Radhakrishnan, linux-arm-kernel, labbott
On Mon, Dec 03, 2018 at 06:03:43PM +0100, Ard Biesheuvel wrote:
> This enables the use of per-task stack canary values if GCC has
> support for emitting the stack canary reference relative to the
> value of sp_el0, which holds the task struct pointer in the arm64
> kernel.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Note that the cc-option invocation below relies on the fact that Ramana's
> current implementation of the GCC support permits -mstack-protector-guard=sysreg
> to appear without defining the register name or offset.
>
> The $(eval) extends KBUILD_CFLAGS at the moment the make rule is applied,
> which means asm-offsets.o (which we rely on for the offset value) is built
> without the arguments, and everything built afterwards has the options set.
>
> arch/arm64/Kconfig | 5 +++++
> arch/arm64/Makefile | 10 ++++++++++
> arch/arm64/include/asm/stackprotector.h | 3 ++-
> arch/arm64/kernel/asm-offsets.c | 3 +++
> arch/arm64/kernel/process.c | 2 +-
> 5 files changed, 21 insertions(+), 2 deletions(-)
This looks really good to me, but I'm not sure what we should do next.
Ramana -- is your implementation stable, or are we likely to see changes
to the way the options are passed?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-12-07 15:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-03 17:03 [PATCH v2] arm64: enable per-task stack canaries Ard Biesheuvel
2018-12-03 20:53 ` Kees Cook
2018-12-03 20:56 ` Ard Biesheuvel
2018-12-03 20:58 ` Ard Biesheuvel
2018-12-07 15:31 ` Will Deacon
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.