| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4898975452179af46f38daa6979b32ba94001419.camel@intel.com>
Date: Tue, 5 Dec 2023 00:10:20 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "dietmar.eggemann@....com" <dietmar.eggemann@....com>,
"broonie@...nel.org" <broonie@...nel.org>,
"Szabolcs.Nagy@....com" <Szabolcs.Nagy@....com>,
"brauner@...nel.org" <brauner@...nel.org>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"debug@...osinc.com" <debug@...osinc.com>,
"mgorman@...e.de" <mgorman@...e.de>,
"vincent.guittot@...aro.org" <vincent.guittot@...aro.org>,
"fweimer@...hat.com" <fweimer@...hat.com>,
"mingo@...hat.com" <mingo@...hat.com>,
"rostedt@...dmis.org" <rostedt@...dmis.org>,
"hjl.tools@...il.com" <hjl.tools@...il.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"vschneid@...hat.com" <vschneid@...hat.com>,
"shuah@...nel.org" <shuah@...nel.org>,
"bristot@...hat.com" <bristot@...hat.com>,
"hpa@...or.com" <hpa@...or.com>,
"peterz@...radead.org" <peterz@...radead.org>,
"bp@...en8.de" <bp@...en8.de>,
"bsegall@...gle.com" <bsegall@...gle.com>,
"x86@...nel.org" <x86@...nel.org>,
"juri.lelli@...hat.com" <juri.lelli@...hat.com>
CC: "keescook@...omium.org" <keescook@...omium.org>,
"jannh@...gle.com" <jannh@...gle.com>,
"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"catalin.marinas@....com" <catalin.marinas@....com>,
"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
"will@...nel.org" <will@...nel.org>
Subject: Re: [PATCH RFT v4 5/5] kselftest/clone3: Test shadow stack support
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]);
Powered by blists - more mailing lists