This patch supports gcc-based SCS protection on ARM64 by adding a plugin.
For each function that x30 will be pushed onto the stack during execution,
this plugin, similar to gcc's pac implementation, will normally:
1) insert "str x30, [x18], #8" at the beginning of function's prologue
2) insert "ldr x30, [x18, #-8]!" immediately before function's
epilogue return/sibling calls
If pac is enabled, scs push/pop will be inserted between paciasp/autiasp.
At present, this patch has been successfully compiled in the following
gcc versions based on defconfig with kernel 5.14 and startup normally
with commands:
make ARCH=arm64 defconfig
./scripts/config -e CONFIG_GCC_PLUGINS -e CONFIG_SHADOW_CALL_STACK \
-e CONFIG_GCC_PLUGIN_SHADOW_CALL_STACK
make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
Tested gcc version:
* 6.3.1
* 7.3.1
* 7.5.0
* 8.2.1
* 9.2.0
* 10.3.1
Signed-off-by: Dan Li <[email protected]>
---
V4:
- change CC_HAVE_SHADOW_CALL_STACK def_bool
V3:
- fix incorrect description of the compilation option
V2:
- fix incorrect config dependency
- tested against DYNAMIC_FTRACE_WITH_REGS
- add support for sibling call
- add support for ARM64_PTR_AUTH_KERNEL/ARM64_BTI_KERNEL
FYI:
- The kernel compiled by Linaro GCC 7.3-2018.05 runs for more than 10
days, ltp> 24 hours without crash.
- This 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;
}
when compiled with:
CONFIG_DYNAMIC_FTRACE_WITH_REGS=y
CONFIG_ARM64_PTR_AUTH_KERNEL=y
CONFIG_ARM64_BTI_KERNEL=y
ffff800010013b60 <scs_test>:
ffff800010013b60: d503245f bti c
ffff800010013b64: d503201f nop
ffff800010013b68: d503201f nop
ffff800010013b6c: d503233f paciasp
ffff800010013b70: f800865e str x30, [x18], #8
ffff800010013b74: a9bf7bfd stp x29, x30, [sp, #-16]!
ffff800010013b78: 910003fd mov x29, sp
ffff800010013b7c: 910003e0 mov x0, sp
ffff800010013b80: f900041f str xzr, [x0, #8]
ffff800010013b84: a8c17bfd ldp x29, x30, [sp], #16
ffff800010013b88: f85f8e5e ldr x30, [x18, #-8]!
ffff800010013b8c: d50323bf autiasp
ffff800010013b90: 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.
Makefile | 20 ++-
arch/Kconfig | 13 +-
arch/arm64/Kconfig | 4 +-
include/linux/compiler-gcc.h | 4 +
scripts/Makefile.gcc-plugins | 7 +
scripts/gcc-plugins/Kconfig | 9 +
scripts/gcc-plugins/arm64_scs_plugin.c | 299 +++++++++++++++++++++++++++++++++
scripts/gcc-plugins/gcc-common.h | 4 +
8 files changed, 346 insertions(+), 14 deletions(-)
create mode 100644 scripts/gcc-plugins/arm64_scs_plugin.c
diff --git a/Makefile b/Makefile
index 61741e9..8039e61 100644
--- a/Makefile
+++ b/Makefile
@@ -923,12 +923,6 @@ KBUILD_CFLAGS_KERNEL += -ffunction-sections -fdata-sections
LDFLAGS_vmlinux += --gc-sections
endif
-ifdef CONFIG_SHADOW_CALL_STACK
-CC_FLAGS_SCS := -fsanitize=shadow-call-stack
-KBUILD_CFLAGS += $(CC_FLAGS_SCS)
-export CC_FLAGS_SCS
-endif
-
ifdef CONFIG_LTO_CLANG
ifdef CONFIG_LTO_CLANG_THIN
CC_FLAGS_LTO := -flto=thin -fsplit-lto-unit
@@ -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
+
# scripts/Makefile.gcc-plugins is intentionally included last.
# Do not add $(call cc-option,...) below this line. When you build the kernel
# from the clean source tree, the GCC plugins do not exist at this point.
diff --git a/arch/Kconfig b/arch/Kconfig
index 98db634..1065cd0 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -593,14 +593,15 @@ config ARCH_SUPPORTS_SHADOW_CALL_STACK
switching.
config SHADOW_CALL_STACK
- bool "Clang Shadow Call Stack"
- depends on CC_IS_CLANG && ARCH_SUPPORTS_SHADOW_CALL_STACK
+ bool "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
- shadow stack to protect function return addresses from being
- overwritten by an attacker. More information can be found in
- Clang's documentation:
+ This option enables Shadow Call Stack(supported as a compiler
+ option in the case of clang, supported as a plugin in the case
+ of gcc), which uses a shadow stack to protect function return
+ addresses from being overwritten by an attacker.
+ More information can be found in Clang's documentation:
https://clang.llvm.org/docs/ShadowCallStack.html
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 62c3c1d..8aaa16e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -81,7 +81,7 @@ config ARM64
select ARCH_SUPPORTS_DEBUG_PAGEALLOC
select ARCH_SUPPORTS_HUGETLBFS
select ARCH_SUPPORTS_MEMORY_FAILURE
- 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
select ARCH_SUPPORTS_LTO_CLANG_THIN
select ARCH_SUPPORTS_CFI_CLANG
@@ -1062,7 +1062,7 @@ config ARCH_HAS_FILTER_PGPROT
# 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)
config PARAVIRT
bool "Enable paravirtualization code"
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..b45bd8c 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -46,6 +46,13 @@ 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
+ifdef CONFIG_GCC_PLUGIN_SHADOW_CALL_STACK
+ ENABLE_SHADOW_CALL_STACK_PLUGIN += -fplugin-arg-arm64_scs_plugin-enable
+endif
+
# 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..92f7f76 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -19,6 +19,15 @@ menuconfig GCC_PLUGINS
if GCC_PLUGINS
+config GCC_PLUGIN_SHADOW_CALL_STACK
+ bool "Plugin for ARM64 Shadow Call Stack"
+ depends on (!CC_HAVE_SHADOW_CALL_STACK) && ARM64
+ help
+ This plugin is used to support 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
diff --git a/scripts/gcc-plugins/arm64_scs_plugin.c b/scripts/gcc-plugins/arm64_scs_plugin.c
new file mode 100644
index 0000000..bb72baa
--- /dev/null
+++ b/scripts/gcc-plugins/arm64_scs_plugin.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "gcc-common.h"
+
+#define v_info(fmt, ...) \
+ do { \
+ if (verbose) \
+ fprintf(stderr, "[SCS]:" fmt, ## __VA_ARGS__); \
+ } while (0)
+
+#define NOSCS_ATTR_STR "no_shadow_call_stack"
+#define SCS_ASM_PUSH_STR "str x30, [x18], #8\n\t"
+#define SCS_ASM_POP_STR "ldr x30, [x18, #-8]!\n\t"
+
+__visible int plugin_is_GPL_compatible;
+
+static struct plugin_info arm64_scs_plugin_info = {
+ .version = "20210926vanilla",
+ .help = "enable\tactivate plugin\n"
+ "verbose\tprint all debug infos\n",
+};
+
+static bool verbose;
+
+#if BUILDING_GCC_VERSION >= 10001
+enum insn_code paciasp_num = CODE_FOR_paciasp;
+enum insn_code autiasp_num = CODE_FOR_autiasp;
+#elif BUILDING_GCC_VERSION >= 7003
+enum insn_code paciasp_num = CODE_FOR_pacisp;
+enum insn_code autiasp_num = CODE_FOR_autisp;
+#else
+enum insn_code paciasp_num = CODE_FOR_nothing;
+enum insn_code autiasp_num = CODE_FOR_nothing;
+#define TARGET_ARMV8_3 0
+#endif
+
+static rtx_insn * (*old_gen_prologue)(void);
+static rtx_insn * (*old_gen_epilogue)(void);
+static rtx_insn * (*old_gen_sibcall_epilogue)(void);
+
+static rtx gen_scs_push(location_t loc)
+{
+ rtx insn = gen_rtx_ASM_INPUT_loc(VOIDmode, ggc_strdup(SCS_ASM_PUSH_STR), loc);
+
+ MEM_VOLATILE_P(insn) = 1;
+ return insn;
+}
+
+static rtx gen_scs_pop(location_t loc)
+{
+ rtx insn = gen_rtx_ASM_INPUT_loc(VOIDmode, ggc_strdup(SCS_ASM_POP_STR), loc);
+
+ MEM_VOLATILE_P(insn) = 1;
+ return insn;
+}
+
+static bool scs_func_ignored(void)
+{
+ bool is_ignored;
+
+#if BUILDING_GCC_VERSION >= 8002
+ is_ignored = !cfun->machine->frame.emit_frame_chain;
+#else
+ is_ignored = !frame_pointer_needed;
+#endif
+
+ /*
+ * Functions that do not push LR into stack are not protected.
+ * Functions that call __builin_eh_return is not protected(consistent with gcc's PAC).
+ */
+ if (is_ignored || crtl->calls_eh_return) {
+ v_info("No protection code inserted into func:%s in file:%s\n",
+ get_name(current_function_decl), main_input_filename);
+ return 1;
+ }
+
+ /* Functions with attribute NOSCS_ATTR_STR need to be unprotected */
+ if (lookup_attribute(NOSCS_ATTR_STR, DECL_ATTRIBUTES(current_function_decl))) {
+ v_info("No protection code inserted into %s func:%s in file:%s\n", NOSCS_ATTR_STR,
+ get_name(current_function_decl), main_input_filename);
+ return 1;
+ }
+
+ return 0;
+}
+
+static rtx_insn *search_insn(enum insn_code code, rtx_insn *seq)
+{
+ rtx_insn *insn;
+
+ for (insn = get_insns(); insn; insn = NEXT_INSN(insn)) {
+ if (code == recog(PATTERN(insn), insn, 0))
+ return insn;
+ }
+
+ return NULL;
+}
+
+static bool scs_return_address_signing_enabled(void)
+{
+#if BUILDING_GCC_VERSION >= 7003
+ return aarch64_return_address_signing_enabled();
+#else
+ return false;
+#endif
+}
+
+static rtx_insn *scs_gen_prologue(void)
+{
+ rtx_insn *seq = NULL, *mark;
+ rtx tmp;
+ bool ret_sign_enabled;
+
+ if (old_gen_prologue)
+ seq = old_gen_prologue();
+
+ if ((!seq) || scs_func_ignored())
+ return seq;
+
+ ret_sign_enabled = scs_return_address_signing_enabled();
+ tmp = gen_scs_push(RESERVED_LOCATION_COUNT);
+
+ start_sequence();
+ emit_insn(seq);
+
+ if (ret_sign_enabled) {
+ /* For functions with pac enabled, insert scs push after the 'paciasp' insn */
+ mark = search_insn(paciasp_num, get_insns());
+ if (!mark)
+ error(G_("Non-standard insn seqs found:\n"
+ "__noscs attr should be added on func:%s,file:%s\n"),
+ get_name(current_function_decl), main_input_filename);
+
+ emit_insn_after(tmp, mark);
+ } else {
+ /* For functions that do not enable pac, insert scs push at the start of insns */
+ mark = get_insns();
+ emit_insn_before(tmp, mark);
+ }
+
+ seq = get_insns();
+ end_sequence();
+ return seq;
+}
+
+static rtx_insn *scs_gen_epilogue(void)
+{
+ rtx_insn *seq = NULL, *mark;
+ rtx tmp;
+ bool ret_sign_enabled;
+
+ if (old_gen_epilogue)
+ seq = old_gen_epilogue();
+
+ if ((!seq) || scs_func_ignored())
+ return seq;
+
+ ret_sign_enabled = scs_return_address_signing_enabled();
+ tmp = gen_scs_pop(RESERVED_LOCATION_COUNT);
+
+ start_sequence();
+ emit_insn(seq);
+
+ if (ret_sign_enabled && (!TARGET_ARMV8_3)) {
+ /* For functions with pac enabled, if 'autiasp' is used in epilogue
+ * (!TARGET_ARMV8_3), scs pop should inserted before this insn.
+ */
+ mark = search_insn(autiasp_num, get_insns());
+ } else {
+ /* For functions do not enabled pac or used 'retaa' as pac check,
+ * scs pop inserted before the last 'return" insn
+ */
+ mark = get_last_insn();
+ }
+
+ if (!mark)
+ error(G_("Non-standard insn seqs found:\n"
+ "__noscs attr should be added on func:%s,file:%s\n"),
+ get_name(current_function_decl), main_input_filename);
+
+ emit_insn_before(tmp, mark);
+
+ seq = get_insns();
+ end_sequence();
+ return seq;
+}
+
+static rtx_insn *scs_gen_sibcall_epilogue(void)
+{
+ rtx_insn *seq = NULL, *mark;
+ rtx tmp;
+ bool ret_sign_enabled;
+
+ if (old_gen_sibcall_epilogue)
+ seq = old_gen_sibcall_epilogue();
+
+ if ((!seq) || scs_func_ignored())
+ return seq;
+
+ ret_sign_enabled = scs_return_address_signing_enabled();
+ tmp = gen_scs_pop(RESERVED_LOCATION_COUNT);
+
+ start_sequence();
+ emit_insn(seq);
+
+ if (ret_sign_enabled) {
+ /* If pac is enabled, sibling_call will always use 'autiasp' as pac check */
+ mark = search_insn(autiasp_num, get_insns());
+ if (!mark)
+ error(G_("Non-standard insn seqs found:\n"
+ "__noscs attr should be added on func:%s,file:%s\n"),
+ get_name(current_function_decl), main_input_filename);
+ emit_insn_before(tmp, mark);
+ } else {
+ /* If pac is disabled, insert scs pop at the end of insns */
+ mark = get_last_insn();
+ emit_insn_after(tmp, mark);
+ }
+
+ seq = get_insns();
+ end_sequence();
+
+ return seq;
+}
+
+static void callback_before_start_unit(void *gcc_data __unused, void *user_data __unused)
+{
+ old_gen_prologue = targetm.gen_prologue;
+ old_gen_epilogue = targetm.gen_epilogue;
+ old_gen_sibcall_epilogue = targetm.gen_sibcall_epilogue;
+
+ targetm.gen_prologue = scs_gen_prologue;
+ targetm.gen_epilogue = scs_gen_epilogue;
+ targetm.gen_sibcall_epilogue = scs_gen_sibcall_epilogue;
+}
+
+static tree handle_noscs_attribute(tree *node, tree name, tree args __unused, int flags,
+ bool *no_add_attrs)
+{
+ /* NOSCS_ATTR_STR can only be used for function declarations */
+ switch (TREE_CODE(*node)) {
+ case FUNCTION_DECL:
+ break;
+ default:
+ error(G_("%qE attribute can be applies to function decl only (%qE)"), name, *node);
+ gcc_unreachable();
+ }
+
+ *no_add_attrs = false;
+ return NULL_TREE;
+}
+
+static struct attribute_spec noscs_attr = {};
+
+static void scs_register_attributes(void *event_data __unused, void *data __unused)
+{
+ noscs_attr.name = NOSCS_ATTR_STR;
+ noscs_attr.decl_required = true;
+ noscs_attr.handler = handle_noscs_attribute;
+ register_attribute(&noscs_attr);
+}
+
+__visible int plugin_init(struct plugin_name_args *plugin_info, struct plugin_gcc_version *version)
+{
+ int i;
+ bool enable = false;
+ const char * const plugin_name = plugin_info->base_name;
+ const int argc = plugin_info->argc;
+ const struct plugin_argument * const argv = plugin_info->argv;
+
+ if (!plugin_default_version_check(version, &gcc_version)) {
+ error(G_("Incompatible gcc/plugin versions"));
+ return 1;
+ }
+
+ for (i = 0; i < argc; ++i) {
+ if (!strcmp(argv[i].key, "enable")) {
+ enable = true;
+ continue;
+ }
+ if (!strcmp(argv[i].key, "verbose")) {
+ verbose = true;
+ continue;
+ }
+ error(G_("unknown option '-fplugin-arg-%s-%s'"), plugin_name, argv[i].key);
+ }
+
+ if (!enable) {
+ v_info("Plugin disabled for file:%s\n", main_input_filename);
+ return 0;
+ }
+
+ register_callback(plugin_name, PLUGIN_INFO, NULL, &arm64_scs_plugin_info);
+
+ register_callback(plugin_name, PLUGIN_ATTRIBUTES, scs_register_attributes, NULL);
+
+ register_callback(plugin_name, PLUGIN_START_UNIT, callback_before_start_unit, NULL);
+
+ return 0;
+}
diff --git a/scripts/gcc-plugins/gcc-common.h b/scripts/gcc-plugins/gcc-common.h
index 0c08761..7251b00 100644
--- a/scripts/gcc-plugins/gcc-common.h
+++ b/scripts/gcc-plugins/gcc-common.h
@@ -27,6 +27,7 @@
#include "except.h"
#include "function.h"
#include "toplev.h"
+#include "insn-codes.h"
#if BUILDING_GCC_VERSION >= 5000
#include "expr.h"
#endif
@@ -535,6 +536,9 @@ static inline void ipa_remove_stmt_references(symtab_node *referring_node, gimpl
}
#endif
+/* RTL related */
+extern int recog(rtx, rtx_insn *, int *);
+
#if BUILDING_GCC_VERSION < 6000
#define get_inner_reference(exp, pbitsize, pbitpos, poffset, pmode, punsignedp, preversep, pvolatilep, keep_aligning) \
get_inner_reference(exp, pbitsize, pbitpos, poffset, pmode, punsignedp, pvolatilep, keep_aligning)
--
2.7.4
On Wed, Oct 13, 2021 at 4:28 PM Dan Li <[email protected]> wrote:
>
> - This 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;
> }
>
> when compiled with:
> CONFIG_DYNAMIC_FTRACE_WITH_REGS=y
> CONFIG_ARM64_PTR_AUTH_KERNEL=y
> CONFIG_ARM64_BTI_KERNEL=y
>
> ffff800010013b60 <scs_test>:
> ffff800010013b60: d503245f bti c
> ffff800010013b64: d503201f nop
> ffff800010013b68: d503201f nop
> ffff800010013b6c: d503233f paciasp
> ffff800010013b70: f800865e str x30, [x18], #8
> ffff800010013b74: a9bf7bfd stp x29, x30, [sp, #-16]!
> ffff800010013b78: 910003fd mov x29, sp
> ffff800010013b7c: 910003e0 mov x0, sp
> ffff800010013b80: f900041f str xzr, [x0, #8]
> ffff800010013b84: a8c17bfd ldp x29, x30, [sp], #16
> ffff800010013b88: f85f8e5e ldr x30, [x18, #-8]!
> ffff800010013b8c: d50323bf autiasp
> ffff800010013b90: 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.
> 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
Cool this is a nice addition, and something I don't think that clang
has. For any new feature, having a function attribute to disable it
at the function granularity is nice, and plays better with LTO than -f
group flags. Though that begs the question: what happens if a __noscs
callee is inlined into a non-__noscs caller, or vice versa?
I noticed that __noscs isn't actually applied anywhere in the kernel,
yet, at least in this series. Were there any places necessary that
you've found thus far?
Overall, I'm happy with the patch and am ready to ack it, but I would
like to see a link to to the upstream GCC feature request for SCS (and
one created if it doesn't exist) cited explicitly in the commit
message. I think that would be a good demonstration that this can or
will be upstreamed into the compiler proper for the compiler vendors
to maintain, rather than the kernel folks. The compiler vendors may
have further feedback on the approach, such as my question above
pertaining to inlining.
--
Thanks,
~Nick Desaulniers
On 10/15/21 2:44 AM, Nick Desaulniers wrote:
> On Wed, Oct 13, 2021 at 4:28 PM Dan Li <[email protected]> wrote:
>> --- 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
>
> Cool this is a nice addition, and something I don't think that clang
> has. For any new feature, having a function attribute to disable it
> at the function granularity is nice, and plays better with LTO than -f
> group flags. Though that begs the question: what happens if a __noscs
> callee is inlined into a non-__noscs caller, or vice versa?
Thanks Nick,
According to my understanding, all inline optimizations in gcc should
happen before inserting scs insns (scs and paciasp/autiasp use the
same insertion point). Therefore, the check for the __noscs attribute
will also occur after all inlining is completed.
As in the following example:
- Since __noscs attribute is specified, scs_test1 does not insert scs insns
- Since normal functions scs_test2/3 uses x30, it needs to insert scs insns
- Since __noscs attribute is specified, scs_test4 after inlining does not
need to insert scs insns
__always_inline __noscs void scs_test1(void)
{
asm volatile("mov x1, x1\n\t":::"x30");
}
//scs insns inserted after function inline
void scs_test2(void)
{
scs_test1();
}
__always_inline void scs_test3(void)
{
asm volatile("mov x3, x3\n\t":::"x30");
}
//no scs insns inserted
__noscs void scs_test4(void)
{
scs_test3();
}
ffff800010012900 <scs_test1>:
ffff800010012900: a9bf7bfd stp x29, x30, [sp, #-16]!
ffff800010012904: 910003fd mov x29, sp
ffff800010012908: aa0103e1 mov x1, x1
ffff80001001290c: a8c17bfd ldp x29, x30, [sp], #16
ffff800010012910: d65f03c0 ret
ffff800010012914 <scs_test2>:
ffff800010012914: f800865e str x30, [x18], #8
ffff800010012918: a9bf7bfd stp x29, x30, [sp, #-16]!
ffff80001001291c: 910003fd mov x29, sp
ffff800010012920: aa0103e1 mov x1, x1
ffff800010012924: a8c17bfd ldp x29, x30, [sp], #16
ffff800010012928: f85f8e5e ldr x30, [x18, #-8]!
ffff80001001292c: d65f03c0 ret
ffff800010012930 <scs_test3>:
ffff800010012930: f800865e str x30, [x18], #8
ffff800010012934: a9bf7bfd stp x29, x30, [sp, #-16]!
ffff800010012938: 910003fd mov x29, sp
ffff80001001293c: aa0303e3 mov x3, x3
ffff800010012940: a8c17bfd ldp x29, x30, [sp], #16
ffff800010012944: f85f8e5e ldr x30, [x18, #-8]!
ffff800010012948: d65f03c0 ret
ffff80001001294c: d503201f nop
ffff800010012950 <scs_test4>:
ffff800010012950: a9bf7bfd stp x29, x30, [sp, #-16]!
ffff800010012954: 910003fd mov x29, sp
ffff800010012958: aa0303e3 mov x3, x3
ffff80001001295c: a8c17bfd ldp x29, x30, [sp], #16
ffff800010012960: d65f03c0 ret
> I noticed that __noscs isn't actually applied anywhere in the kernel,
> yet, at least in this series. Were there any places necessary that
> you've found thus far?
At present, I have not found a function that must use the __noscs
attribute in the kernel. I have only used this attribute in test cases.
> Overall, I'm happy with the patch and am ready to ack it, but I would
> like to see a link to to the upstream GCC feature request for SCS (and
> one created if it doesn't exist) cited explicitly in the commit
> message. I think that would be a good demonstration that this can or
> will be upstreamed into the compiler proper for the compiler vendors
> to maintain, rather than the kernel folks. The compiler vendors may
> have further feedback on the approach, such as my question above
> pertaining to inlining.
>
I have submitted a feature request to the gcc community, and waiting
for a follow-up response.
Is it fine to add the following description in [PATCH V5]?
A similar feature request has also been sent to gcc.
link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102768
On Fri, Oct 15, 2021 at 11:29 AM Dan Li <[email protected]> wrote:
>
>
>
> On 10/15/21 2:44 AM, Nick Desaulniers wrote:
> > Overall, I'm happy with the patch and am ready to ack it, but I would
> > like to see a link to to the upstream GCC feature request for SCS (and
> > one created if it doesn't exist) cited explicitly in the commit
> > message. I think that would be a good demonstration that this can or
> > will be upstreamed into the compiler proper for the compiler vendors
> > to maintain, rather than the kernel folks. The compiler vendors may
> > have further feedback on the approach, such as my question above
> > pertaining to inlining.
> >
> I have submitted a feature request to the gcc community, and waiting
> for a follow-up response.
>
> Is it fine to add the following description in [PATCH V5]?
>
> A similar feature request has also been sent to gcc.
> link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102768
Yes, and you can include
Acked-by: Nick Desaulniers <[email protected]>
too.
--
Thanks,
~Nick Desaulniers
On Fri, Oct 15, 2021 at 11:29 AM Dan Li <[email protected]> wrote:
>
>
>
> On 10/15/21 2:44 AM, Nick Desaulniers wrote:
> > On Wed, Oct 13, 2021 at 4:28 PM Dan Li <[email protected]> wrote:
> >> --- 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
> >
> > Cool this is a nice addition, and something I don't think that clang
> > has. For any new feature, having a function attribute to disable it
> > at the function granularity is nice, and plays better with LTO than -f
> > group flags. Though that begs the question: what happens if a __noscs
> > callee is inlined into a non-__noscs caller, or vice versa?
> Thanks Nick,
>
> According to my understanding, all inline optimizations in gcc should
> happen before inserting scs insns (scs and paciasp/autiasp use the
> same insertion point). Therefore, the check for the __noscs attribute
> will also occur after all inlining is completed.
>
> As in the following example:
> - Since __noscs attribute is specified, scs_test1 does not insert scs insns
> - Since normal functions scs_test2/3 uses x30, it needs to insert scs insns
> - Since __noscs attribute is specified, scs_test4 after inlining does not
> need to insert scs insns
>
> __always_inline __noscs void scs_test1(void)
> {
> asm volatile("mov x1, x1\n\t":::"x30");
> }
>
> //scs insns inserted after function inline
> void scs_test2(void)
> {
> scs_test1();
> }
That may be surprising to developers. Perhaps __always_inline on
scs_test1 is distracting this test case, but I suspect it may not make
a difference. This particular issue comes up time and again with
stack protectors; ie. the callee is marked no stack protector, then
gets inlined into a caller and suddenly gets a stack protector.
>
> __always_inline void scs_test3(void)
> {
> asm volatile("mov x3, x3\n\t":::"x30");
> }
>
> //no scs insns inserted
> __noscs void scs_test4(void)
> {
> scs_test3();
> }
>
> ffff800010012900 <scs_test1>:
> ffff800010012900: a9bf7bfd stp x29, x30, [sp, #-16]!
> ffff800010012904: 910003fd mov x29, sp
> ffff800010012908: aa0103e1 mov x1, x1
> ffff80001001290c: a8c17bfd ldp x29, x30, [sp], #16
> ffff800010012910: d65f03c0 ret
>
> ffff800010012914 <scs_test2>:
> ffff800010012914: f800865e str x30, [x18], #8
> ffff800010012918: a9bf7bfd stp x29, x30, [sp, #-16]!
> ffff80001001291c: 910003fd mov x29, sp
> ffff800010012920: aa0103e1 mov x1, x1
> ffff800010012924: a8c17bfd ldp x29, x30, [sp], #16
> ffff800010012928: f85f8e5e ldr x30, [x18, #-8]!
> ffff80001001292c: d65f03c0 ret
>
> ffff800010012930 <scs_test3>:
> ffff800010012930: f800865e str x30, [x18], #8
> ffff800010012934: a9bf7bfd stp x29, x30, [sp, #-16]!
> ffff800010012938: 910003fd mov x29, sp
> ffff80001001293c: aa0303e3 mov x3, x3
> ffff800010012940: a8c17bfd ldp x29, x30, [sp], #16
> ffff800010012944: f85f8e5e ldr x30, [x18, #-8]!
> ffff800010012948: d65f03c0 ret
> ffff80001001294c: d503201f nop
>
> ffff800010012950 <scs_test4>:
> ffff800010012950: a9bf7bfd stp x29, x30, [sp, #-16]!
> ffff800010012954: 910003fd mov x29, sp
> ffff800010012958: aa0303e3 mov x3, x3
> ffff80001001295c: a8c17bfd ldp x29, x30, [sp], #16
> ffff800010012960: d65f03c0 ret
> > I noticed that __noscs isn't actually applied anywhere in the kernel,
> > yet, at least in this series. Were there any places necessary that
> > you've found thus far?
> At present, I have not found a function that must use the __noscs
> attribute in the kernel. I have only used this attribute in test cases.
--
Thanks,
~Nick Desaulniers
On 10/16/21 3:13 AM, Nick Desaulniers wrote:
> On Fri, Oct 15, 2021 at 11:29 AM Dan Li <[email protected]> wrote:
>>
>>
>>
>> On 10/15/21 2:44 AM, Nick Desaulniers wrote:
>>> On Wed, Oct 13, 2021 at 4:28 PM Dan Li <[email protected]> wrote:
>>>> --- 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
>>>
>>> Cool this is a nice addition, and something I don't think that clang
>>> has. For any new feature, having a function attribute to disable it
>>> at the function granularity is nice, and plays better with LTO than -f
>>> group flags. Though that begs the question: what happens if a __noscs
>>> callee is inlined into a non-__noscs caller, or vice versa?
>> Thanks Nick,
>>
>> According to my understanding, all inline optimizations in gcc should
>> happen before inserting scs insns (scs and paciasp/autiasp use the
>> same insertion point). Therefore, the check for the __noscs attribute
>> will also occur after all inlining is completed.
>>
>> As in the following example:
>> - Since __noscs attribute is specified, scs_test1 does not insert scs insns
>> - Since normal functions scs_test2/3 uses x30, it needs to insert scs insns
>> - Since __noscs attribute is specified, scs_test4 after inlining does not
>> need to insert scs insns
>>
>> __always_inline __noscs void scs_test1(void)
>> {
>> asm volatile("mov x1, x1\n\t":::"x30");
>> }
>>
>> //scs insns inserted after function inline
>> void scs_test2(void)
>> {
>> scs_test1();
>> }
>
> That may be surprising to developers. Perhaps __always_inline on
> scs_test1 is distracting this test case, but I suspect it may not make
> a difference. This particular issue comes up time and again with
> stack protectors; ie. the callee is marked no stack protector, then
> gets inlined into a caller and suddenly gets a stack protector.
>
Yes.
I haven’t noticed this issue. I just took a quick look at the stack
canary code, and found that the instructions are generated in the
RTL stage like scs/pac (AST => GIMPLE => RTL => asm output), and
the inlining should have been completed before this.
Generally, instructions that are sensitive to assembly order can
only be inserted during RTL, otherwise their order is difficult to
guarantee. I think this may be the reason why the similar problem
always appears.