From: Nathan Chancellor <nathan@kernel.org>
To: Dan Li <ashimida@linux.alibaba.com>
Cc: masahiroy@kernel.org, michal.lkml@markovi.net,
keescook@chromium.org, ndesaulniers@google.com,
akpm@linux-foundation.org, tglx@linutronix.de,
peterz@infradead.org, samitolvanen@google.com,
frederic@kernel.org, rppt@kernel.org, yifeifz2@illinois.edu,
viresh.kumar@linaro.org, colin.king@canonical.com,
andreyknvl@gmail.com, mark.rutland@arm.com, ojeda@kernel.org,
will@kernel.org, ardb@kernel.org, luc.vanoostenryck@gmail.com,
elver@google.com, nivedita@alum.mit.edu,
linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hardening@vger.kernel.org,
clang-built-linux@googlegroups.com
Subject: Re: [PATCH] [RFC/RFT]SCS:Add gcc plugin to support Shadow Call Stack
Date: Sun, 19 Sep 2021 14:45:15 -0700 [thread overview]
Message-ID: <YUeva0jP7P2qCr+R@archlinux-ax161> (raw)
In-Reply-To: <1632069436-25075-1-git-send-email-ashimida@linux.alibaba.com>
Hi Dan,
A couple of initial high level comments, I do not really feel qualified
to review the plugin itself.
On Mon, Sep 20, 2021 at 12:37:16AM +0800, Dan Li wrote:
> The Clang-based shadow call stack protection has been integrated into the
> mainline, but kernel compiled by gcc cannot enable this feature for now.
>
> This Patch supports gcc-based SCS protection by adding a plugin.
>
> For each function that x30 will be pushed onto the stack during execution,
> this plugin:
> 1) insert "str x30, [x18], #8" at the entry of the function to save x30
> to current SCS
> 2) insert "ldr x30, [x18, #-8]!" before the exit of this function to
> restore x30
>
> At present, this patch has been successfully compiled(based on defconfig)
> in the following gcc versions(if plugin is supported) and startup normally:
> * 6.3.1
> * 7.3.1
> * 7.5.0
> * 8.2.1
> * 9.2.0
> * 10.3.1
>
> with commands:
> make ARCH=arm64 defconfig
> ./scripts/config -e CONFIG_GCC_PLUGINS -e CONFIG_GCC_PLUGIN_SHADOW_CALL_STACK
> make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
>
> ---
> FYI:
> 1) The function can be used to test whether the shadow stack is effective:
> //noinline void __noscs scs_test(void)
> noinline void scs_test(void)
> {
> register unsigned long *sp asm("sp");
> unsigned long * lr = sp + 1;
>
> asm volatile("":::"x30");
> *lr = 0;
> }
>
> ffff800010012670 <scs_test>:
> ffff800010012670: f800865e str x30, [x18], #8
> ffff800010012674: a9bf7bfd stp x29, x30, [sp, #-16]!
> ffff800010012678: 910003fd mov x29, sp
> ffff80001001267c: f90007ff str xzr, [sp, #8]
> ffff800010012680: a8c17bfd ldp x29, x30, [sp], #16
> ffff800010012684: f85f8e5e ldr x30, [x18, #-8]!
> ffff800010012688: d65f03c0 ret
>
> If SCS protection is enabled, this function will return normally.
> If the function has __noscs attribute (scs disabled), it will crash due to 0
> address access.
>
> 2) Other tests are in progress ...
>
> Signed-off-by: Dan Li <ashimida@linux.alibaba.com>
> ---
> Makefile | 2 +-
> arch/Kconfig | 2 +-
> include/linux/compiler-gcc.h | 4 +
> scripts/Makefile.gcc-plugins | 4 +
> scripts/gcc-plugins/Kconfig | 8 ++
> scripts/gcc-plugins/arm64_scs_plugin.c | 256 +++++++++++++++++++++++++++++++++
> 6 files changed, 274 insertions(+), 2 deletions(-)
> create mode 100644 scripts/gcc-plugins/arm64_scs_plugin.c
>
> diff --git a/Makefile b/Makefile
> index 61741e9..0f0121a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -924,7 +924,7 @@ LDFLAGS_vmlinux += --gc-sections
> endif
>
> ifdef CONFIG_SHADOW_CALL_STACK
I would rather see this become
ifeq ($(CONFIG_SHADOW_CALL_STACK)$(CONFIG_CC_IS_CLANG), yy)
...
endif
rather than just avoiding assigning to CC_FLAGS_SCS.
However, how does disabling the shadow call stack plugin work for a
whole translation unit or directory? There are a few places where
CC_FLAGS_SCS are filtered out and I am not sure I see where that happens
here? It looks like the plugin has a disabled option but I do not see it
hooked in anywhere.
> -CC_FLAGS_SCS := -fsanitize=shadow-call-stack
> +CC_FLAGS_SCS := $(if $(CONFIG_CC_IS_CLANG),-fsanitize=shadow-call-stack,)
> KBUILD_CFLAGS += $(CC_FLAGS_SCS)
> export CC_FLAGS_SCS
> endif
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 98db634..81ff127 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -594,7 +594,7 @@ config ARCH_SUPPORTS_SHADOW_CALL_STACK
>
> config SHADOW_CALL_STACK
> bool "Clang Shadow Call Stack"
> - depends on CC_IS_CLANG && ARCH_SUPPORTS_SHADOW_CALL_STACK
> + depends on (CC_IS_CLANG && ARCH_SUPPORTS_SHADOW_CALL_STACK) || GCC_PLUGIN_SHADOW_CALL_STACK
Is this logic right? SHADOW_CALL_STACK is only supported by arm64 (as
they set ARCH_SUPPORTS_SHADOW_CALL_STACK) but now you are enabling it
for any architecture, even though it seems like it still only works on
arm64. I think this wants to be
depends on (CC_IS_CLANG || GCC_PLUGIN_SHADOW_CALL_STACK) && ARCH_SUPPORTS_SHADOW_CALL_STACK
> depends on DYNAMIC_FTRACE_WITH_REGS || !FUNCTION_GRAPH_TRACER
> help
> This option enables Clang's Shadow Call Stack, which uses a
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index cb9217f..426c8e5 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -50,6 +50,10 @@
> #define __latent_entropy __attribute__((latent_entropy))
> #endif
>
> +#if defined(SHADOW_CALL_STACK_PLUGIN) && !defined(__CHECKER__)
> +#define __noscs __attribute__((no_shadow_call_stack))
> +#endif
> +
> /*
> * calling noreturn functions, __builtin_unreachable() and __builtin_trap()
> * confuse the stack allocation in gcc, leading to overly large stack
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index 952e468..eeaf2c6 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -46,6 +46,10 @@ ifdef CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK
> endif
> export DISABLE_ARM_SSP_PER_TASK_PLUGIN
>
> +gcc-plugin-$(CONFIG_GCC_PLUGIN_SHADOW_CALL_STACK) += arm64_scs_plugin.so
> +gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_SHADOW_CALL_STACK) \
> + += -DSHADOW_CALL_STACK_PLUGIN
> +
> # All the plugin CFLAGS are collected here in case a build target needs to
> # filter them out of the KBUILD_CFLAGS.
> GCC_PLUGINS_CFLAGS := $(strip $(addprefix -fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y)) $(gcc-plugin-cflags-y))
> diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> index ab9eb4c..2534195e 100644
> --- a/scripts/gcc-plugins/Kconfig
> +++ b/scripts/gcc-plugins/Kconfig
> @@ -19,6 +19,14 @@ menuconfig GCC_PLUGINS
>
> if GCC_PLUGINS
>
> +config GCC_PLUGIN_SHADOW_CALL_STACK
> + bool "GCC Shadow Call Stack plugin"
This should also have a
depends on ARCH_SUPPORTS_SHADOW_CALL_STACK
if you are selecting SHADOW_CALL_STACK, as selecting does not account
for dependencies.
> + select SHADOW_CALL_STACK
> + help
> + This plugin is used to support the kernel CONFIG_SHADOW_CALL_STACK
> + compiled by gcc. Its principle is basically the same as that of CLANG.
> + For more information, please refer to "config SHADOW_CALL_STACK"
> +
> config GCC_PLUGIN_CYC_COMPLEXITY
> bool "Compute the cyclomatic complexity of a function" if EXPERT
> depends on !COMPILE_TEST # too noisy
Cheers,
Nathan
next prev parent reply other threads:[~2021-09-19 21:45 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-19 16:37 [PATCH] [RFC/RFT]SCS:Add gcc plugin to support Shadow Call Stack Dan Li
2021-09-19 21:45 ` Nathan Chancellor [this message]
2021-09-20 18:07 ` Dan Li
2021-09-20 7:18 ` Ard Biesheuvel
2021-09-20 18:53 ` Dan Li
2021-09-20 21:22 ` Ard Biesheuvel
2021-09-21 6:00 ` Dan Li
2021-09-22 13:48 ` Ard Biesheuvel
2021-09-23 15:25 ` Dan Li
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=YUeva0jP7P2qCr+R@archlinux-ax161 \
--to=nathan@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=andreyknvl@gmail.com \
--cc=ardb@kernel.org \
--cc=ashimida@linux.alibaba.com \
--cc=clang-built-linux@googlegroups.com \
--cc=colin.king@canonical.com \
--cc=elver@google.com \
--cc=frederic@kernel.org \
--cc=keescook@chromium.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luc.vanoostenryck@gmail.com \
--cc=mark.rutland@arm.com \
--cc=masahiroy@kernel.org \
--cc=michal.lkml@markovi.net \
--cc=ndesaulniers@google.com \
--cc=nivedita@alum.mit.edu \
--cc=ojeda@kernel.org \
--cc=peterz@infradead.org \
--cc=rppt@kernel.org \
--cc=samitolvanen@google.com \
--cc=tglx@linutronix.de \
--cc=viresh.kumar@linaro.org \
--cc=will@kernel.org \
--cc=yifeifz2@illinois.edu \
/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).