On Tue, 2023-11-28 at 18:22 +0000, Mark Brown wrote:
+#define ENABLE_SHADOW_STACK +static inline void enable_shadow_stack(void) +{ + int ret = ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK); + if (ret == 0) + shadow_stack_enabled = true; +}
+#endif
+#ifndef ENABLE_SHADOW_STACK +static void enable_shadow_stack(void) +{ +} +#endif
Without this diff, the test crashed for me on a shadow stack system: diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c index dbe52582573c..3236d97ed261 100644 --- a/tools/testing/selftests/clone3/clone3.c +++ b/tools/testing/selftests/clone3/clone3.c @@ -423,7 +423,7 @@ static const struct test tests[] = { })
#define ENABLE_SHADOW_STACK -static inline void enable_shadow_stack(void) +static inline __attribute__((always_inline)) void enable_shadow_stack(void) { int ret = ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK); if (ret == 0)
The fix works by making sure control flow never returns to before the point shadow stack was enabled. Otherwise it will underflow the shadow stack.
But I wonder if the clone3 test should get its shadow stack enabled the conventional elf bit way. So if it's all there (HW, kernel, glibc) then the test will run with shadow stack. Otherwise the test will run without shadow stack.
The other reason is that the shadow stack test in the x86 selftest manual enabling is designed to work without a shadow stack enabled glibc and has to be specially crafted to work around the missing support. I'm not sure the more generic selftests should have to know how to do this. So what about something like this instead:
diff --git a/tools/testing/selftests/clone3/Makefile b/tools/testing/selftests/clone3/Makefile index 84832c369a2e..792bc9685c82 100644 --- a/tools/testing/selftests/clone3/Makefile +++ b/tools/testing/selftests/clone3/Makefile @@ -2,6 +2,13 @@ CFLAGS += -g -std=gnu99 $(KHDR_INCLUDES) LDLIBS += -lcap
+ifeq ($(shell uname -m),x86_64) +CAN_BUILD_WITH_SHSTK := $(shell ../x86/check_cc.sh gcc ../x86/trivial_program.c -mshstk -fcf-protection) +ifeq ($(CAN_BUILD_WITH_SHSTK),1) +CFLAGS += -mshstk -fcf-protection=return +endif +endif + TEST_GEN_PROGS := clone3 clone3_clear_sighand clone3_set_tid \ clone3_cap_checkpoint_restore
diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c index dbe52582573c..eff5e8d5a5a6 100644 --- a/tools/testing/selftests/clone3/clone3.c +++ b/tools/testing/selftests/clone3/clone3.c @@ -23,7 +23,6 @@ #include "clone3_selftests.h"
static bool shadow_stack_enabled; -static bool shadow_stack_supported; static size_t max_supported_args_size;
enum test_mode { @@ -50,36 +49,6 @@ struct test { filter_function filter; };
-#ifndef __NR_map_shadow_stack -#define __NR_map_shadow_stack 453 -#endif - -/* - * We check for shadow stack support by attempting to use - * map_shadow_stack() since features may have been locked by the - * dynamic linker resulting in spurious errors when we attempt to - * enable on startup. We warn if the enable failed. - */ -static void test_shadow_stack_supported(void) -{ - long shadow_stack; - - shadow_stack = syscall(__NR_map_shadow_stack, 0, getpagesize(), 0); - if (shadow_stack == -1) { - ksft_print_msg("map_shadow_stack() not supported\n"); - } else if ((void *)shadow_stack == MAP_FAILED) { - ksft_print_msg("Failed to map shadow stack\n"); - } else { - ksft_print_msg("Shadow stack supportd\n"); - shadow_stack_supported = true; - - if (!shadow_stack_enabled) - ksft_print_msg("Mapped but did not enable shadow stack\n"); - - munmap((void *)shadow_stack, getpagesize()); - } -} - static int call_clone3(uint64_t flags, size_t size, enum test_mode test_mode) { struct __clone_args args = { @@ -220,7 +189,7 @@ static bool no_timenamespace(void)
static bool have_shadow_stack(void) { - if (shadow_stack_supported) { + if (shadow_stack_enabled) { ksft_print_msg("Shadow stack supported\n"); return true; } @@ -230,7 +199,7 @@ static bool have_shadow_stack(void)
static bool no_shadow_stack(void) { - if (!shadow_stack_supported) { + if (!shadow_stack_enabled) { ksft_print_msg("Shadow stack not supported\n"); return true; } @@ -402,38 +371,18 @@ static const struct test tests[] = { };
#ifdef __x86_64__ -#define ARCH_SHSTK_ENABLE 0x5001 +#define ARCH_SHSTK_STATUS 0x5005 #define ARCH_SHSTK_SHSTK (1ULL << 0)
-#define ARCH_PRCTL(arg1, arg2) \ -({ \ - long _ret; \ - register long _num asm("eax") = __NR_arch_prctl; \ - register long _arg1 asm("rdi") = (long)(arg1); \ - register long _arg2 asm("rsi") = (long)(arg2); \ - \ - asm volatile ( \ - "syscall\n" \ - : "=a"(_ret) \ - : "r"(_arg1), "r"(_arg2), \ - "0"(_num) \ - : "rcx", "r11", "memory", "cc" \ - ); \ - _ret; \ -}) - -#define ENABLE_SHADOW_STACK -static inline void enable_shadow_stack(void) +static inline __attribute__((always_inline)) void check_shadow_stack(void) { - int ret = ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK); - if (ret == 0) - shadow_stack_enabled = true; + unsigned long status = 0; + + syscall(SYS_arch_prctl, ARCH_SHSTK_STATUS, &status); + shadow_stack_enabled = status & ARCH_SHSTK_SHSTK; } - -#endif - -#ifndef ENABLE_SHADOW_STACK -static void enable_shadow_stack(void) +#else /* __x86_64__ */ +static void check_shadow_stack(void) { } #endif @@ -443,12 +392,11 @@ int main(int argc, char *argv[]) size_t size; int i;
- enable_shadow_stack(); + check_shadow_stack();
ksft_print_header(); ksft_set_plan(ARRAY_SIZE(tests)); test_clone3_supported(); - test_shadow_stack_supported();
for (i = 0; i < ARRAY_SIZE(tests); i++) test_clone3(&tests[i]);