From: Dan Li <ashimida@linux.alibaba.com>
To: Nathan Chancellor <nathan@kernel.org>
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: Tue, 21 Sep 2021 02:07:17 +0800 [thread overview]
Message-ID: <1e00d088-4ced-d345-63b0-7428e9b8452a@linux.alibaba.com> (raw)
In-Reply-To: <YUeva0jP7P2qCr+R@archlinux-ax161>
Hi Nathan,
Thanks for your comments.
I rewrite the configuration as follows:
1) Change the plugin to be enabled by default, and add this option to CC_FLAGS_SCS to keep its behavior consistent with clang
---
diff --git a/Makefile b/Makefile
@@ -923,12 +923,6 @@ KBUILD_CFLAGS_KERNEL += -ffunction-sections -fdata-sections
-ifdef CONFIG_SHADOW_CALL_STACK
-CC_FLAGS_SCS := -fsanitize=shadow-call-stack
-KBUILD_CFLAGS += $(CC_FLAGS_SCS)
-export CC_FLAGS_SCS
-endif
@@ -1034,6 +1028,20 @@ include-$(CONFIG_GCC_PLUGINS) += scripts/Makefile.gcc-plugins
include $(addprefix $(srctree)/, $(include-y))
+ifdef CONFIG_SHADOW_CALL_STACK
+
+ifdef CONFIG_CC_IS_CLANG
+CC_FLAGS_SCS := -fsanitize=shadow-call-stack
+endif
+
+ifdef CONFIG_CC_IS_GCC
+CC_FLAGS_SCS := $(ENABLE_SHADOW_CALL_STACK_PLUGIN)
+endif
+
+KBUILD_CFLAGS += $(CC_FLAGS_SCS)
+export CC_FLAGS_SCS
+endif
diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
@@ -46,6 +46,13 @@ ifdef CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK
+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
+ifdef CONFIG_GCC_PLUGIN_SHADOW_CALL_STACK
+ ENABLE_SHADOW_CALL_STACK_PLUGIN += -fplugin-arg-arm64_scs_plugin-enable
+endif
2) Whether SCS is turned on or not is determined by CONFIG_SHADOW_CALL_STACK
* GCC_PLUGIN_SHADOW_CALL_STACK is only used to indicate whether current platform needs the support of the gcc SCS plugin
- It only enabled on ARM64 platform with gcc which does not support SCS(!CC_HAVE_SHADOW_CALL_STACK)
- If one compiler supports SCS (clang or gcc), then CC_HAVE_SHADOW_CALL_STACK should be true at this time, and the plugin is automatically closed
* As long as the current platform can support SCS(compiler or plugin), ARCH_SUPPORTS_SHADOW_CALL_STACK is always selected
* CONFIG_SHADOW_CALL_STACK no longer depends on CC_IS_CLANG
---
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
@@ -19,6 +19,15 @@ menuconfig GCC_PLUGINS
+config GCC_PLUGIN_SHADOW_CALL_STACK
+ bool "GCC Shadow Call Stack plugin"
+ depends on (!CC_HAVE_SHADOW_CALL_STACK) && ARM64
+ default y
+ help ....
diff --git 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 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/arch/arm64/Kconfig b/arch/arm64/Kconfig
@@ -81,7 +81,7 @@ config ARM64
- select ARCH_SUPPORTS_SHADOW_CALL_STACK if CC_HAVE_SHADOW_CALL_STACK
+ select ARCH_SUPPORTS_SHADOW_CALL_STACK if (CC_HAVE_SHADOW_CALL_STACK || GCC_PLUGIN_SHADOW_CALL_STACK)
select ARCH_SUPPORTS_LTO_CLANG if CPU_LITTLE_ENDIAN
@@ -1060,9 +1060,13 @@ config HW_PERF_EVENTS
# Supported by clang >= 7.0
config CC_HAVE_SHADOW_CALL_STACK
- def_bool $(cc-option, -fsanitize=shadow-call-stack -ffixed-x18)
+ def_bool (CC_IS_CLANG && $(cc-option, -fsanitize=shadow-call-stack -ffixed-x18))
On 9/20/21 5:45 AM, Nathan Chancellor wrote:
> Hi Dan,
>> diff --git a/Makefile b/Makefile
>> 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.
In the new code, translation unit can only enable SCS when CC_FLAGS_SCS is specified.
This behavior will be consistent with clang.
If there are other problems in the future, those two can be modified together.
>
>> -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
>
It's modified to rely only on ARCH_SUPPORTS_SHADOW_CALL_STACK
>> --- 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 is removed from the code above
>> + 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-20 18:09 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
2021-09-20 18:07 ` Dan Li [this message]
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=1e00d088-4ced-d345-63b0-7428e9b8452a@linux.alibaba.com \
--to=ashimida@linux.alibaba.com \
--cc=akpm@linux-foundation.org \
--cc=andreyknvl@gmail.com \
--cc=ardb@kernel.org \
--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=nathan@kernel.org \
--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).