Messages in this thread Patch in this message | | | Date | Sun, 5 Jul 2020 13:40:30 -0700 | From | Kees Cook <> | Subject | Re: [RFC PATCH] riscv: enable per-task stack canaries |
| |
On Sun, Jul 05, 2020 at 02:13:17PM +0000, guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > After compare arm64 and x86 implementations, seems arm64's is more > flexible and readable. The key point is how gcc get the offset of > stack_canary from gs/el0_sp. > > x86: Use a fix offset from gs, not flexible. > > struct fixed_percpu_data { > /* > * GCC hardcodes the stack canary as %gs:40. Since the > * irq_stack is the object at %gs:0, we reserve the bottom > * 48 bytes of the irq stack for the canary. > */ > char gs_base[40]; // :( > unsigned long stack_canary; > };
Yes, x86's compiler's implementation of "thread local" stack canary isn't great for the kernel.
> arm64: Use -mstack-protector-guard-offset & guard-reg > > 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 > > I prefer arm64, but x86 percpu_data design needs to be considered ?
I don't know riscv internals, so I leave that to y'all! :)
> After the discussion, let's continue the work for riscv gcc > stack-protector.
I think you'll need some buy-in from GCC before this kernel patch can land.
> Here is arm64 gcc's work [1]. > > [1] https://github.com/gcc-mirror/gcc/commit/cd0b2d361df82c848dc7e1c3078651bb0624c3c6
Can this kind of thing be made general-purposes, instead of having to reimplement it each time there's a new arch wanting to do it?
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Cc: Kees Cook <keescook@chromium.org> > --- > arch/riscv/Kconfig | 7 +++++++ > arch/riscv/Makefile | 10 ++++++++++ > arch/riscv/include/asm/stackprotector.h | 3 ++- > arch/riscv/kernel/asm-offsets.c | 3 +++ > arch/riscv/kernel/process.c | 2 +- > 5 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 4b0e308..4b4e833 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -394,6 +394,13 @@ config CMDLINE_FORCE > > endchoice > > +config CC_HAVE_STACKPROTECTOR_SYSREG > + def_bool $(cc-option,-mstack-protector-guard=gpr -mstack-protector-guard-reg=tp -mstack-protector-guard-offset=0)
And, as I'm sure you realize, it's not supported by the riscv backend yet:
riscv64-unknown-linux-gnu-gcc: error: unrecognized command line option '-mstack-protector-guard=gpr'; did you mean '-fstack-protector-strong'? riscv64-unknown-linux-gnu-gcc: error: unrecognized command line option '-mstack-protector-guard-reg=tp'; did you mean '-fstack-protector-strong'? riscv64-unknown-linux-gnu-gcc: error: unrecognized command line option '-mstack-protector-guard-offset=0'; did you mean '-fstack-protector-strong'?
-- Kees Cook
| |