Messages in this thread Patch in this message | | | Date | Sat, 4 Jul 2020 23:53:45 -0700 | From | Kees Cook <> | Subject | Re: [PATCH] riscv: Add STACKPROTECTOR supported |
| |
On Sun, Jul 05, 2020 at 06:24:15AM +0000, guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > The -fstack-protector & -fstack-protector-strong features are from > gcc. The patch only add basic kernel support to stack-protector > feature and some arch could have its own solution such as > ARM64_PTR_AUTH. > > After enabling STACKPROTECTOR and STACKPROTECTOR_STRONG, the .text > size is expanded from 0x7de066 to 0x81fb32 (only 5%) to add canary > checking code. > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Cc: Paul Walmsley <paul.walmsley@sifive.com> > Cc: Palmer Dabbelt <palmerdabbelt@google.com> > Cc: Albert Ou <aou@eecs.berkeley.edu> > Cc: Masami Hiramatsu <mhiramat@kernel.org> > Cc: Björn Töpel <bjorn.topel@gmail.com> > Cc: Greentime Hu <green.hu@gmail.com> > Cc: Atish Patra <atish.patra@wdc.com> > --- > arch/riscv/Kconfig | 1 + > arch/riscv/include/asm/stackprotector.h | 29 +++++++++++++++++++++++++++++ > arch/riscv/kernel/process.c | 6 ++++++ > 3 files changed, 36 insertions(+) > create mode 100644 arch/riscv/include/asm/stackprotector.h > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index f927a91..4b0e308 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -63,6 +63,7 @@ config RISCV > select HAVE_PERF_EVENTS > select HAVE_PERF_REGS > select HAVE_PERF_USER_STACK_DUMP > + select HAVE_STACKPROTECTOR > select HAVE_SYSCALL_TRACEPOINTS > select IRQ_DOMAIN > select MODULES_USE_ELF_RELA if MODULES > diff --git a/arch/riscv/include/asm/stackprotector.h b/arch/riscv/include/asm/stackprotector.h > new file mode 100644 > index 00000000..5962f88 > --- /dev/null > +++ b/arch/riscv/include/asm/stackprotector.h > @@ -0,0 +1,29 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#ifndef _ASM_RISCV_STACKPROTECTOR_H > +#define _ASM_RISCV_STACKPROTECTOR_H > + > +#include <linux/random.h> > +#include <linux/version.h> > + > +extern unsigned long __stack_chk_guard; > + > +/* > + * 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. */ > + get_random_bytes(&canary, sizeof(canary)); > + canary ^= LINUX_VERSION_CODE; > + canary &= CANARY_MASK; Does riscv have any kind of instruction counters or other trivial timers that could be mixed in here? (e.g. x86's TSC)
> + > + current->stack_canary = canary; > + __stack_chk_guard = current->stack_canary;
What's needed for riscv to support a per-task canary? (e.g. x86's TLS or arm64's register-specific methods)
> +} > +#endif /* _ASM_RISCV_STACKPROTECTOR_H */ > diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c > index 824d117..6548929 100644 > --- a/arch/riscv/kernel/process.c > +++ b/arch/riscv/kernel/process.c > @@ -24,6 +24,12 @@ > > register unsigned long gp_in_global __asm__("gp"); > > +#ifdef CONFIG_STACKPROTECTOR > +#include <linux/stackprotector.h> > +unsigned long __stack_chk_guard __read_mostly; > +EXPORT_SYMBOL(__stack_chk_guard); > +#endif > + > extern asmlinkage void ret_from_fork(void); > extern asmlinkage void ret_from_kernel_thread(void); > > -- > 2.7.4 >
But yes, as a starting point, better to have a single per-boot global canary than none at all. :)
Reviewed-by: Kees Cook <keescook@chromium.org>
-- Kees Cook
| |