On Sun, Jul 05, 2020 at 02:13:17PM +0000, guoren@xxxxxxxxxx wrote:
> From: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
> 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
