From: Kees Cook <keescook@chromium.org>
To: guoren@kernel.org
Cc: palmerdabbelt@google.com, paul.walmsley@sifive.com,
anup@brainfault.org, greentime.hu@sifive.com, zong.li@sifive.com,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
linux-csky@vger.kernel.org, "Guo Ren" <guoren@linux.alibaba.com>,
"Albert Ou" <aou@eecs.berkeley.edu>,
"Masami Hiramatsu" <mhiramat@kernel.org>,
"Björn Töpel" <bjorn.topel@gmail.com>,
"Greentime Hu" <green.hu@gmail.com>,
"Atish Patra" <atish.patra@wdc.com>
Subject: Re: [PATCH] riscv: Add STACKPROTECTOR supported
Date: Sat, 4 Jul 2020 23:53:45 -0700 [thread overview]
Message-ID: <202007042350.4C153C4F8@keescook> (raw)
In-Reply-To: <1593930255-12378-1-git-send-email-guoren@kernel.org>
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
next prev parent reply other threads:[~2020-07-05 6:53 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-05 6:24 [PATCH] riscv: Add STACKPROTECTOR supported guoren
2020-07-05 6:53 ` Kees Cook [this message]
2020-07-05 14:16 ` Guo Ren
2020-07-05 20:31 ` Kees Cook
2020-07-06 0:55 ` Guo Ren
2020-07-06 1:19 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202007042350.4C153C4F8@keescook \
--to=keescook@chromium.org \
--cc=anup@brainfault.org \
--cc=aou@eecs.berkeley.edu \
--cc=atish.patra@wdc.com \
--cc=bjorn.topel@gmail.com \
--cc=green.hu@gmail.com \
--cc=greentime.hu@sifive.com \
--cc=guoren@kernel.org \
--cc=guoren@linux.alibaba.com \
--cc=linux-csky@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mhiramat@kernel.org \
--cc=palmerdabbelt@google.com \
--cc=paul.walmsley@sifive.com \
--cc=zong.li@sifive.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).