* [PATCH 01/17] x86/cet: Check user_shstk in /proc/cpuinfo
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
@ 2023-12-06 17:19 ` H.J. Lu
2023-12-06 17:19 ` [PATCH 02/17] x86/cet: Update tst-cet-vfork-1 H.J. Lu
` (15 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:19 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Linux kernel reports CPU shadow stack feature in /proc/cpuinfo as
user_shstk, instead of shstk.
---
sysdeps/x86/tst-cpu-features-cpuinfo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sysdeps/x86/tst-cpu-features-cpuinfo.c b/sysdeps/x86/tst-cpu-features-cpuinfo.c
index 18d64375ca..1d6c647b70 100644
--- a/sysdeps/x86/tst-cpu-features-cpuinfo.c
+++ b/sysdeps/x86/tst-cpu-features-cpuinfo.c
@@ -246,7 +246,7 @@ do_test (int argc, char **argv)
fails += CHECK_PROC (sgx, SGX);
fails += CHECK_PROC (sgx_lc, SGX_LC);
fails += CHECK_PROC (sha_ni, SHA);
- fails += CHECK_PROC (shstk, SHSTK);
+ fails += CHECK_PROC (user_shstk, SHSTK);
fails += CHECK_PROC (smap, SMAP);
fails += CHECK_PROC (smep, SMEP);
fails += CHECK_PROC (smx, SMX);
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 02/17] x86/cet: Update tst-cet-vfork-1
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
2023-12-06 17:19 ` [PATCH 01/17] x86/cet: Check user_shstk in /proc/cpuinfo H.J. Lu
@ 2023-12-06 17:19 ` H.J. Lu
2023-12-06 17:19 ` [PATCH 03/17] x86/cet: Don't assume that SHSTK implies IBT H.J. Lu
` (14 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:19 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Change tst-cet-vfork-1.c to verify that vfork child return triggers
SIGSEGV due to shadow stack mismatch.
---
sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c | 43 ++++++++-----------
1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c b/sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c
index 9c4b6f4d42..c92ed9e737 100644
--- a/sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c
+++ b/sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c
@@ -18,34 +18,26 @@
<https://www.gnu.org/licenses/>. */
#include <stdio.h>
-#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <x86intrin.h>
#include <support/test-driver.h>
-#include <support/xsignal.h>
-#include <support/xunistd.h>
__attribute__ ((noclone, noinline))
static void
do_test_1 (void)
{
pid_t p1;
- int fd[2];
- if (pipe (fd) == -1)
- {
- puts ("pipe failed");
- _exit (EXIT_FAILURE);
- }
+ /* NB: Since child return pops shadow stack which is shared with
+ parent, child must not return after vfork. */
if ((p1 = vfork ()) == 0)
{
- pid_t p = getpid ();
- TEMP_FAILURE_RETRY (write (fd[1], &p, sizeof (p)));
- /* Child return should trigger SIGSEGV. */
+ /* Child return should trigger SIGSEGV due to shadow stack
+ mismatch. */
return;
}
else if (p1 == -1)
@@ -54,22 +46,22 @@ do_test_1 (void)
_exit (EXIT_FAILURE);
}
- pid_t p2 = 0;
- if (TEMP_FAILURE_RETRY (read (fd[0], &p2, sizeof (pid_t)))
- != sizeof (pid_t))
- puts ("pipd read failed");
- else
+ int r;
+ if (TEMP_FAILURE_RETRY (waitpid (p1, &r, 0)) != p1)
{
- int r;
- if (TEMP_FAILURE_RETRY (waitpid (p1, &r, 0)) != p1)
- puts ("waitpid failed");
- else if (r != 0)
- puts ("pip write in child failed");
+ puts ("waitpid failed");
+ _exit (EXIT_FAILURE);
+ }
+
+ if (!WIFSIGNALED (r) || WTERMSIG (r) != SIGSEGV)
+ {
+ puts ("Child not terminated with SIGSEGV");
+ _exit (EXIT_FAILURE);
}
/* Parent exits immediately so that parent returns without triggering
- SIGSEGV when shadow stack isn't in use. */
- _exit (EXIT_FAILURE);
+ SIGSEGV when shadow stack is in use. */
+ _exit (EXIT_SUCCESS);
}
static int
@@ -80,9 +72,8 @@ do_test (void)
return EXIT_UNSUPPORTED;
do_test_1 ();
/* Child exits immediately so that child returns without triggering
- SIGSEGV when shadow stack isn't in use. */
+ SIGSEGV when shadow stack is in use. */
_exit (EXIT_FAILURE);
}
-#define EXPECTED_SIGNAL (_get_ssp () == 0 ? 0 : SIGSEGV)
#include <support/test-driver.c>
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 03/17] x86/cet: Don't assume that SHSTK implies IBT
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
2023-12-06 17:19 ` [PATCH 01/17] x86/cet: Check user_shstk in /proc/cpuinfo H.J. Lu
2023-12-06 17:19 ` [PATCH 02/17] x86/cet: Update tst-cet-vfork-1 H.J. Lu
@ 2023-12-06 17:19 ` H.J. Lu
2023-12-06 17:19 ` [PATCH 04/17] x86/cet: Check legacy shadow stack applications H.J. Lu
` (13 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:19 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Since shadow stack (SHSTK) is enabled in the Linux kernel without
enabling indirect branch tracking (IBT), don't assume that SHSTK
implies IBT. Use "CPU_FEATURE_ACTIVE (IBT)" to check if IBT is active
and "CPU_FEATURE_ACTIVE (SHSTK)" to check if SHSTK is active.
---
sysdeps/x86/Makefile | 1 -
sysdeps/x86/tst-cet-legacy-10.c | 6 +++---
sysdeps/x86/tst-cet-legacy-8.c | 15 ++++++++-------
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 917c26f116..ea45aad34c 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -165,7 +165,6 @@ CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=branch
CFLAGS-tst-cet-legacy-mod-6b.c += -fcf-protection
CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection
CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none
-CFLAGS-tst-cet-legacy-8.c += -mshstk
CFLAGS-tst-cet-legacy-10.c += -mshstk
CFLAGS-tst-cet-legacy-10-static.c += -mshstk
diff --git a/sysdeps/x86/tst-cet-legacy-10.c b/sysdeps/x86/tst-cet-legacy-10.c
index a85cdc3171..ae2c34de3e 100644
--- a/sysdeps/x86/tst-cet-legacy-10.c
+++ b/sysdeps/x86/tst-cet-legacy-10.c
@@ -21,19 +21,19 @@
#include <support/test-driver.h>
#include <support/xunistd.h>
-/* Check that CPU_FEATURE_ACTIVE on IBT and SHSTK matches _get_ssp. */
+/* Check that CPU_FEATURE_ACTIVE on SHSTK matches _get_ssp. */
static int
do_test (void)
{
if (_get_ssp () != 0)
{
- if (CPU_FEATURE_ACTIVE (IBT) && CPU_FEATURE_ACTIVE (SHSTK))
+ if (CPU_FEATURE_ACTIVE (SHSTK))
return EXIT_SUCCESS;
}
else
{
- if (!CPU_FEATURE_ACTIVE (IBT) && !CPU_FEATURE_ACTIVE (SHSTK))
+ if (!CPU_FEATURE_ACTIVE (SHSTK))
return EXIT_SUCCESS;
}
diff --git a/sysdeps/x86/tst-cet-legacy-8.c b/sysdeps/x86/tst-cet-legacy-8.c
index 5d8d9ba7dc..77d77a5408 100644
--- a/sysdeps/x86/tst-cet-legacy-8.c
+++ b/sysdeps/x86/tst-cet-legacy-8.c
@@ -18,7 +18,7 @@
#include <stdio.h>
#include <stdlib.h>
-#include <x86intrin.h>
+#include <sys/platform/x86.h>
#include <sys/mman.h>
#include <support/test-driver.h>
#include <support/xsignal.h>
@@ -29,11 +29,6 @@
static int
do_test (void)
{
- /* NB: This test should trigger SIGSEGV on CET platforms. If SHSTK
- is disabled, assuming IBT is also disabled. */
- if (_get_ssp () == 0)
- return EXIT_UNSUPPORTED;
-
void (*funcp) (void);
funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE, -1);
@@ -41,8 +36,14 @@ do_test (void)
/* Write RET instruction. */
*(char *) funcp = 0xc3;
funcp ();
+
+ /* NB: This test should trigger SIGSEGV when IBT is active. We should
+ reach here if IBT isn't active. */
+ if (!CPU_FEATURE_ACTIVE (IBT))
+ return EXIT_UNSUPPORTED;
+
return EXIT_FAILURE;
}
-#define EXPECTED_SIGNAL (_get_ssp () == 0 ? 0 : SIGSEGV)
+#define EXPECTED_SIGNAL (CPU_FEATURE_ACTIVE (IBT) ? SIGSEGV : 0)
#include <support/test-driver.c>
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 04/17] x86/cet: Check legacy shadow stack applications
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (2 preceding siblings ...)
2023-12-06 17:19 ` [PATCH 03/17] x86/cet: Don't assume that SHSTK implies IBT H.J. Lu
@ 2023-12-06 17:19 ` H.J. Lu
2023-12-06 17:19 ` [PATCH 05/17] x86/cet: Check CPU_FEATURE_ACTIVE when CET is disabled H.J. Lu
` (12 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:19 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Add tests to verify that legacy shadow stack applications run properly
when shadow stack is enabled in Linux kernel.
---
sysdeps/x86/Makefile | 23 ++++++++++++++
| 35 ++++++++++++++++++++++
sysdeps/x86/tst-shstk-legacy-1a-static.c | 1 +
sysdeps/x86/tst-shstk-legacy-1a.c | 32 ++++++++++++++++++++
sysdeps/x86/tst-shstk-legacy-1b-static.c | 1 +
sysdeps/x86/tst-shstk-legacy-1b.c | 38 ++++++++++++++++++++++++
6 files changed, 130 insertions(+)
create mode 100644 sysdeps/x86/tst-shstk-legacy-1-extra.S
create mode 100644 sysdeps/x86/tst-shstk-legacy-1a-static.c
create mode 100644 sysdeps/x86/tst-shstk-legacy-1a.c
create mode 100644 sysdeps/x86/tst-shstk-legacy-1b-static.c
create mode 100644 sysdeps/x86/tst-shstk-legacy-1b.c
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index ea45aad34c..dea14e343c 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -129,6 +129,21 @@ tests += tst-cet-legacy-1 tst-cet-legacy-1a tst-cet-legacy-2 \
tst-cet-legacy-10 tst-cet-legacy-10-static
tests-static += tst-cet-legacy-9-static tst-cet-legacy-10-static
tst-cet-legacy-1a-ARGS = -- $(host-test-program-cmd)
+
+tests += \
+ tst-shstk-legacy-1a \
+ tst-shstk-legacy-1a-static \
+ tst-shstk-legacy-1b \
+ tst-shstk-legacy-1b-static \
+# tests
+tests-static += \
+ tst-shstk-legacy-1a-static \
+ tst-shstk-legacy-1b-static \
+# tests-static
+extra-objs += \
+ tst-shstk-legacy-1-extra.o \
+# extra-objs
+
tests += tst-cet-legacy-4a tst-cet-legacy-4b tst-cet-legacy-4c \
tst-cet-legacy-5b tst-cet-legacy-6b
modules-names += tst-cet-legacy-mod-1 tst-cet-legacy-mod-2 \
@@ -168,6 +183,9 @@ CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none
CFLAGS-tst-cet-legacy-10.c += -mshstk
CFLAGS-tst-cet-legacy-10-static.c += -mshstk
+CFLAGS-tst-shstk-legacy-1a.c += -fcf-protection=none
+CFLAGS-tst-shstk-legacy-1a-static.c += -fcf-protection=none
+
$(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \
$(objpfx)tst-cet-legacy-mod-2.so
$(objpfx)tst-cet-legacy-1a: $(objpfx)tst-cet-legacy-mod-1.so \
@@ -200,6 +218,11 @@ $(objpfx)tst-cet-legacy-6b.out: $(objpfx)tst-cet-legacy-mod-6a.so \
tst-cet-legacy-6b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
tst-cet-legacy-9-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
tst-cet-legacy-9-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
+
+$(objpfx)tst-shstk-legacy-1a: $(objpfx)tst-shstk-legacy-1-extra.o
+$(objpfx)tst-shstk-legacy-1a-static: $(objpfx)tst-shstk-legacy-1-extra.o
+$(objpfx)tst-shstk-legacy-1b: $(objpfx)tst-shstk-legacy-1-extra.o
+$(objpfx)tst-shstk-legacy-1b-static: $(objpfx)tst-shstk-legacy-1-extra.o
endif
# Add -fcf-protection to CFLAGS when CET is enabled.
--git a/sysdeps/x86/tst-shstk-legacy-1-extra.S b/sysdeps/x86/tst-shstk-legacy-1-extra.S
new file mode 100644
index 0000000000..f3adb9f639
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1-extra.S
@@ -0,0 +1,35 @@
+/* Legacy shadow stack code.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+ .text
+ .globl legacy
+ .type legacy, @function
+legacy:
+ .cfi_startproc
+#ifdef __x86_64__
+ movq (%rsp), %rax
+ addq $8, %rsp
+ jmp *%rax
+#else
+ movl (%esp), %eax
+ addl $4, %esp
+ jmp *%eax
+#endif
+ .cfi_endproc
+ .size legacy, .-legacy
+ .section .note.GNU-stack,"",@progbits
diff --git a/sysdeps/x86/tst-shstk-legacy-1a-static.c b/sysdeps/x86/tst-shstk-legacy-1a-static.c
new file mode 100644
index 0000000000..dd549890a0
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1a-static.c
@@ -0,0 +1 @@
+#include "tst-shstk-legacy-1a.c"
diff --git a/sysdeps/x86/tst-shstk-legacy-1a.c b/sysdeps/x86/tst-shstk-legacy-1a.c
new file mode 100644
index 0000000000..c6f5810838
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1a.c
@@ -0,0 +1,32 @@
+/* Check that legacy shadow stack code won't trigger segfault.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stdlib.h>
+#include <support/test-driver.h>
+
+/* Check that legacy shadow stack code won't trigger segfault. */
+extern void legacy (void);
+
+static int
+do_test (void)
+{
+ legacy ();
+ return EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-shstk-legacy-1b-static.c b/sysdeps/x86/tst-shstk-legacy-1b-static.c
new file mode 100644
index 0000000000..4945344675
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1b-static.c
@@ -0,0 +1 @@
+#include "tst-shstk-legacy-1b.c"
diff --git a/sysdeps/x86/tst-shstk-legacy-1b.c b/sysdeps/x86/tst-shstk-legacy-1b.c
new file mode 100644
index 0000000000..05231e60ae
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1b.c
@@ -0,0 +1,38 @@
+/* Check that legacy shadow stack code will trigger segfault.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stdlib.h>
+#include <sys/platform/x86.h>
+#include <support/test-driver.h>
+#include <support/xsignal.h>
+
+/* Check that legacy shadow stack code will trigger segfault. */
+extern void legacy (void);
+
+static int
+do_test (void)
+{
+ if (!CPU_FEATURE_ACTIVE (SHSTK))
+ return EXIT_UNSUPPORTED;
+
+ legacy ();
+ return EXIT_FAILURE;
+}
+
+#define EXPECTED_SIGNAL (CPU_FEATURE_ACTIVE (SHSTK) ? SIGSEGV : 0)
+#include <support/test-driver.c>
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 05/17] x86/cet: Check CPU_FEATURE_ACTIVE when CET is disabled
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (3 preceding siblings ...)
2023-12-06 17:19 ` [PATCH 04/17] x86/cet: Check legacy shadow stack applications H.J. Lu
@ 2023-12-06 17:19 ` H.J. Lu
2023-12-06 23:53 ` Noah Goldstein
2023-12-06 17:19 ` [PATCH 06/17] x86/cet: Add tests for GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK H.J. Lu
` (11 subsequent siblings)
16 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:19 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Verify that CPU_FEATURE_ACTIVE (SHSTK) works properly when CET is
disabled.
---
sysdeps/x86/Makefile | 8 ++++++--
sysdeps/x86/tst-cet-legacy-10a-static.c | 2 ++
sysdeps/x86/tst-cet-legacy-10a.c | 2 ++
3 files changed, 10 insertions(+), 2 deletions(-)
create mode 100644 sysdeps/x86/tst-cet-legacy-10a-static.c
create mode 100644 sysdeps/x86/tst-cet-legacy-10a.c
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index dea14e343c..580c3ecdc5 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -126,8 +126,10 @@ tests += tst-cet-legacy-1 tst-cet-legacy-1a tst-cet-legacy-2 \
tst-cet-legacy-2a tst-cet-legacy-3 tst-cet-legacy-4 \
tst-cet-legacy-5a tst-cet-legacy-6a tst-cet-legacy-7 \
tst-cet-legacy-8 tst-cet-legacy-9 tst-cet-legacy-9-static \
- tst-cet-legacy-10 tst-cet-legacy-10-static
-tests-static += tst-cet-legacy-9-static tst-cet-legacy-10-static
+ tst-cet-legacy-10 tst-cet-legacy-10-static \
+ tst-cet-legacy-10a tst-cet-legacy-10a-static
+tests-static += tst-cet-legacy-9-static tst-cet-legacy-10-static \
+ tst-cet-legacy-10a-static
tst-cet-legacy-1a-ARGS = -- $(host-test-program-cmd)
tests += \
@@ -182,6 +184,8 @@ CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection
CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none
CFLAGS-tst-cet-legacy-10.c += -mshstk
CFLAGS-tst-cet-legacy-10-static.c += -mshstk
+CFLAGS-tst-cet-legacy-10a.c += -fcf-protection=none
+CFLAGS-tst-cet-legacy-10a-static.c += -fcf-protection=none
CFLAGS-tst-shstk-legacy-1a.c += -fcf-protection=none
CFLAGS-tst-shstk-legacy-1a-static.c += -fcf-protection=none
diff --git a/sysdeps/x86/tst-cet-legacy-10a-static.c b/sysdeps/x86/tst-cet-legacy-10a-static.c
new file mode 100644
index 0000000000..05073a5d1e
--- /dev/null
+++ b/sysdeps/x86/tst-cet-legacy-10a-static.c
@@ -0,0 +1,2 @@
+#pragma GCC target ("shstk")
+#include "tst-cet-legacy-10.c"
diff --git a/sysdeps/x86/tst-cet-legacy-10a.c b/sysdeps/x86/tst-cet-legacy-10a.c
new file mode 100644
index 0000000000..05073a5d1e
--- /dev/null
+++ b/sysdeps/x86/tst-cet-legacy-10a.c
@@ -0,0 +1,2 @@
+#pragma GCC target ("shstk")
+#include "tst-cet-legacy-10.c"
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 05/17] x86/cet: Check CPU_FEATURE_ACTIVE when CET is disabled
2023-12-06 17:19 ` [PATCH 05/17] x86/cet: Check CPU_FEATURE_ACTIVE when CET is disabled H.J. Lu
@ 2023-12-06 23:53 ` Noah Goldstein
2023-12-07 21:07 ` H.J. Lu
0 siblings, 1 reply; 42+ messages in thread
From: Noah Goldstein @ 2023-12-06 23:53 UTC (permalink / raw)
To: H.J. Lu; +Cc: libc-alpha, rick.p.edgecombe
On Wed, Dec 6, 2023 at 11:20 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> Verify that CPU_FEATURE_ACTIVE (SHSTK) works properly when CET is
> disabled.
> ---
> sysdeps/x86/Makefile | 8 ++++++--
> sysdeps/x86/tst-cet-legacy-10a-static.c | 2 ++
> sysdeps/x86/tst-cet-legacy-10a.c | 2 ++
> 3 files changed, 10 insertions(+), 2 deletions(-)
> create mode 100644 sysdeps/x86/tst-cet-legacy-10a-static.c
> create mode 100644 sysdeps/x86/tst-cet-legacy-10a.c
>
> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> index dea14e343c..580c3ecdc5 100644
> --- a/sysdeps/x86/Makefile
> +++ b/sysdeps/x86/Makefile
> @@ -126,8 +126,10 @@ tests += tst-cet-legacy-1 tst-cet-legacy-1a tst-cet-legacy-2 \
> tst-cet-legacy-2a tst-cet-legacy-3 tst-cet-legacy-4 \
> tst-cet-legacy-5a tst-cet-legacy-6a tst-cet-legacy-7 \
> tst-cet-legacy-8 tst-cet-legacy-9 tst-cet-legacy-9-static \
> - tst-cet-legacy-10 tst-cet-legacy-10-static
> -tests-static += tst-cet-legacy-9-static tst-cet-legacy-10-static
> + tst-cet-legacy-10 tst-cet-legacy-10-static \
> + tst-cet-legacy-10a tst-cet-legacy-10a-static
> +tests-static += tst-cet-legacy-9-static tst-cet-legacy-10-static \
> + tst-cet-legacy-10a-static
> tst-cet-legacy-1a-ARGS = -- $(host-test-program-cmd)
>
maybe pre commit splitting to target per line.
> tests += \
> @@ -182,6 +184,8 @@ CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection
> CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none
> CFLAGS-tst-cet-legacy-10.c += -mshstk
> CFLAGS-tst-cet-legacy-10-static.c += -mshstk
> +CFLAGS-tst-cet-legacy-10a.c += -fcf-protection=none
> +CFLAGS-tst-cet-legacy-10a-static.c += -fcf-protection=none
>
> CFLAGS-tst-shstk-legacy-1a.c += -fcf-protection=none
> CFLAGS-tst-shstk-legacy-1a-static.c += -fcf-protection=none
> diff --git a/sysdeps/x86/tst-cet-legacy-10a-static.c b/sysdeps/x86/tst-cet-legacy-10a-static.c
> new file mode 100644
> index 0000000000..05073a5d1e
> --- /dev/null
> +++ b/sysdeps/x86/tst-cet-legacy-10a-static.c
> @@ -0,0 +1,2 @@
> +#pragma GCC target ("shstk")
> +#include "tst-cet-legacy-10.c"
> diff --git a/sysdeps/x86/tst-cet-legacy-10a.c b/sysdeps/x86/tst-cet-legacy-10a.c
> new file mode 100644
> index 0000000000..05073a5d1e
> --- /dev/null
> +++ b/sysdeps/x86/tst-cet-legacy-10a.c
> @@ -0,0 +1,2 @@
> +#pragma GCC target ("shstk")
> +#include "tst-cet-legacy-10.c"
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 05/17] x86/cet: Check CPU_FEATURE_ACTIVE when CET is disabled
2023-12-06 23:53 ` Noah Goldstein
@ 2023-12-07 21:07 ` H.J. Lu
0 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-07 21:07 UTC (permalink / raw)
To: Noah Goldstein; +Cc: libc-alpha, rick.p.edgecombe
On Wed, Dec 6, 2023 at 3:53 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Wed, Dec 6, 2023 at 11:20 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > Verify that CPU_FEATURE_ACTIVE (SHSTK) works properly when CET is
> > disabled.
> > ---
> > sysdeps/x86/Makefile | 8 ++++++--
> > sysdeps/x86/tst-cet-legacy-10a-static.c | 2 ++
> > sysdeps/x86/tst-cet-legacy-10a.c | 2 ++
> > 3 files changed, 10 insertions(+), 2 deletions(-)
> > create mode 100644 sysdeps/x86/tst-cet-legacy-10a-static.c
> > create mode 100644 sysdeps/x86/tst-cet-legacy-10a.c
> >
> > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> > index dea14e343c..580c3ecdc5 100644
> > --- a/sysdeps/x86/Makefile
> > +++ b/sysdeps/x86/Makefile
> > @@ -126,8 +126,10 @@ tests += tst-cet-legacy-1 tst-cet-legacy-1a tst-cet-legacy-2 \
> > tst-cet-legacy-2a tst-cet-legacy-3 tst-cet-legacy-4 \
> > tst-cet-legacy-5a tst-cet-legacy-6a tst-cet-legacy-7 \
> > tst-cet-legacy-8 tst-cet-legacy-9 tst-cet-legacy-9-static \
> > - tst-cet-legacy-10 tst-cet-legacy-10-static
> > -tests-static += tst-cet-legacy-9-static tst-cet-legacy-10-static
> > + tst-cet-legacy-10 tst-cet-legacy-10-static \
> > + tst-cet-legacy-10a tst-cet-legacy-10a-static
> > +tests-static += tst-cet-legacy-9-static tst-cet-legacy-10-static \
> > + tst-cet-legacy-10a-static
> > tst-cet-legacy-1a-ARGS = -- $(host-test-program-cmd)
> >
> maybe pre commit splitting to target per line.
I sent out a separate patch against the master branch to put each test
on a separate line.
Thanks.
> > tests += \
> > @@ -182,6 +184,8 @@ CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection
> > CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none
> > CFLAGS-tst-cet-legacy-10.c += -mshstk
> > CFLAGS-tst-cet-legacy-10-static.c += -mshstk
> > +CFLAGS-tst-cet-legacy-10a.c += -fcf-protection=none
> > +CFLAGS-tst-cet-legacy-10a-static.c += -fcf-protection=none
> >
> > CFLAGS-tst-shstk-legacy-1a.c += -fcf-protection=none
> > CFLAGS-tst-shstk-legacy-1a-static.c += -fcf-protection=none
> > diff --git a/sysdeps/x86/tst-cet-legacy-10a-static.c b/sysdeps/x86/tst-cet-legacy-10a-static.c
> > new file mode 100644
> > index 0000000000..05073a5d1e
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-cet-legacy-10a-static.c
> > @@ -0,0 +1,2 @@
> > +#pragma GCC target ("shstk")
> > +#include "tst-cet-legacy-10.c"
> > diff --git a/sysdeps/x86/tst-cet-legacy-10a.c b/sysdeps/x86/tst-cet-legacy-10a.c
> > new file mode 100644
> > index 0000000000..05073a5d1e
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-cet-legacy-10a.c
> > @@ -0,0 +1,2 @@
> > +#pragma GCC target ("shstk")
> > +#include "tst-cet-legacy-10.c"
> > --
> > 2.43.0
> >
--
H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 06/17] x86/cet: Add tests for GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (4 preceding siblings ...)
2023-12-06 17:19 ` [PATCH 05/17] x86/cet: Check CPU_FEATURE_ACTIVE when CET is disabled H.J. Lu
@ 2023-12-06 17:19 ` H.J. Lu
2023-12-06 17:20 ` [PATCH 07/17] x86/cet: Check legacy shadow stack code in .init_array section H.J. Lu
` (10 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:19 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Verify that GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK turns off shadow
stack properly.
---
sysdeps/x86/Makefile | 7 +++++++
sysdeps/x86/tst-shstk-legacy-1c-static.c | 1 +
sysdeps/x86/tst-shstk-legacy-1c.c | 20 ++++++++++++++++++++
3 files changed, 28 insertions(+)
create mode 100644 sysdeps/x86/tst-shstk-legacy-1c-static.c
create mode 100644 sysdeps/x86/tst-shstk-legacy-1c.c
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 580c3ecdc5..5f1414fba3 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -137,10 +137,13 @@ tests += \
tst-shstk-legacy-1a-static \
tst-shstk-legacy-1b \
tst-shstk-legacy-1b-static \
+ tst-shstk-legacy-1c \
+ tst-shstk-legacy-1c-static \
# tests
tests-static += \
tst-shstk-legacy-1a-static \
tst-shstk-legacy-1b-static \
+ tst-shstk-legacy-1c-static \
# tests-static
extra-objs += \
tst-shstk-legacy-1-extra.o \
@@ -227,6 +230,10 @@ $(objpfx)tst-shstk-legacy-1a: $(objpfx)tst-shstk-legacy-1-extra.o
$(objpfx)tst-shstk-legacy-1a-static: $(objpfx)tst-shstk-legacy-1-extra.o
$(objpfx)tst-shstk-legacy-1b: $(objpfx)tst-shstk-legacy-1-extra.o
$(objpfx)tst-shstk-legacy-1b-static: $(objpfx)tst-shstk-legacy-1-extra.o
+tst-shstk-legacy-1c-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK
+tst-shstk-legacy-1c-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK
+$(objpfx)tst-shstk-legacy-1c: $(objpfx)tst-shstk-legacy-1-extra.o
+$(objpfx)tst-shstk-legacy-1c-static: $(objpfx)tst-shstk-legacy-1-extra.o
endif
# Add -fcf-protection to CFLAGS when CET is enabled.
diff --git a/sysdeps/x86/tst-shstk-legacy-1c-static.c b/sysdeps/x86/tst-shstk-legacy-1c-static.c
new file mode 100644
index 0000000000..91ea346aaf
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1c-static.c
@@ -0,0 +1 @@
+#include "tst-shstk-legacy-1c.c"
diff --git a/sysdeps/x86/tst-shstk-legacy-1c.c b/sysdeps/x86/tst-shstk-legacy-1c.c
new file mode 100644
index 0000000000..eb218c6c70
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1c.c
@@ -0,0 +1,20 @@
+/* Check that legacy shadow stack code won't trigger segfault with
+ GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include "tst-shstk-legacy-1a.c"
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 07/17] x86/cet: Check legacy shadow stack code in .init_array section
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (5 preceding siblings ...)
2023-12-06 17:19 ` [PATCH 06/17] x86/cet: Add tests for GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
2023-12-06 17:20 ` [PATCH 08/17] x86/cet: Check CPU_FEATURE_ACTIVE in permissive mode H.J. Lu
` (9 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Verify that legacy shadow stack code in .init_array section in application
and shared library, which are marked as shadow stack enabled, will trigger
segfault.
---
sysdeps/x86/Makefile | 36 +++++++++++++++
sysdeps/x86/tst-shstk-legacy-1d-static.c | 1 +
sysdeps/x86/tst-shstk-legacy-1d.c | 47 ++++++++++++++++++++
sysdeps/x86/tst-shstk-legacy-1e-static.c | 1 +
sysdeps/x86/tst-shstk-legacy-1e-static.sh | 32 ++++++++++++++
sysdeps/x86/tst-shstk-legacy-1e.c | 53 +++++++++++++++++++++++
sysdeps/x86/tst-shstk-legacy-1e.sh | 34 +++++++++++++++
sysdeps/x86/tst-shstk-legacy-1f.c | 29 +++++++++++++
sysdeps/x86/tst-shstk-legacy-1g.c | 35 +++++++++++++++
sysdeps/x86/tst-shstk-legacy-1g.sh | 34 +++++++++++++++
sysdeps/x86/tst-shstk-legacy-mod-1.c | 28 ++++++++++++
11 files changed, 330 insertions(+)
create mode 100644 sysdeps/x86/tst-shstk-legacy-1d-static.c
create mode 100644 sysdeps/x86/tst-shstk-legacy-1d.c
create mode 100644 sysdeps/x86/tst-shstk-legacy-1e-static.c
create mode 100755 sysdeps/x86/tst-shstk-legacy-1e-static.sh
create mode 100644 sysdeps/x86/tst-shstk-legacy-1e.c
create mode 100755 sysdeps/x86/tst-shstk-legacy-1e.sh
create mode 100644 sysdeps/x86/tst-shstk-legacy-1f.c
create mode 100644 sysdeps/x86/tst-shstk-legacy-1g.c
create mode 100755 sysdeps/x86/tst-shstk-legacy-1g.sh
create mode 100644 sysdeps/x86/tst-shstk-legacy-mod-1.c
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 5f1414fba3..b8b98177e0 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -139,11 +139,22 @@ tests += \
tst-shstk-legacy-1b-static \
tst-shstk-legacy-1c \
tst-shstk-legacy-1c-static \
+ tst-shstk-legacy-1d \
+ tst-shstk-legacy-1d-static \
+ tst-shstk-legacy-1e \
+ tst-shstk-legacy-1e-static \
+ tst-shstk-legacy-1f \
+ tst-shstk-legacy-1g \
# tests
+modules-names += \
+ tst-shstk-legacy-mod-1 \
+# modules-names
tests-static += \
tst-shstk-legacy-1a-static \
tst-shstk-legacy-1b-static \
tst-shstk-legacy-1c-static \
+ tst-shstk-legacy-1d-static \
+ tst-shstk-legacy-1e-static \
# tests-static
extra-objs += \
tst-shstk-legacy-1-extra.o \
@@ -192,6 +203,9 @@ CFLAGS-tst-cet-legacy-10a-static.c += -fcf-protection=none
CFLAGS-tst-shstk-legacy-1a.c += -fcf-protection=none
CFLAGS-tst-shstk-legacy-1a-static.c += -fcf-protection=none
+CFLAGS-tst-shstk-legacy-1d.c += -fcf-protection=none
+CFLAGS-tst-shstk-legacy-1d-static.c += -fcf-protection=none
+CFLAGS-tst-shstk-legacy-1f.c += -fcf-protection=none
$(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \
$(objpfx)tst-cet-legacy-mod-2.so
@@ -234,6 +248,28 @@ tst-shstk-legacy-1c-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK
tst-shstk-legacy-1c-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK
$(objpfx)tst-shstk-legacy-1c: $(objpfx)tst-shstk-legacy-1-extra.o
$(objpfx)tst-shstk-legacy-1c-static: $(objpfx)tst-shstk-legacy-1-extra.o
+$(objpfx)tst-shstk-legacy-1d: $(objpfx)tst-shstk-legacy-1-extra.o
+$(objpfx)tst-shstk-legacy-1d-static: $(objpfx)tst-shstk-legacy-1-extra.o
+$(objpfx)tst-shstk-legacy-1e: $(objpfx)tst-shstk-legacy-1-extra.o
+$(objpfx)tst-shstk-legacy-1e-static: $(objpfx)tst-shstk-legacy-1-extra.o
+$(objpfx)tst-shstk-legacy-1e.out: \
+ $(..)/sysdeps/x86/tst-shstk-legacy-1e.sh $(objpfx)tst-shstk-legacy-1e
+ $(SHELL) $< $(common-objpfx) '$(test-program-prefix)' 2> $@; \
+ $(evaluate-test)
+$(objpfx)tst-shstk-legacy-1e-static.out: \
+ $(..)/sysdeps/x86/tst-shstk-legacy-1e-static.sh \
+ $(objpfx)tst-shstk-legacy-1e-static
+ $(SHELL) $< $(common-objpfx) 2> $@; \
+ $(evaluate-test)
+$(objpfx)tst-shstk-legacy-1f: $(objpfx)tst-shstk-legacy-mod-1.so
+$(objpfx)tst-shstk-legacy-mod-1.so: \
+ $(objpfx)tst-shstk-legacy-mod-1.os \
+ $(objpfx)tst-shstk-legacy-1-extra.os
+$(objpfx)tst-shstk-legacy-1g: $(objpfx)tst-shstk-legacy-mod-1.so
+$(objpfx)tst-shstk-legacy-1g.out: \
+ $(..)/sysdeps/x86/tst-shstk-legacy-1g.sh $(objpfx)tst-shstk-legacy-1g
+ $(SHELL) $< $(common-objpfx) '$(test-program-prefix)' 2> $@; \
+ $(evaluate-test)
endif
# Add -fcf-protection to CFLAGS when CET is enabled.
diff --git a/sysdeps/x86/tst-shstk-legacy-1d-static.c b/sysdeps/x86/tst-shstk-legacy-1d-static.c
new file mode 100644
index 0000000000..dca27a5482
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1d-static.c
@@ -0,0 +1 @@
+#include "tst-shstk-legacy-1d.c"
diff --git a/sysdeps/x86/tst-shstk-legacy-1d.c b/sysdeps/x86/tst-shstk-legacy-1d.c
new file mode 100644
index 0000000000..465cfab1db
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1d.c
@@ -0,0 +1,47 @@
+/* Check that legacy shadow stack code in init_array won't trigger
+ segfault.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stdlib.h>
+#include <support/test-driver.h>
+
+/* Check that legacy shadow stack code in init_array won't trigger
+ segfault. */
+extern void legacy (void);
+int done;
+
+void
+legacy_1 (void)
+{
+ legacy ();
+ done = 1;
+}
+
+void (*init_array []) (void)
+ __attribute__ ((section (".init_array"), aligned (sizeof (void *)))) =
+{
+ &legacy_1
+};
+
+static int
+do_test (void)
+{
+ return EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-shstk-legacy-1e-static.c b/sysdeps/x86/tst-shstk-legacy-1e-static.c
new file mode 100644
index 0000000000..cb6ce0de00
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1e-static.c
@@ -0,0 +1 @@
+#include "tst-shstk-legacy-1e.c"
diff --git a/sysdeps/x86/tst-shstk-legacy-1e-static.sh b/sysdeps/x86/tst-shstk-legacy-1e-static.sh
new file mode 100755
index 0000000000..e943aec70e
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1e-static.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+# Check that legacy shadow stack code in init_array will trigger
+# segfault.
+# Copyright (C) 2023 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+common_objpfx=$1; shift
+
+${common_objpfx}elf/tst-shstk-legacy-1e-static
+# The exit status should only be unsupported (77) or segfault (139).
+status=$?
+if test $status -eq 77; then
+ exit 77
+elif test $status == 139; then
+ exit 0
+else
+ exit 1
+fi
diff --git a/sysdeps/x86/tst-shstk-legacy-1e.c b/sysdeps/x86/tst-shstk-legacy-1e.c
new file mode 100644
index 0000000000..e78a4b776e
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1e.c
@@ -0,0 +1,53 @@
+/* Check that legacy shadow stack code in init_array will trigger
+ segfault.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stdlib.h>
+#include <sys/platform/x86.h>
+#include <support/test-driver.h>
+#include <support/xsignal.h>
+
+/* Check that legacy shadow stack code in init_array will trigger
+ segfault. */
+extern void legacy (void);
+int done;
+
+void
+legacy_1 (void)
+{
+ legacy ();
+ done = 1;
+}
+
+void (*init_array []) (void)
+ __attribute__ ((section (".init_array"), aligned (sizeof (void *)))) =
+{
+ &legacy_1
+};
+
+static int
+do_test (void)
+{
+ if (!CPU_FEATURE_ACTIVE (SHSTK))
+ return EXIT_UNSUPPORTED;
+
+ return EXIT_FAILURE;
+}
+
+#define EXPECTED_SIGNAL (CPU_FEATURE_ACTIVE (SHSTK) ? SIGSEGV : 0)
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-shstk-legacy-1e.sh b/sysdeps/x86/tst-shstk-legacy-1e.sh
new file mode 100755
index 0000000000..b0467aa899
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1e.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+# Check that legacy shadow stack code in init_array will trigger
+# segfault.
+# Copyright (C) 2023 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+common_objpfx=$1; shift
+test_program_prefix=$1; shift
+
+${test_program_prefix} \
+ ${common_objpfx}elf/tst-shstk-legacy-1e
+# The exit status should only be unsupported (77) or segfault (139).
+status=$?
+if test $status -eq 77; then
+ exit 77
+elif test $status == 139; then
+ exit 0
+else
+ exit 1
+fi
diff --git a/sysdeps/x86/tst-shstk-legacy-1f.c b/sysdeps/x86/tst-shstk-legacy-1f.c
new file mode 100644
index 0000000000..27e01a229e
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1f.c
@@ -0,0 +1,29 @@
+/* Check that legacy shadow stack code in init_array won't trigger
+ segfault.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stdlib.h>
+#include <support/test-driver.h>
+
+static int
+do_test (void)
+{
+ return EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-shstk-legacy-1g.c b/sysdeps/x86/tst-shstk-legacy-1g.c
new file mode 100644
index 0000000000..a1f3d242e9
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1g.c
@@ -0,0 +1,35 @@
+/* Check that legacy shadow stack code in init_array will trigger
+ segfault.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stdlib.h>
+#include <sys/platform/x86.h>
+#include <support/test-driver.h>
+#include <support/xsignal.h>
+
+static int
+do_test (void)
+{
+ if (!CPU_FEATURE_ACTIVE (SHSTK))
+ return EXIT_UNSUPPORTED;
+
+ return EXIT_FAILURE;
+}
+
+#define EXPECTED_SIGNAL (CPU_FEATURE_ACTIVE (SHSTK) ? SIGSEGV : 0)
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-shstk-legacy-1g.sh b/sysdeps/x86/tst-shstk-legacy-1g.sh
new file mode 100755
index 0000000000..c112bf6d8d
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-1g.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+# Check that legacy shadow stack code in init_array will trigger
+# segfault.
+# Copyright (C) 2023 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+common_objpfx=$1; shift
+test_program_prefix=$1; shift
+
+${test_program_prefix} \
+ ${common_objpfx}elf/tst-shstk-legacy-1g
+# The exit status should only be unsupported (77) or segfault (139).
+status=$?
+if test $status -eq 77; then
+ exit 77
+elif test $status == 139; then
+ exit 0
+else
+ exit 1
+fi
diff --git a/sysdeps/x86/tst-shstk-legacy-mod-1.c b/sysdeps/x86/tst-shstk-legacy-mod-1.c
new file mode 100644
index 0000000000..b75b5484d9
--- /dev/null
+++ b/sysdeps/x86/tst-shstk-legacy-mod-1.c
@@ -0,0 +1,28 @@
+/* Check legacy shadow stack code in init_array.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stdlib.h>
+
+/* Check legacy shadow stack code in init_array. */
+extern void legacy (void) __attribute__ ((visibility ("hidden")));
+
+void (*init_array []) (void)
+ __attribute__ ((section (".init_array"), aligned (sizeof (void *)))) =
+{
+ &legacy
+};
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 08/17] x86/cet: Check CPU_FEATURE_ACTIVE in permissive mode
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (6 preceding siblings ...)
2023-12-06 17:20 ` [PATCH 07/17] x86/cet: Check legacy shadow stack code in .init_array section H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
2023-12-06 17:20 ` [PATCH 09/17] x86: Check PT_GNU_PROPERTY early H.J. Lu
` (8 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Verify that CPU_FEATURE_ACTIVE works properly in permissive mode.
---
sysdeps/x86/Makefile | 1 +
sysdeps/x86/tst-cet-legacy-4.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index b8b98177e0..6ceefe16c7 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -174,6 +174,7 @@ CFLAGS-tst-cet-legacy-mod-1.c += -fcf-protection=none
CFLAGS-tst-cet-legacy-mod-2.c += -fcf-protection=none
CFLAGS-tst-cet-legacy-3.c += -fcf-protection=none
CFLAGS-tst-cet-legacy-4.c += -fcf-protection=branch
+CPPFLAGS-tst-cet-legacy-4a.c += -DCET_IS_PERMISSIVE=1
CFLAGS-tst-cet-legacy-4a.c += -fcf-protection
CFLAGS-tst-cet-legacy-4b.c += -fcf-protection
CFLAGS-tst-cet-legacy-mod-4.c += -fcf-protection=none
diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c
index d75fb0e61c..c098120253 100644
--- a/sysdeps/x86/tst-cet-legacy-4.c
+++ b/sysdeps/x86/tst-cet-legacy-4.c
@@ -21,6 +21,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/platform/x86.h>
#include <support/check.h>
@@ -40,6 +41,10 @@ do_test (void)
return 0;
}
+#ifdef CET_IS_PERMISSIVE
+ TEST_VERIFY (!CPU_FEATURE_ACTIVE (IBT) && !CPU_FEATURE_ACTIVE (SHSTK));
+#endif
+
fp = dlsym (h, "test");
if (fp == NULL)
FAIL_EXIT1 ("cannot get symbol 'test': %s\n", dlerror ());
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 09/17] x86: Check PT_GNU_PROPERTY early
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (7 preceding siblings ...)
2023-12-06 17:20 ` [PATCH 08/17] x86/cet: Check CPU_FEATURE_ACTIVE in permissive mode H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
2023-12-06 23:57 ` Noah Goldstein
2023-12-06 17:20 ` [PATCH 10/17] x86: Modularize sysdeps/x86/dl-cet.c H.J. Lu
` (7 subsequent siblings)
16 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
The PT_GNU_PROPERTY segment is scanned before PT_NOTE. For binaries
with the PT_GNU_PROPERTY segment, we can check it to avoid scan of
the PT_NOTE segment.
---
sysdeps/x86/dl-prop.h | 120 ++++++++++++++++++++++++++++--------------
1 file changed, 80 insertions(+), 40 deletions(-)
diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index b2836f3009..d2c53c2182 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -81,6 +81,60 @@ _dl_open_check (struct link_map *m)
#endif
}
+/* Check the GNU property and return its value. It returns:
+ -1: Skip this note.
+ 0: Stop checking.
+ 1: Continue to check.
+ */
+static inline int
+_dl_check_gnu_property (unsigned int type, unsigned int datasz,
+ void *ptr, unsigned int *feature_1_and,
+ unsigned int *needed_1,
+ unsigned int *isa_1_needed)
+{
+ if (type == GNU_PROPERTY_X86_FEATURE_1_AND
+ || type == GNU_PROPERTY_X86_ISA_1_NEEDED
+ || type == GNU_PROPERTY_1_NEEDED)
+ {
+ /* The sizes of types which we are searching for are
+ 4 bytes. There is no point to continue if this
+ note is ill-formed. */
+ if (datasz != 4)
+ return -1;
+
+ /* NB: Stop the scan only after seeing all types which
+ we are searching for. */
+ _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
+ > GNU_PROPERTY_X86_FEATURE_1_AND)
+ && (GNU_PROPERTY_X86_FEATURE_1_AND
+ > GNU_PROPERTY_1_NEEDED)),
+ "GNU_PROPERTY_X86_ISA_1_NEEDED > "
+ "GNU_PROPERTY_X86_FEATURE_1_AND && "
+ "GNU_PROPERTY_X86_FEATURE_1_AND > "
+ "GNU_PROPERTY_1_NEEDED");
+ if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
+ *feature_1_and = *(unsigned int *) ptr;
+ else if (type == GNU_PROPERTY_1_NEEDED)
+ *needed_1 = *(unsigned int *) ptr;
+ else
+ {
+ *isa_1_needed = *(unsigned int *) ptr;
+
+ /* Keep searching for the next GNU property note
+ generated by the older linker. */
+ return 0;
+ }
+ }
+ else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
+ {
+ /* Stop the scan since property type is in ascending
+ order. */
+ return 0;
+ }
+
+ return 1;
+}
+
static inline void __attribute__ ((unused))
_dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
const ElfW(Addr) size, const ElfW(Addr) align)
@@ -141,45 +195,14 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
last_type = type;
- if (type == GNU_PROPERTY_X86_FEATURE_1_AND
- || type == GNU_PROPERTY_X86_ISA_1_NEEDED
- || type == GNU_PROPERTY_1_NEEDED)
- {
- /* The sizes of types which we are searching for are
- 4 bytes. There is no point to continue if this
- note is ill-formed. */
- if (datasz != 4)
- return;
-
- /* NB: Stop the scan only after seeing all types which
- we are searching for. */
- _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
- > GNU_PROPERTY_X86_FEATURE_1_AND)
- && (GNU_PROPERTY_X86_FEATURE_1_AND
- > GNU_PROPERTY_1_NEEDED)),
- "GNU_PROPERTY_X86_ISA_1_NEEDED > "
- "GNU_PROPERTY_X86_FEATURE_1_AND && "
- "GNU_PROPERTY_X86_FEATURE_1_AND > "
- "GNU_PROPERTY_1_NEEDED");
- if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
- feature_1_and = *(unsigned int *) ptr;
- else if (type == GNU_PROPERTY_1_NEEDED)
- needed_1 = *(unsigned int *) ptr;
- else
- {
- isa_1_needed = *(unsigned int *) ptr;
-
- /* Keep searching for the next GNU property note
- generated by the older linker. */
- break;
- }
- }
- else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
- {
- /* Stop the scan since property type is in ascending
- order. */
- break;
- }
+ int result = _dl_check_gnu_property (type, datasz, ptr,
+ &feature_1_and,
+ &needed_1,
+ &isa_1_needed);
+ if (result == -1)
+ return; /* Skip this note. */
+ else if (result == 0)
+ break; /* Stop checking. */
/* Check the next property item. */
ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
@@ -217,7 +240,24 @@ static inline int __attribute__ ((always_inline))
_dl_process_gnu_property (struct link_map *l, int fd, uint32_t type,
uint32_t datasz, void *data)
{
- return 0;
+ /* This is called on each GNU property. */
+ unsigned int needed_1 = 0;
+ unsigned int feature_1_and = 0;
+ unsigned int isa_1_needed = 0;
+ int result = _dl_check_gnu_property (type, datasz, data,
+ &feature_1_and, &needed_1,
+ &isa_1_needed);
+ if (needed_1 != 0)
+ l->l_1_needed = needed_1;
+ if (isa_1_needed != 0)
+ l->l_x86_isa_1_needed = isa_1_needed;
+ if (feature_1_and != 0)
+ l->l_x86_feature_1_and = feature_1_and;
+ if ((needed_1 | isa_1_needed | feature_1_and) != 0)
+ l->l_property = lc_property_valid;
+ else if (l->l_property == lc_property_unknown)
+ l->l_property = lc_property_none;
+ return result <= 0 ? 0 : result;
}
#endif /* _DL_PROP_H */
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 09/17] x86: Check PT_GNU_PROPERTY early
2023-12-06 17:20 ` [PATCH 09/17] x86: Check PT_GNU_PROPERTY early H.J. Lu
@ 2023-12-06 23:57 ` Noah Goldstein
2023-12-07 21:06 ` H.J. Lu
0 siblings, 1 reply; 42+ messages in thread
From: Noah Goldstein @ 2023-12-06 23:57 UTC (permalink / raw)
To: H.J. Lu; +Cc: libc-alpha, rick.p.edgecombe
On Wed, Dec 6, 2023 at 11:20 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> The PT_GNU_PROPERTY segment is scanned before PT_NOTE. For binaries
> with the PT_GNU_PROPERTY segment, we can check it to avoid scan of
> the PT_NOTE segment.
> ---
> sysdeps/x86/dl-prop.h | 120 ++++++++++++++++++++++++++++--------------
> 1 file changed, 80 insertions(+), 40 deletions(-)
>
> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> index b2836f3009..d2c53c2182 100644
> --- a/sysdeps/x86/dl-prop.h
> +++ b/sysdeps/x86/dl-prop.h
> @@ -81,6 +81,60 @@ _dl_open_check (struct link_map *m)
> #endif
> }
>
> +/* Check the GNU property and return its value. It returns:
> + -1: Skip this note.
> + 0: Stop checking.
> + 1: Continue to check.
> + */
> +static inline int
> +_dl_check_gnu_property (unsigned int type, unsigned int datasz,
> + void *ptr, unsigned int *feature_1_and,
> + unsigned int *needed_1,
> + unsigned int *isa_1_needed)
> +{
> + if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> + || type == GNU_PROPERTY_X86_ISA_1_NEEDED
> + || type == GNU_PROPERTY_1_NEEDED)
> + {
> + /* The sizes of types which we are searching for are
> + 4 bytes. There is no point to continue if this
> + note is ill-formed. */
> + if (datasz != 4)
> + return -1;
> +
> + /* NB: Stop the scan only after seeing all types which
> + we are searching for. */
> + _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
> + > GNU_PROPERTY_X86_FEATURE_1_AND)
> + && (GNU_PROPERTY_X86_FEATURE_1_AND
> + > GNU_PROPERTY_1_NEEDED)),
> + "GNU_PROPERTY_X86_ISA_1_NEEDED > "
> + "GNU_PROPERTY_X86_FEATURE_1_AND && "
> + "GNU_PROPERTY_X86_FEATURE_1_AND > "
> + "GNU_PROPERTY_1_NEEDED");
> + if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> + *feature_1_and = *(unsigned int *) ptr;
> + else if (type == GNU_PROPERTY_1_NEEDED)
> + *needed_1 = *(unsigned int *) ptr;
> + else
> + {
> + *isa_1_needed = *(unsigned int *) ptr;
> +
> + /* Keep searching for the next GNU property note
> + generated by the older linker. */
> + return 0;
> + }
> + }
> + else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
> + {
> + /* Stop the scan since property type is in ascending
> + order. */
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> static inline void __attribute__ ((unused))
> _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
> const ElfW(Addr) size, const ElfW(Addr) align)
> @@ -141,45 +195,14 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
>
> last_type = type;
>
> - if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> - || type == GNU_PROPERTY_X86_ISA_1_NEEDED
> - || type == GNU_PROPERTY_1_NEEDED)
> - {
> - /* The sizes of types which we are searching for are
> - 4 bytes. There is no point to continue if this
> - note is ill-formed. */
> - if (datasz != 4)
> - return;
> -
> - /* NB: Stop the scan only after seeing all types which
> - we are searching for. */
> - _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
> - > GNU_PROPERTY_X86_FEATURE_1_AND)
> - && (GNU_PROPERTY_X86_FEATURE_1_AND
> - > GNU_PROPERTY_1_NEEDED)),
> - "GNU_PROPERTY_X86_ISA_1_NEEDED > "
> - "GNU_PROPERTY_X86_FEATURE_1_AND && "
> - "GNU_PROPERTY_X86_FEATURE_1_AND > "
> - "GNU_PROPERTY_1_NEEDED");
> - if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> - feature_1_and = *(unsigned int *) ptr;
> - else if (type == GNU_PROPERTY_1_NEEDED)
> - needed_1 = *(unsigned int *) ptr;
> - else
> - {
> - isa_1_needed = *(unsigned int *) ptr;
> -
> - /* Keep searching for the next GNU property note
> - generated by the older linker. */
> - break;
> - }
> - }
> - else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
> - {
> - /* Stop the scan since property type is in ascending
> - order. */
> - break;
> - }
> + int result = _dl_check_gnu_property (type, datasz, ptr,
> + &feature_1_and,
> + &needed_1,
> + &isa_1_needed);
> + if (result == -1)
> + return; /* Skip this note. */
> + else if (result == 0)
> + break; /* Stop checking. */
>
> /* Check the next property item. */
> ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> @@ -217,7 +240,24 @@ static inline int __attribute__ ((always_inline))
> _dl_process_gnu_property (struct link_map *l, int fd, uint32_t type,
> uint32_t datasz, void *data)
> {
> - return 0;
> + /* This is called on each GNU property. */
> + unsigned int needed_1 = 0;
> + unsigned int feature_1_and = 0;
> + unsigned int isa_1_needed = 0;
> + int result = _dl_check_gnu_property (type, datasz, data,
> + &feature_1_and, &needed_1,
> + &isa_1_needed);
> + if (needed_1 != 0)
> + l->l_1_needed = needed_1;
> + if (isa_1_needed != 0)
> + l->l_x86_isa_1_needed = isa_1_needed;
> + if (feature_1_and != 0)
> + l->l_x86_feature_1_and = feature_1_and;
> + if ((needed_1 | isa_1_needed | feature_1_and) != 0)
> + l->l_property = lc_property_valid;
> + else if (l->l_property == lc_property_unknown)
> + l->l_property = lc_property_none;
> + return result <= 0 ? 0 : result;
These seems like a seperate change from the refactor mentioned in the
commit message?
> }
>
> #endif /* _DL_PROP_H */
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 09/17] x86: Check PT_GNU_PROPERTY early
2023-12-06 23:57 ` Noah Goldstein
@ 2023-12-07 21:06 ` H.J. Lu
0 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-07 21:06 UTC (permalink / raw)
To: Noah Goldstein; +Cc: libc-alpha, rick.p.edgecombe
On Wed, Dec 6, 2023 at 3:57 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Wed, Dec 6, 2023 at 11:20 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > The PT_GNU_PROPERTY segment is scanned before PT_NOTE. For binaries
> > with the PT_GNU_PROPERTY segment, we can check it to avoid scan of
> > the PT_NOTE segment.
> > ---
> > sysdeps/x86/dl-prop.h | 120 ++++++++++++++++++++++++++++--------------
> > 1 file changed, 80 insertions(+), 40 deletions(-)
> >
> > diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> > index b2836f3009..d2c53c2182 100644
> > --- a/sysdeps/x86/dl-prop.h
> > +++ b/sysdeps/x86/dl-prop.h
> > @@ -81,6 +81,60 @@ _dl_open_check (struct link_map *m)
> > #endif
> > }
> >
> > +/* Check the GNU property and return its value. It returns:
> > + -1: Skip this note.
> > + 0: Stop checking.
> > + 1: Continue to check.
> > + */
> > +static inline int
> > +_dl_check_gnu_property (unsigned int type, unsigned int datasz,
> > + void *ptr, unsigned int *feature_1_and,
> > + unsigned int *needed_1,
> > + unsigned int *isa_1_needed)
> > +{
> > + if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> > + || type == GNU_PROPERTY_X86_ISA_1_NEEDED
> > + || type == GNU_PROPERTY_1_NEEDED)
> > + {
> > + /* The sizes of types which we are searching for are
> > + 4 bytes. There is no point to continue if this
> > + note is ill-formed. */
> > + if (datasz != 4)
> > + return -1;
> > +
> > + /* NB: Stop the scan only after seeing all types which
> > + we are searching for. */
> > + _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
> > + > GNU_PROPERTY_X86_FEATURE_1_AND)
> > + && (GNU_PROPERTY_X86_FEATURE_1_AND
> > + > GNU_PROPERTY_1_NEEDED)),
> > + "GNU_PROPERTY_X86_ISA_1_NEEDED > "
> > + "GNU_PROPERTY_X86_FEATURE_1_AND && "
> > + "GNU_PROPERTY_X86_FEATURE_1_AND > "
> > + "GNU_PROPERTY_1_NEEDED");
> > + if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> > + *feature_1_and = *(unsigned int *) ptr;
> > + else if (type == GNU_PROPERTY_1_NEEDED)
> > + *needed_1 = *(unsigned int *) ptr;
> > + else
> > + {
> > + *isa_1_needed = *(unsigned int *) ptr;
> > +
> > + /* Keep searching for the next GNU property note
> > + generated by the older linker. */
> > + return 0;
> > + }
> > + }
> > + else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
> > + {
> > + /* Stop the scan since property type is in ascending
> > + order. */
> > + return 0;
> > + }
> > +
> > + return 1;
> > +}
> > +
> > static inline void __attribute__ ((unused))
> > _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
> > const ElfW(Addr) size, const ElfW(Addr) align)
> > @@ -141,45 +195,14 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
> >
> > last_type = type;
> >
> > - if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> > - || type == GNU_PROPERTY_X86_ISA_1_NEEDED
> > - || type == GNU_PROPERTY_1_NEEDED)
> > - {
> > - /* The sizes of types which we are searching for are
> > - 4 bytes. There is no point to continue if this
> > - note is ill-formed. */
> > - if (datasz != 4)
> > - return;
> > -
> > - /* NB: Stop the scan only after seeing all types which
> > - we are searching for. */
> > - _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
> > - > GNU_PROPERTY_X86_FEATURE_1_AND)
> > - && (GNU_PROPERTY_X86_FEATURE_1_AND
> > - > GNU_PROPERTY_1_NEEDED)),
> > - "GNU_PROPERTY_X86_ISA_1_NEEDED > "
> > - "GNU_PROPERTY_X86_FEATURE_1_AND && "
> > - "GNU_PROPERTY_X86_FEATURE_1_AND > "
> > - "GNU_PROPERTY_1_NEEDED");
> > - if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> > - feature_1_and = *(unsigned int *) ptr;
> > - else if (type == GNU_PROPERTY_1_NEEDED)
> > - needed_1 = *(unsigned int *) ptr;
> > - else
> > - {
> > - isa_1_needed = *(unsigned int *) ptr;
> > -
> > - /* Keep searching for the next GNU property note
> > - generated by the older linker. */
> > - break;
> > - }
> > - }
> > - else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
> > - {
> > - /* Stop the scan since property type is in ascending
> > - order. */
> > - break;
> > - }
> > + int result = _dl_check_gnu_property (type, datasz, ptr,
> > + &feature_1_and,
> > + &needed_1,
> > + &isa_1_needed);
> > + if (result == -1)
> > + return; /* Skip this note. */
> > + else if (result == 0)
> > + break; /* Stop checking. */
> >
> > /* Check the next property item. */
> > ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> > @@ -217,7 +240,24 @@ static inline int __attribute__ ((always_inline))
> > _dl_process_gnu_property (struct link_map *l, int fd, uint32_t type,
> > uint32_t datasz, void *data)
> > {
> > - return 0;
> > + /* This is called on each GNU property. */
> > + unsigned int needed_1 = 0;
> > + unsigned int feature_1_and = 0;
> > + unsigned int isa_1_needed = 0;
> > + int result = _dl_check_gnu_property (type, datasz, data,
> > + &feature_1_and, &needed_1,
> > + &isa_1_needed);
> > + if (needed_1 != 0)
> > + l->l_1_needed = needed_1;
> > + if (isa_1_needed != 0)
> > + l->l_x86_isa_1_needed = isa_1_needed;
> > + if (feature_1_and != 0)
> > + l->l_x86_feature_1_and = feature_1_and;
> > + if ((needed_1 | isa_1_needed | feature_1_and) != 0)
> > + l->l_property = lc_property_valid;
> > + else if (l->l_property == lc_property_unknown)
> > + l->l_property = lc_property_none;
> > + return result <= 0 ? 0 : result;
> These seems like a seperate change from the refactor mentioned in the
> commit message?
Done. I sent out a separate patch.
Thanks.
> > }
> >
> > #endif /* _DL_PROP_H */
> > --
> > 2.43.0
> >
--
H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 10/17] x86: Modularize sysdeps/x86/dl-cet.c
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (8 preceding siblings ...)
2023-12-06 17:20 ` [PATCH 09/17] x86: Check PT_GNU_PROPERTY early H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
2023-12-06 17:20 ` [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface H.J. Lu
` (6 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Improve readability and make maintenance easier for dl-feature.c by
modularizing sysdeps/x86/dl-cet.c:
1. Support processors with:
a. Only IBT. Or
b. Only SHSTK. Or
c. Both IBT and SHSTK.
2. Lock CET features only if IBT or SHSTK are enabled and are not
enabled permissively.
---
sysdeps/x86/dl-cet.c | 456 ++++++++++++++++++++++++++-----------------
1 file changed, 280 insertions(+), 176 deletions(-)
diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
index 60ea1cb558..67c51ee8c2 100644
--- a/sysdeps/x86/dl-cet.c
+++ b/sysdeps/x86/dl-cet.c
@@ -32,206 +32,310 @@
# error GNU_PROPERTY_X86_FEATURE_1_SHSTK != X86_FEATURE_1_SHSTK
#endif
-/* Check if object M is compatible with CET. */
+struct dl_cet_info
+{
+ const char *program;
+
+ /* Check how IBT and SHSTK should be enabled. */
+ enum dl_x86_cet_control enable_ibt_type;
+ enum dl_x86_cet_control enable_shstk_type;
+
+ /* If IBT and SHSTK were previously enabled. */
+ unsigned int feature_1_enabled;
+
+ /* If IBT and SHSTK should be enabled. */
+ unsigned int enable_feature_1;
+
+ /* If there are any legacy shared object. */
+ unsigned int feature_1_legacy;
+
+ /* Which shared object is the first legacy shared object. */
+ unsigned int feature_1_legacy_ibt;
+ unsigned int feature_1_legacy_shstk;
+};
+
+/* Check if the object M and its dependencies are legacy object. */
static void
-dl_cet_check (struct link_map *m, const char *program)
+dl_check_legacy_object (struct link_map *m,
+ struct dl_cet_info *info)
{
- /* Check how IBT should be enabled. */
- enum dl_x86_cet_control enable_ibt_type
- = GL(dl_x86_feature_control).ibt;
- /* Check how SHSTK should be enabled. */
- enum dl_x86_cet_control enable_shstk_type
- = GL(dl_x86_feature_control).shstk;
-
- /* No legacy object check if both IBT and SHSTK are always on. */
- if (enable_ibt_type == cet_always_on
- && enable_shstk_type == cet_always_on)
+ unsigned int i;
+ struct link_map *l = NULL;
+
+ i = m->l_searchlist.r_nlist;
+ while (i-- > 0)
{
- THREAD_SETMEM (THREAD_SELF, header.feature_1, GL(dl_x86_feature_1));
- return;
- }
+ /* Check each shared object to see if IBT and SHSTK are enabled. */
+ l = m->l_initfini[i];
- /* Check if IBT is enabled by kernel. */
- bool ibt_enabled
- = (GL(dl_x86_feature_1) & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0;
- /* Check if SHSTK is enabled by kernel. */
- bool shstk_enabled
- = (GL(dl_x86_feature_1) & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0;
+ if (l->l_init_called)
+ continue;
- if (ibt_enabled || shstk_enabled)
- {
- struct link_map *l = NULL;
- unsigned int ibt_legacy = 0, shstk_legacy = 0;
- bool found_ibt_legacy = false, found_shstk_legacy = false;
-
- /* Check if IBT and SHSTK are enabled in object. */
- bool enable_ibt = (ibt_enabled
- && enable_ibt_type != cet_always_off);
- bool enable_shstk = (shstk_enabled
- && enable_shstk_type != cet_always_off);
- if (program)
+#ifdef SHARED
+ /* Skip check for ld.so since it has the features enabled. The
+ features will be disabled later if they are not enabled in
+ executable. */
+ if (l == &GL(dl_rtld_map)
+ || l->l_real == &GL(dl_rtld_map)
+ || (info->program != NULL && l == m))
+ continue;
+#endif
+
+ /* IBT and SHSTK set only if enabled in executable and all DSOs.
+ NB: cet_always_on is handled outside of the loop. */
+ info->enable_feature_1 &= ((l->l_x86_feature_1_and
+ & (GNU_PROPERTY_X86_FEATURE_1_IBT
+ | GNU_PROPERTY_X86_FEATURE_1_SHSTK))
+ | ~(GNU_PROPERTY_X86_FEATURE_1_IBT
+ | GNU_PROPERTY_X86_FEATURE_1_SHSTK));
+ if ((info->feature_1_legacy
+ & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0
+ && ((info->enable_feature_1
+ & GNU_PROPERTY_X86_FEATURE_1_IBT)
+ != (info->feature_1_enabled
+ & GNU_PROPERTY_X86_FEATURE_1_IBT)))
{
- /* Enable IBT and SHSTK only if they are enabled in executable.
- NB: IBT and SHSTK may be disabled by environment variable:
-
- GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
- */
- enable_ibt &= (CPU_FEATURE_USABLE (IBT)
- && (enable_ibt_type == cet_always_on
- || (m->l_x86_feature_1_and
- & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0));
- enable_shstk &= (CPU_FEATURE_USABLE (SHSTK)
- && (enable_shstk_type == cet_always_on
- || (m->l_x86_feature_1_and
- & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0));
+ info->feature_1_legacy_ibt = i;
+ info->feature_1_legacy |= GNU_PROPERTY_X86_FEATURE_1_IBT;
}
- /* ld.so is CET-enabled by kernel. But shared objects may not
- support IBT nor SHSTK. */
- if (enable_ibt || enable_shstk)
- {
- unsigned int i;
+ if ((info->feature_1_legacy
+ & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0
+ && ((info->enable_feature_1
+ & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
+ != (info->feature_1_enabled
+ & GNU_PROPERTY_X86_FEATURE_1_SHSTK)))
+ {
+ info->feature_1_legacy_shstk = i;
+ info->feature_1_legacy |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+ }
+ }
- i = m->l_searchlist.r_nlist;
- while (i-- > 0)
- {
- /* Check each shared object to see if IBT and SHSTK are
- enabled. */
- l = m->l_initfini[i];
+ /* Handle cet_always_on. */
+ if ((info->feature_1_enabled
+ & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0
+ && info->enable_ibt_type == cet_always_on)
+ {
+ info->feature_1_legacy &= ~GNU_PROPERTY_X86_FEATURE_1_IBT;
+ info->enable_feature_1 |= GNU_PROPERTY_X86_FEATURE_1_IBT;
+ }
- if (l->l_init_called)
- continue;
+ if ((info->feature_1_enabled
+ & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0
+ && info->enable_shstk_type == cet_always_on)
+ {
+ info->feature_1_legacy &= ~GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+ info->enable_feature_1 |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+ }
+}
#ifdef SHARED
- /* Skip CET check for ld.so since ld.so is CET-enabled.
- CET will be disabled later if CET isn't enabled in
- executable. */
- if (l == &GL(dl_rtld_map)
- || l->l_real == &GL(dl_rtld_map)
- || (program && l == m))
- continue;
+/* Enable IBT and SHSTK only if they are enabled in executable. Set
+ feature bits properly at the start of the program. */
+
+static void
+dl_cet_check_startup (struct link_map *m, struct dl_cet_info *info)
+{
+ /* NB: IBT and SHSTK may be disabled by environment variable:
+
+ GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK.
+ */
+ if (CPU_FEATURE_USABLE (IBT))
+ {
+ if (info->enable_ibt_type == cet_always_on)
+ info->enable_feature_1 |= GNU_PROPERTY_X86_FEATURE_1_IBT;
+ else
+ info->enable_feature_1 &= ((m->l_x86_feature_1_and
+ & GNU_PROPERTY_X86_FEATURE_1_IBT)
+ | ~GNU_PROPERTY_X86_FEATURE_1_IBT);
+ }
+ else
+ info->enable_feature_1 &= ~GNU_PROPERTY_X86_FEATURE_1_IBT;
+
+ if (CPU_FEATURE_USABLE (SHSTK))
+ {
+ if (info->enable_shstk_type == cet_always_on)
+ info->enable_feature_1 |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+ else
+ info->enable_feature_1 &= ((m->l_x86_feature_1_and
+ & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
+ | ~GNU_PROPERTY_X86_FEATURE_1_SHSTK);
+ }
+ else
+ info->enable_feature_1 &= ~GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+
+ if (info->enable_feature_1 != 0)
+ dl_check_legacy_object (m, info);
+
+ unsigned int disable_feature_1
+ = info->enable_feature_1 ^ info->feature_1_enabled;
+ if (disable_feature_1 != 0)
+ {
+ /* Disable features in the kernel because of legacy objects or
+ cet_always_off. */
+ if (dl_cet_disable_cet (disable_feature_1) != 0)
+ _dl_fatal_printf ("%s: can't disable x86 Features\n",
+ info->program);
+
+ /* Clear the disabled bits. Sync dl_x86_feature_1 and
+ info->feature_1_enabled with info->enable_feature_1. */
+ info->feature_1_enabled = info->enable_feature_1;
+ GL(dl_x86_feature_1) = info->enable_feature_1;
+ }
+
+ if (HAS_CPU_FEATURE (IBT) || HAS_CPU_FEATURE (SHSTK))
+ {
+ /* Lock CET features only if IBT or SHSTK are enabled and are not
+ enabled permissively. */
+ unsigned int feature_1_lock = 0;
+
+ if (((info->feature_1_enabled & GNU_PROPERTY_X86_FEATURE_1_IBT)
+ != 0)
+ && info->enable_ibt_type != cet_permissive)
+ feature_1_lock |= GNU_PROPERTY_X86_FEATURE_1_IBT;
+
+ if (((info->feature_1_enabled & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
+ != 0)
+ && info->enable_shstk_type != cet_permissive)
+ feature_1_lock |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+
+ if (feature_1_lock != 0
+ && dl_cet_lock_cet () != 0)
+ _dl_fatal_printf ("%s: can't lock CET\n", info->program);
+ }
+
+ THREAD_SETMEM (THREAD_SELF, header.feature_1, GL(dl_x86_feature_1));
+}
#endif
- /* IBT is enabled only if it is enabled in executable as
- well as all shared objects. */
- enable_ibt &= (enable_ibt_type == cet_always_on
- || (l->l_x86_feature_1_and
- & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0);
- if (!found_ibt_legacy && enable_ibt != ibt_enabled)
- {
- found_ibt_legacy = true;
- ibt_legacy = i;
- }
-
- /* SHSTK is enabled only if it is enabled in executable as
- well as all shared objects. */
- enable_shstk &= (enable_shstk_type == cet_always_on
- || (l->l_x86_feature_1_and
- & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0);
- if (enable_shstk != shstk_enabled)
- {
- found_shstk_legacy = true;
- shstk_legacy = i;
- }
- }
- }
+/* Check feature bits when dlopening the shared object M. */
+
+static void
+dl_cet_check_dlopen (struct link_map *m, struct dl_cet_info *info)
+{
+ /* Check if there are any legacy objects loaded. */
+ if (info->enable_feature_1 != 0)
+ {
+ dl_check_legacy_object (m, info);
- bool cet_feature_changed = false;
+ /* Skip if there are no legacy shared objects loaded. */
+ if (info->feature_1_legacy == 0)
+ return;
+ }
- if (enable_ibt != ibt_enabled || enable_shstk != shstk_enabled)
- {
- if (!program)
- {
- if (enable_ibt_type != cet_permissive)
- {
- /* When IBT is enabled, we cannot dlopen a shared
- object without IBT. */
- if (found_ibt_legacy)
- _dl_signal_error (0,
- m->l_initfini[ibt_legacy]->l_name,
- "dlopen",
- N_("rebuild shared object with IBT support enabled"));
- }
-
- if (enable_shstk_type != cet_permissive)
- {
- /* When SHSTK is enabled, we cannot dlopen a shared
- object without SHSTK. */
- if (found_shstk_legacy)
- _dl_signal_error (0,
- m->l_initfini[shstk_legacy]->l_name,
- "dlopen",
- N_("rebuild shared object with SHSTK support enabled"));
- }
-
- if (enable_ibt_type != cet_permissive
- && enable_shstk_type != cet_permissive)
- return;
- }
-
- /* Disable IBT and/or SHSTK if they are enabled by kernel, but
- disabled in executable or shared objects. */
- unsigned int cet_feature = 0;
-
- if (!enable_ibt)
- cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT;
- if (!enable_shstk)
- cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
-
- int res = dl_cet_disable_cet (cet_feature);
- if (res != 0)
- {
- if (program)
- _dl_fatal_printf ("%s: can't disable CET\n", program);
- else
- {
- if (found_ibt_legacy)
- l = m->l_initfini[ibt_legacy];
- else
- l = m->l_initfini[shstk_legacy];
- _dl_signal_error (-res, l->l_name, "dlopen",
- N_("can't disable CET"));
- }
- }
-
- /* Clear the disabled bits in dl_x86_feature_1. */
- GL(dl_x86_feature_1) &= ~cet_feature;
-
- cet_feature_changed = true;
- }
+ unsigned int disable_feature_1 = 0;
+ unsigned int legacy_obj = 0;
+ const char *msg = NULL;
-#ifdef SHARED
- if (program && (ibt_enabled || shstk_enabled))
+ if ((info->feature_1_enabled
+ & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0
+ && (info->feature_1_legacy
+ & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0)
+ {
+ if (info->enable_ibt_type != cet_permissive)
{
- if ((!ibt_enabled
- || enable_ibt_type != cet_permissive)
- && (!shstk_enabled
- || enable_shstk_type != cet_permissive))
- {
- /* Lock CET if IBT or SHSTK is enabled in executable unless
- IBT or SHSTK is enabled permissively. */
- int res = dl_cet_lock_cet ();
- if (res != 0)
- _dl_fatal_printf ("%s: can't lock CET\n", program);
- }
-
- /* Set feature_1 if IBT or SHSTK is enabled in executable. */
- cet_feature_changed = true;
+ legacy_obj = info->feature_1_legacy_ibt;
+ msg = N_("rebuild shared object with IBT support enabled");
}
-#endif
+ else
+ disable_feature_1 |= GNU_PROPERTY_X86_FEATURE_1_IBT;
+ }
- if (cet_feature_changed)
+ /* Check the next feature only if there is no error. */
+ if (msg == NULL
+ && (info->feature_1_enabled
+ & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0
+ && (info->feature_1_legacy
+ & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0)
+ {
+ if (info->enable_shstk_type != cet_permissive)
{
- unsigned int feature_1 = 0;
- if (enable_ibt)
- feature_1 |= GNU_PROPERTY_X86_FEATURE_1_IBT;
- if (enable_shstk)
- feature_1 |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
- struct pthread *self = THREAD_SELF;
- THREAD_SETMEM (self, header.feature_1, feature_1);
+ legacy_obj = info->feature_1_legacy_shstk;
+ msg = N_("rebuild shared object with SHSTK support enabled");
}
+ else
+ disable_feature_1 |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+ }
+
+ /* If there is an error, long jump back to the caller. */
+ if (msg != NULL)
+ _dl_signal_error (0, m->l_initfini[legacy_obj]->l_name, "dlopen",
+ msg);
+
+ if (disable_feature_1 != 0)
+ {
+ int res = dl_cet_disable_cet (disable_feature_1);
+ if (res)
+ {
+ if ((disable_feature_1
+ & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0)
+ msg = N_("can't disable IBT");
+ else
+ msg = N_("can't disable SHSTK");
+ /* Long jump back to the caller on error. */
+ _dl_signal_error (-res, m->l_initfini[legacy_obj]->l_name,
+ "dlopen", msg);
+ }
+
+ /* Clear the disabled bits in dl_x86_feature_1. */
+ GL(dl_x86_feature_1) &= ~disable_feature_1;
+
+ THREAD_SETMEM (THREAD_SELF, header.feature_1,
+ GL(dl_x86_feature_1));
+ }
+}
+
+static void
+dl_cet_check (struct link_map *m, const char *program)
+{
+ struct dl_cet_info info;
+
+ /* Check how IBT and SHSTK should be enabled. */
+ info.enable_ibt_type = GL(dl_x86_feature_control).ibt;
+ info.enable_shstk_type = GL(dl_x86_feature_control).shstk;
+
+ info.feature_1_enabled = GL(dl_x86_feature_1);
+
+ /* No legacy object check if IBT and SHSTK are always on. */
+ if (info.enable_ibt_type == cet_always_on
+ && info.enable_shstk_type == cet_always_on)
+ {
+#ifdef SHARED
+ /* Set it only during startup. */
+ if (program != NULL)
+ THREAD_SETMEM (THREAD_SELF, header.feature_1,
+ info.feature_1_enabled);
+#endif
+ return;
}
+
+ /* Check if IBT and SHSTK were enabled by kernel. */
+ if (info.feature_1_enabled == 0)
+ return;
+
+ info.program = program;
+
+ /* Check which features should be enabled. */
+ info.enable_feature_1 = 0;
+ if (info.enable_ibt_type != cet_always_off)
+ info.enable_feature_1 |= (info.feature_1_enabled
+ & GNU_PROPERTY_X86_FEATURE_1_IBT);
+ if (info.enable_shstk_type != cet_always_off)
+ info.enable_feature_1 |= (info.feature_1_enabled
+ & GNU_PROPERTY_X86_FEATURE_1_SHSTK);
+
+ /* Start with no legacy objects. */
+ info.feature_1_legacy = 0;
+ info.feature_1_legacy_ibt = 0;
+ info.feature_1_legacy_shstk = 0;
+
+#ifdef SHARED
+ if (program)
+ dl_cet_check_startup (m, &info);
+ else
+#endif
+ dl_cet_check_dlopen (m, &info);
}
void
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (9 preceding siblings ...)
2023-12-06 17:20 ` [PATCH 10/17] x86: Modularize sysdeps/x86/dl-cet.c H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
2023-12-11 11:34 ` Szabolcs Nagy
2023-12-06 17:20 ` [PATCH 12/17] elf: Always provide _dl_get_dl_main_map in libc.a H.J. Lu
` (5 subsequent siblings)
16 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Sync with Linux kernel 6.6 shadow stack interface. Since only x86-64 is
supported, i386 shadow stack codes are unchanged and CET shouldn't be
enabled for i386.
1. When the shadow stack base in TCB is unset, the default shadow stack
is in use. Use the current shadow stack pointer as the marker for the
default shadow stack.
2. Allocate shadow stack with the map_shadow_stack syscall.
3. Rename arch_prctl CET commands to ARCH_SHSTK_XXX.
4. Rewrite the CET control functions with the current kernel shadow stack
interface.
Since CET is no longer enabled by kernel, a separate patch will enable
shadow stack during startup.
---
sysdeps/unix/sysv/linux/x86/Makefile | 1 +
.../sysv/linux/x86/allocate-shadow-stack.c | 54 +++++++++++++++++++
.../sysv/linux/x86/allocate-shadow-stack.h | 27 ++++++++++
sysdeps/unix/sysv/linux/x86/bits/mman.h | 5 ++
sysdeps/unix/sysv/linux/x86/cpu-features.c | 13 +++--
sysdeps/unix/sysv/linux/x86/dl-cet.h | 16 ++++--
.../unix/sysv/linux/x86/include/asm/prctl.h | 37 ++++++-------
.../sysv/linux/x86/tst-cet-setcontext-1.c | 17 +++---
.../unix/sysv/linux/x86_64/__start_context.S | 38 +++----------
sysdeps/unix/sysv/linux/x86_64/getcontext.S | 30 ++---------
sysdeps/unix/sysv/linux/x86_64/makecontext.c | 29 +++++-----
sysdeps/unix/sysv/linux/x86_64/swapcontext.S | 22 ++------
sysdeps/x86/cpu-features.c | 15 ++++--
sysdeps/x86/dl-cet.c | 2 +-
sysdeps/x86_64/nptl/tls.h | 2 +-
15 files changed, 176 insertions(+), 132 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/x86/allocate-shadow-stack.c
create mode 100644 sysdeps/unix/sysv/linux/x86/allocate-shadow-stack.h
diff --git a/sysdeps/unix/sysv/linux/x86/Makefile b/sysdeps/unix/sysv/linux/x86/Makefile
index 9dfdd689a9..ed0d6500b9 100644
--- a/sysdeps/unix/sysv/linux/x86/Makefile
+++ b/sysdeps/unix/sysv/linux/x86/Makefile
@@ -44,6 +44,7 @@ CFLAGS-tst-cet-vfork-1.c += -mshstk
endif
ifeq ($(subdir),stdlib)
+sysdep_routines += allocate-shadow-stack
tests += tst-cet-setcontext-1
CFLAGS-tst-cet-setcontext-1.c += -mshstk
endif
diff --git a/sysdeps/unix/sysv/linux/x86/allocate-shadow-stack.c b/sysdeps/unix/sysv/linux/x86/allocate-shadow-stack.c
new file mode 100644
index 0000000000..3a76db1a60
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/allocate-shadow-stack.c
@@ -0,0 +1,54 @@
+/* Helper function to allocate shadow stack.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <sysdep.h>
+#include <stdint.h>
+#include <errno.h>
+#include <sys/mman.h>
+#include <libc-pointer-arith.h>
+#include <allocate-shadow-stack.h>
+
+/* NB: This can be treated as a syscall by caller. */
+
+#ifndef __x86_64__
+__attribute__ ((regparm (2)))
+#endif
+long int
+__allocate_shadow_stack (size_t stack_size,
+ shadow_stack_size_t *child_stack)
+{
+#ifdef __NR_map_shadow_stack
+ size_t shadow_stack_size
+ = stack_size >> STACK_SIZE_TO_SHADOW_STACK_SIZE_SHIFT;
+ /* Align shadow stack to 8 bytes. */
+ shadow_stack_size = ALIGN_UP (shadow_stack_size, 8);
+ void *shadow_stack = (void *)INLINE_SYSCALL_CALL
+ (map_shadow_stack, NULL, shadow_stack_size, SHADOW_STACK_SET_TOKEN);
+ /* Report the map_shadow_stack error. */
+ if (shadow_stack == MAP_FAILED)
+ return -errno;
+
+ /* Save the shadow stack base and size on child stack. */
+ child_stack[0] = (uintptr_t) shadow_stack;
+ child_stack[1] = shadow_stack_size;
+
+ return 0;
+#else
+ return -ENOSYS;
+#endif
+}
diff --git a/sysdeps/unix/sysv/linux/x86/allocate-shadow-stack.h b/sysdeps/unix/sysv/linux/x86/allocate-shadow-stack.h
new file mode 100644
index 0000000000..834373e0d3
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/allocate-shadow-stack.h
@@ -0,0 +1,27 @@
+/* Helper function to allocate shadow stack.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <ucontext.h>
+
+typedef __typeof (((ucontext_t *) 0)->__ssp[0]) shadow_stack_size_t;
+
+extern long int __allocate_shadow_stack (size_t, shadow_stack_size_t *)
+#ifndef __x86_64__
+ __attribute__ ((regparm (2)))
+#endif
+ attribute_hidden;
diff --git a/sysdeps/unix/sysv/linux/x86/bits/mman.h b/sysdeps/unix/sysv/linux/x86/bits/mman.h
index 3d356e86a0..221f7c82bd 100644
--- a/sysdeps/unix/sysv/linux/x86/bits/mman.h
+++ b/sysdeps/unix/sysv/linux/x86/bits/mman.h
@@ -27,6 +27,11 @@
#define MAP_32BIT 0x40 /* Only give out 32-bit addresses. */
#define MAP_ABOVE4G 0x80 /* Only map above 4GB. */
+#ifdef __USE_MISC
+/* Set up a restore token in the newly allocatd shadow stack */
+# define SHADOW_STACK_SET_TOKEN 0x1
+#endif
+
#include <bits/mman-map-flags-generic.h>
/* Include generic Linux declarations. */
diff --git a/sysdeps/unix/sysv/linux/x86/cpu-features.c b/sysdeps/unix/sysv/linux/x86/cpu-features.c
index 41e7600668..0e6e2bf855 100644
--- a/sysdeps/unix/sysv/linux/x86/cpu-features.c
+++ b/sysdeps/unix/sysv/linux/x86/cpu-features.c
@@ -23,10 +23,15 @@
static inline int __attribute__ ((always_inline))
get_cet_status (void)
{
- unsigned long long cet_status[3];
- if (INTERNAL_SYSCALL_CALL (arch_prctl, ARCH_CET_STATUS, cet_status) == 0)
- return cet_status[0];
- return 0;
+ unsigned long long kernel_feature;
+ unsigned int status = 0;
+ if (INTERNAL_SYSCALL_CALL (arch_prctl, ARCH_SHSTK_STATUS,
+ &kernel_feature) == 0)
+ {
+ if ((kernel_feature & ARCH_SHSTK_SHSTK) != 0)
+ status = GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+ }
+ return status;
}
# ifndef SHARED
diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h b/sysdeps/unix/sysv/linux/x86/dl-cet.h
index c885bf1323..da220ac627 100644
--- a/sysdeps/unix/sysv/linux/x86/dl-cet.h
+++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h
@@ -21,12 +21,20 @@
static inline int __attribute__ ((always_inline))
dl_cet_disable_cet (unsigned int cet_feature)
{
- return (int) INTERNAL_SYSCALL_CALL (arch_prctl, ARCH_CET_DISABLE,
- cet_feature);
+ if (cet_feature != GNU_PROPERTY_X86_FEATURE_1_SHSTK)
+ return -1;
+ long long int kernel_feature = ARCH_SHSTK_SHSTK;
+ return (int) INTERNAL_SYSCALL_CALL (arch_prctl, ARCH_SHSTK_DISABLE,
+ kernel_feature);
}
static inline int __attribute__ ((always_inline))
-dl_cet_lock_cet (void)
+dl_cet_lock_cet (unsigned int cet_feature)
{
- return (int) INTERNAL_SYSCALL_CALL (arch_prctl, ARCH_CET_LOCK, 0);
+ if (cet_feature != GNU_PROPERTY_X86_FEATURE_1_SHSTK)
+ return -1;
+ /* Lock all SHSTK features. */
+ long long int kernel_feature = -1;
+ return (int) INTERNAL_SYSCALL_CALL (arch_prctl, ARCH_SHSTK_LOCK,
+ kernel_feature);
}
diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h
index 45ad0b052f..2f511321ad 100644
--- a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h
+++ b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h
@@ -4,24 +4,19 @@
#include_next <asm/prctl.h>
-#ifndef ARCH_CET_STATUS
-/* CET features:
- IBT: GNU_PROPERTY_X86_FEATURE_1_IBT
- SHSTK: GNU_PROPERTY_X86_FEATURE_1_SHSTK
- */
-/* Return CET features in unsigned long long *addr:
- features: addr[0].
- shadow stack base address: addr[1].
- shadow stack size: addr[2].
- */
-# define ARCH_CET_STATUS 0x3001
-/* Disable CET features in unsigned int features. */
-# define ARCH_CET_DISABLE 0x3002
-/* Lock all CET features. */
-# define ARCH_CET_LOCK 0x3003
-/* Allocate a new shadow stack with unsigned long long *addr:
- IN: requested shadow stack size: *addr.
- OUT: allocated shadow stack address: *addr.
- */
-# define ARCH_CET_ALLOC_SHSTK 0x3004
-#endif /* ARCH_CET_STATUS */
+#ifndef ARCH_SHSTK_ENABLE
+/* Enable SHSTK features in unsigned long int features. */
+# define ARCH_SHSTK_ENABLE 0x5001
+/* Disable SHSTK features in unsigned long int features. */
+# define ARCH_SHSTK_DISABLE 0x5002
+/* Lock SHSTK features in unsigned long int features. */
+# define ARCH_SHSTK_LOCK 0x5003
+/* Unlock SHSTK features in unsigned long int features. */
+# define ARCH_SHSTK_UNLOCK 0x5004
+/* Return SHSTK features in unsigned long int features. */
+# define ARCH_SHSTK_STATUS 0x5005
+
+/* ARCH_SHSTK_ features bits */
+# define ARCH_SHSTK_SHSTK 0x1
+# define ARCH_SHSTK_WRSS 0x2
+#endif
diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-setcontext-1.c b/sysdeps/unix/sysv/linux/x86/tst-cet-setcontext-1.c
index 837a9fd0eb..2ea66c803b 100644
--- a/sysdeps/unix/sysv/linux/x86/tst-cet-setcontext-1.c
+++ b/sysdeps/unix/sysv/linux/x86/tst-cet-setcontext-1.c
@@ -87,15 +87,14 @@ do_test (void)
ctx[4].uc_link = &ctx[0];
makecontext (&ctx[4], (void (*) (void)) f1, 0);
- /* NB: When shadow stack is enabled, makecontext calls arch_prctl
- with ARCH_CET_ALLOC_SHSTK to allocate a new shadow stack which
- can be unmapped. The base address and size of the new shadow
- stack are returned in __ssp[1] and __ssp[2]. makecontext is
- called for CTX1, CTX3 and CTX4. But only CTX1 is used. New
- shadow stacks are allocated in the order of CTX3, CTX1, CTX4.
- It is very likely that CTX1's shadow stack is placed between
- CTX3 and CTX4. We munmap CTX3's and CTX4's shadow stacks to
- create gaps above and below CTX1's shadow stack. We check
+ /* NB: When shadow stack is enabled, makecontext calls map_shadow_stack
+ to allocate a new shadow stack which can be unmapped. The base
+ address and size of the new shadow stack are returned in __ssp[1]
+ and __ssp[2]. makecontext is called for CTX1, CTX3 and CTX4. But
+ only CTX1 is used. New shadow stacks are allocated in the order
+ of CTX3, CTX1, CTX4. It is very likely that CTX1's shadow stack is
+ placed between CTX3 and CTX4. We munmap CTX3's and CTX4's shadow
+ stacks to create gaps above and below CTX1's shadow stack. We check
that setcontext CTX1 works correctly in this case. */
if (_get_ssp () != 0)
{
diff --git a/sysdeps/unix/sysv/linux/x86_64/__start_context.S b/sysdeps/unix/sysv/linux/x86_64/__start_context.S
index f6436dd6bb..ae04203c90 100644
--- a/sysdeps/unix/sysv/linux/x86_64/__start_context.S
+++ b/sysdeps/unix/sysv/linux/x86_64/__start_context.S
@@ -24,20 +24,14 @@
/* Use CALL to push __start_context onto the new stack as well as the new
shadow stack. RDI points to ucontext:
Incoming:
- __ssp[0]: The original caller's shadow stack pointer.
- __ssp[1]: The size of the new shadow stack.
- __ssp[2]: The size of the new shadow stack.
- Outgoing:
__ssp[0]: The new shadow stack pointer.
__ssp[1]: The base address of the new shadow stack.
__ssp[2]: The size of the new shadow stack.
*/
ENTRY(__push___start_context)
- /* Save the pointer to ucontext. */
- movq %rdi, %r9
/* Get the original shadow stack pointer. */
- rdsspq %r8
+ rdsspq %rcx
/* Save the original stack pointer. */
movq %rsp, %rdx
/* Load the top of the new stack into RSI. */
@@ -45,24 +39,12 @@ ENTRY(__push___start_context)
/* Add 8 bytes to RSI since CALL will push the 8-byte return
address onto stack. */
leaq 8(%rsi), %rsp
- /* Allocate the new shadow stack. The size of the new shadow
- stack is passed in __ssp[1]. */
- lea (oSSP + 8)(%rdi), %RSI_LP
- movl $ARCH_CET_ALLOC_SHSTK, %edi
- movl $__NR_arch_prctl, %eax
- /* The new shadow stack base is returned in __ssp[1]. */
- syscall
- testq %rax, %rax
- jne L(hlt) /* This should never happen. */
-
- /* Get the size of the new shadow stack. */
- movq 8(%rsi), %rdi
-
- /* Get the base address of the new shadow stack. */
- movq (%rsi), %rsi
-
+ /* The size of the new shadow stack is stored in __ssp[2]. */
+ mov (oSSP + 16)(%rdi), %RSI_LP
+ /* The new shadow stack base is stored in __ssp[1]. */
+ mov (oSSP + 8)(%rdi), %RAX_LP
/* Use the restore stoken to restore the new shadow stack. */
- rstorssp -8(%rsi, %rdi)
+ rstorssp -8(%rax, %rsi)
/* Save the restore token on the original shadow stack. */
saveprevssp
@@ -73,18 +55,12 @@ ENTRY(__push___start_context)
jmp __start_context
1:
- /* Get the new shadow stack pointer. */
- rdsspq %rdi
-
/* Use the restore stoken to restore the original shadow stack. */
- rstorssp -8(%r8)
+ rstorssp -8(%rcx)
/* Save the restore token on the new shadow stack. */
saveprevssp
- /* Store the new shadow stack pointer in __ssp[0]. */
- movq %rdi, oSSP(%r9)
-
/* Restore the original stack. */
mov %rdx, %rsp
ret
diff --git a/sysdeps/unix/sysv/linux/x86_64/getcontext.S b/sysdeps/unix/sysv/linux/x86_64/getcontext.S
index a00e2f6290..71f3802dca 100644
--- a/sysdeps/unix/sysv/linux/x86_64/getcontext.S
+++ b/sysdeps/unix/sysv/linux/x86_64/getcontext.S
@@ -58,35 +58,15 @@ ENTRY(__getcontext)
testl $X86_FEATURE_1_SHSTK, %fs:FEATURE_1_OFFSET
jz L(no_shstk)
- /* Save RDI in RDX which won't be clobbered by syscall. */
- movq %rdi, %rdx
-
xorl %eax, %eax
cmpq %fs:SSP_BASE_OFFSET, %rax
jnz L(shadow_stack_bound_recorded)
- /* Get the base address and size of the default shadow stack
- which must be the current shadow stack since nothing has
- been recorded yet. */
- sub $24, %RSP_LP
- mov %RSP_LP, %RSI_LP
- movl $ARCH_CET_STATUS, %edi
- movl $__NR_arch_prctl, %eax
- syscall
- testq %rax, %rax
- jz L(continue_no_err)
-
- /* This should never happen. */
- hlt
-
-L(continue_no_err):
- /* Record the base of the current shadow stack. */
- movq 8(%rsp), %rax
+ /* When the shadow stack base is unset, the default shadow
+ stack is in use. Use the current shadow stack pointer
+ as the marker for the default shadow stack. */
+ rdsspq %rax
movq %rax, %fs:SSP_BASE_OFFSET
- add $24, %RSP_LP
-
- /* Restore RDI. */
- movq %rdx, %rdi
L(shadow_stack_bound_recorded):
/* Get the current shadow stack pointer. */
@@ -94,7 +74,7 @@ L(shadow_stack_bound_recorded):
/* NB: Save the caller's shadow stack so that we can jump back
to the caller directly. */
addq $8, %rax
- movq %rax, oSSP(%rdx)
+ movq %rax, oSSP(%rdi)
/* Save the current shadow stack base in ucontext. */
movq %fs:SSP_BASE_OFFSET, %rax
diff --git a/sysdeps/unix/sysv/linux/x86_64/makecontext.c b/sysdeps/unix/sysv/linux/x86_64/makecontext.c
index de9e03eb81..788b730132 100644
--- a/sysdeps/unix/sysv/linux/x86_64/makecontext.c
+++ b/sysdeps/unix/sysv/linux/x86_64/makecontext.c
@@ -24,6 +24,8 @@
# include <pthread.h>
# include <libc-pointer-arith.h>
# include <sys/prctl.h>
+# include <sys/mman.h>
+# include <allocate-shadow-stack.h>
#endif
#include "ucontext_i.h"
@@ -88,23 +90,24 @@ __makecontext (ucontext_t *ucp, void (*func) (void), int argc, ...)
if ((feature_1 & X86_FEATURE_1_SHSTK) != 0)
{
/* Shadow stack is enabled. We need to allocate a new shadow
- stack. */
- unsigned long ssp_size = (((uintptr_t) sp
- - (uintptr_t) ucp->uc_stack.ss_sp)
- >> STACK_SIZE_TO_SHADOW_STACK_SIZE_SHIFT);
- /* Align shadow stack to 8 bytes. */
- ssp_size = ALIGN_UP (ssp_size, 8);
-
- ucp->__ssp[1] = ssp_size;
- ucp->__ssp[2] = ssp_size;
-
- /* Call __push___start_context to allocate a new shadow stack,
- push __start_context onto the new stack as well as the new
- shadow stack. NB: After __push___start_context returns,
+ stack. NB:
ucp->__ssp[0]: The new shadow stack pointer.
ucp->__ssp[1]: The base address of the new shadow stack.
ucp->__ssp[2]: The size of the new shadow stack.
*/
+ long int ret
+ = __allocate_shadow_stack (((uintptr_t) sp
+ - (uintptr_t) ucp->uc_stack.ss_sp),
+ &ucp->__ssp[1]);
+ if (ret != 0)
+ {
+ /* FIXME: What should we do? */
+ abort ();
+ }
+
+ ucp->__ssp[0] = ucp->__ssp[1] + ucp->__ssp[2] - 8;
+ /* Call __push___start_context to push __start_context onto the new
+ stack as well as the new shadow stack. */
__push___start_context (ucp);
}
else
diff --git a/sysdeps/unix/sysv/linux/x86_64/swapcontext.S b/sysdeps/unix/sysv/linux/x86_64/swapcontext.S
index 5925752164..2f2fe9875b 100644
--- a/sysdeps/unix/sysv/linux/x86_64/swapcontext.S
+++ b/sysdeps/unix/sysv/linux/x86_64/swapcontext.S
@@ -109,25 +109,11 @@ ENTRY(__swapcontext)
cmpq %fs:SSP_BASE_OFFSET, %rax
jnz L(shadow_stack_bound_recorded)
- /* Get the base address and size of the default shadow stack
- which must be the current shadow stack since nothing has
- been recorded yet. */
- sub $24, %RSP_LP
- mov %RSP_LP, %RSI_LP
- movl $ARCH_CET_STATUS, %edi
- movl $__NR_arch_prctl, %eax
- syscall
- testq %rax, %rax
- jz L(continue_no_err)
-
- /* This should never happen. */
- hlt
-
-L(continue_no_err):
- /* Record the base of the current shadow stack. */
- movq 8(%rsp), %rax
+ /* When the shadow stack base is unset, the default shadow
+ stack is in use. Use the current shadow stack pointer
+ as the marker for the default shadow stack. */
+ rdsspq %rax
movq %rax, %fs:SSP_BASE_OFFSET
- add $24, %RSP_LP
L(shadow_stack_bound_recorded):
/* If we unwind the stack, we can't undo stack unwinding. Just
diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index 0bf923d48b..f180f0d9a4 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -1121,8 +1121,9 @@ no_cpuid:
# ifndef SHARED
/* Check if IBT and SHSTK are enabled by kernel. */
- if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT)
- || (cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
+ if ((cet_status
+ & (GNU_PROPERTY_X86_FEATURE_1_IBT
+ | GNU_PROPERTY_X86_FEATURE_1_SHSTK)))
{
/* Disable IBT and/or SHSTK if they are enabled by kernel, but
disabled by environment variable:
@@ -1131,9 +1132,11 @@ no_cpuid:
*/
unsigned int cet_feature = 0;
if (!CPU_FEATURE_USABLE (IBT))
- cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT;
+ cet_feature |= (cet_status
+ & GNU_PROPERTY_X86_FEATURE_1_IBT);
if (!CPU_FEATURE_USABLE (SHSTK))
- cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+ cet_feature |= (cet_status
+ & GNU_PROPERTY_X86_FEATURE_1_SHSTK);
if (cet_feature)
{
@@ -1148,7 +1151,9 @@ no_cpuid:
lock CET if IBT or SHSTK is enabled permissively. */
if (GL(dl_x86_feature_control).ibt != cet_permissive
&& GL(dl_x86_feature_control).shstk != cet_permissive)
- dl_cet_lock_cet ();
+ dl_cet_lock_cet (GL(dl_x86_feature_1)
+ & (GNU_PROPERTY_X86_FEATURE_1_IBT
+ | GNU_PROPERTY_X86_FEATURE_1_SHSTK));
}
# endif
}
diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
index 67c51ee8c2..8b911fd931 100644
--- a/sysdeps/x86/dl-cet.c
+++ b/sysdeps/x86/dl-cet.c
@@ -201,7 +201,7 @@ dl_cet_check_startup (struct link_map *m, struct dl_cet_info *info)
feature_1_lock |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
if (feature_1_lock != 0
- && dl_cet_lock_cet () != 0)
+ && dl_cet_lock_cet (feature_1_lock) != 0)
_dl_fatal_printf ("%s: can't lock CET\n", info->program);
}
diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h
index 1403f939f7..4bcc2552a1 100644
--- a/sysdeps/x86_64/nptl/tls.h
+++ b/sysdeps/x86_64/nptl/tls.h
@@ -60,7 +60,7 @@ typedef struct
void *__private_tm[4];
/* GCC split stack support. */
void *__private_ss;
- /* The lowest address of shadow stack, */
+ /* The marker for the current shadow stack. */
unsigned long long int ssp_base;
/* Must be kept even if it is no longer used by glibc since programs,
like AddressSanitizer, depend on the size of tcbhead_t. */
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-06 17:20 ` [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface H.J. Lu
@ 2023-12-11 11:34 ` Szabolcs Nagy
2023-12-11 16:44 ` H.J. Lu
0 siblings, 1 reply; 42+ messages in thread
From: Szabolcs Nagy @ 2023-12-11 11:34 UTC (permalink / raw)
To: H.J. Lu, libc-alpha; +Cc: rick.p.edgecombe
The 12/06/2023 09:20, H.J. Lu wrote:
> Sync with Linux kernel 6.6 shadow stack interface. Since only x86-64 is
> supported, i386 shadow stack codes are unchanged and CET shouldn't be
> enabled for i386.
>
> 1. When the shadow stack base in TCB is unset, the default shadow stack
> is in use. Use the current shadow stack pointer as the marker for the
> default shadow stack.
what is the role of ssp_base in the tcb?
> 2. Allocate shadow stack with the map_shadow_stack syscall.
> 3. Rename arch_prctl CET commands to ARCH_SHSTK_XXX.
> 4. Rewrite the CET control functions with the current kernel shadow stack
> interface.
>
> Since CET is no longer enabled by kernel, a separate patch will enable
> shadow stack during startup.
...
> +/* NB: This can be treated as a syscall by caller. */
> +
> +#ifndef __x86_64__
> +__attribute__ ((regparm (2)))
> +#endif
> +long int
> +__allocate_shadow_stack (size_t stack_size,
> + shadow_stack_size_t *child_stack)
> +{
> +#ifdef __NR_map_shadow_stack
> + size_t shadow_stack_size
> + = stack_size >> STACK_SIZE_TO_SHADOW_STACK_SIZE_SHIFT;
> + /* Align shadow stack to 8 bytes. */
> + shadow_stack_size = ALIGN_UP (shadow_stack_size, 8);
since sigaltstack shares shadow stack with the current context
in the thread, this should include the shadow stack requirement
for signal handlers too. otherwise a user can use makecontext
to fill up a shadow stack such that a stack overflow signal
handler cannot run (presumably a crash handler should be able
to handle crashes in all situations).
i think a fixed fixed size is enough to cover for reasonable
signal handler (e.g. ~20 stack frames).
> + void *shadow_stack = (void *)INLINE_SYSCALL_CALL
> + (map_shadow_stack, NULL, shadow_stack_size, SHADOW_STACK_SET_TOKEN);
> + /* Report the map_shadow_stack error. */
> + if (shadow_stack == MAP_FAILED)
> + return -errno;
> +
> + /* Save the shadow stack base and size on child stack. */
> + child_stack[0] = (uintptr_t) shadow_stack;
> + child_stack[1] = shadow_stack_size;
> +
> + return 0;
> +#else
> + return -ENOSYS;
> +#endif
> +}
...
> --- a/sysdeps/unix/sysv/linux/x86_64/makecontext.c
> +++ b/sysdeps/unix/sysv/linux/x86_64/makecontext.c
> @@ -24,6 +24,8 @@
> # include <pthread.h>
> # include <libc-pointer-arith.h>
> # include <sys/prctl.h>
> +# include <sys/mman.h>
> +# include <allocate-shadow-stack.h>
> #endif
>
> #include "ucontext_i.h"
> @@ -88,23 +90,24 @@ __makecontext (ucontext_t *ucp, void (*func) (void), int argc, ...)
> if ((feature_1 & X86_FEATURE_1_SHSTK) != 0)
> {
> /* Shadow stack is enabled. We need to allocate a new shadow
> - stack. */
> - unsigned long ssp_size = (((uintptr_t) sp
> - - (uintptr_t) ucp->uc_stack.ss_sp)
> - >> STACK_SIZE_TO_SHADOW_STACK_SIZE_SHIFT);
> - /* Align shadow stack to 8 bytes. */
> - ssp_size = ALIGN_UP (ssp_size, 8);
> -
> - ucp->__ssp[1] = ssp_size;
> - ucp->__ssp[2] = ssp_size;
> -
> - /* Call __push___start_context to allocate a new shadow stack,
> - push __start_context onto the new stack as well as the new
> - shadow stack. NB: After __push___start_context returns,
> + stack. NB:
> ucp->__ssp[0]: The new shadow stack pointer.
> ucp->__ssp[1]: The base address of the new shadow stack.
> ucp->__ssp[2]: The size of the new shadow stack.
> */
> + long int ret
> + = __allocate_shadow_stack (((uintptr_t) sp
> + - (uintptr_t) ucp->uc_stack.ss_sp),
> + &ucp->__ssp[1]);
this allocation in glibc does not seem to be freed in glibc.
so normal makecontext use will leak the shadow stack.
(e.g. this is true even if the makecontext function returns
and thus the shadow stack lifetime ends).
> + if (ret != 0)
> + {
> + /* FIXME: What should we do? */
> + abort ();
makecontext cannot report errors, so we can make this an
error in setcontext/swapcontext (not sure if that's
better in practice, but it is more compatible with the
posix api).
> + }
> +
> + ucp->__ssp[0] = ucp->__ssp[1] + ucp->__ssp[2] - 8;
> + /* Call __push___start_context to push __start_context onto the new
> + stack as well as the new shadow stack. */
> __push___start_context (ucp);
...
> --- a/sysdeps/x86_64/nptl/tls.h
> +++ b/sysdeps/x86_64/nptl/tls.h
> @@ -60,7 +60,7 @@ typedef struct
> void *__private_tm[4];
> /* GCC split stack support. */
> void *__private_ss;
> - /* The lowest address of shadow stack, */
> + /* The marker for the current shadow stack. */
> unsigned long long int ssp_base;
is this abi between libc/unwinder or compiler or something else?
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-11 11:34 ` Szabolcs Nagy
@ 2023-12-11 16:44 ` H.J. Lu
2023-12-12 18:02 ` Szabolcs Nagy
0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-11 16:44 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, rick.p.edgecombe
On Mon, Dec 11, 2023 at 3:34 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 12/06/2023 09:20, H.J. Lu wrote:
> > Sync with Linux kernel 6.6 shadow stack interface. Since only x86-64 is
> > supported, i386 shadow stack codes are unchanged and CET shouldn't be
> > enabled for i386.
> >
> > 1. When the shadow stack base in TCB is unset, the default shadow stack
> > is in use. Use the current shadow stack pointer as the marker for the
> > default shadow stack.
>
> what is the role of ssp_base in the tcb?
It is used to identify if the current stack is the same as the target
shadow stack when switching ucontexts. If yes, INCSSP will
be used to unwind shadow stack. Otherwise, shadow stack
restore token will be used.
> > 2. Allocate shadow stack with the map_shadow_stack syscall.
> > 3. Rename arch_prctl CET commands to ARCH_SHSTK_XXX.
> > 4. Rewrite the CET control functions with the current kernel shadow stack
> > interface.
> >
> > Since CET is no longer enabled by kernel, a separate patch will enable
> > shadow stack during startup.
> ...
> > +/* NB: This can be treated as a syscall by caller. */
> > +
> > +#ifndef __x86_64__
> > +__attribute__ ((regparm (2)))
> > +#endif
> > +long int
> > +__allocate_shadow_stack (size_t stack_size,
> > + shadow_stack_size_t *child_stack)
> > +{
> > +#ifdef __NR_map_shadow_stack
> > + size_t shadow_stack_size
> > + = stack_size >> STACK_SIZE_TO_SHADOW_STACK_SIZE_SHIFT;
> > + /* Align shadow stack to 8 bytes. */
> > + shadow_stack_size = ALIGN_UP (shadow_stack_size, 8);
>
> since sigaltstack shares shadow stack with the current context
> in the thread, this should include the shadow stack requirement
> for signal handlers too. otherwise a user can use makecontext
> to fill up a shadow stack such that a stack overflow signal
> handler cannot run (presumably a crash handler should be able
> to handle crashes in all situations).
>
> i think a fixed fixed size is enough to cover for reasonable
> signal handler (e.g. ~20 stack frames).
It sounds reasonable.
>
> > + void *shadow_stack = (void *)INLINE_SYSCALL_CALL
> > + (map_shadow_stack, NULL, shadow_stack_size, SHADOW_STACK_SET_TOKEN);
> > + /* Report the map_shadow_stack error. */
> > + if (shadow_stack == MAP_FAILED)
> > + return -errno;
> > +
> > + /* Save the shadow stack base and size on child stack. */
> > + child_stack[0] = (uintptr_t) shadow_stack;
> > + child_stack[1] = shadow_stack_size;
> > +
> > + return 0;
> > +#else
> > + return -ENOSYS;
> > +#endif
> > +}
> ...
> > --- a/sysdeps/unix/sysv/linux/x86_64/makecontext.c
> > +++ b/sysdeps/unix/sysv/linux/x86_64/makecontext.c
> > @@ -24,6 +24,8 @@
> > # include <pthread.h>
> > # include <libc-pointer-arith.h>
> > # include <sys/prctl.h>
> > +# include <sys/mman.h>
> > +# include <allocate-shadow-stack.h>
> > #endif
> >
> > #include "ucontext_i.h"
> > @@ -88,23 +90,24 @@ __makecontext (ucontext_t *ucp, void (*func) (void), int argc, ...)
> > if ((feature_1 & X86_FEATURE_1_SHSTK) != 0)
> > {
> > /* Shadow stack is enabled. We need to allocate a new shadow
> > - stack. */
> > - unsigned long ssp_size = (((uintptr_t) sp
> > - - (uintptr_t) ucp->uc_stack.ss_sp)
> > - >> STACK_SIZE_TO_SHADOW_STACK_SIZE_SHIFT);
> > - /* Align shadow stack to 8 bytes. */
> > - ssp_size = ALIGN_UP (ssp_size, 8);
> > -
> > - ucp->__ssp[1] = ssp_size;
> > - ucp->__ssp[2] = ssp_size;
> > -
> > - /* Call __push___start_context to allocate a new shadow stack,
> > - push __start_context onto the new stack as well as the new
> > - shadow stack. NB: After __push___start_context returns,
> > + stack. NB:
> > ucp->__ssp[0]: The new shadow stack pointer.
> > ucp->__ssp[1]: The base address of the new shadow stack.
> > ucp->__ssp[2]: The size of the new shadow stack.
> > */
> > + long int ret
> > + = __allocate_shadow_stack (((uintptr_t) sp
> > + - (uintptr_t) ucp->uc_stack.ss_sp),
> > + &ucp->__ssp[1]);
>
> this allocation in glibc does not seem to be freed in glibc.
>
> so normal makecontext use will leak the shadow stack.
>
> (e.g. this is true even if the makecontext function returns
> and thus the shadow stack lifetime ends).
Correct. Since there is no function to explicitly release ucontext,
there is no place to release shadow stack and shadow stack will
be leaked.
> > + if (ret != 0)
> > + {
> > + /* FIXME: What should we do? */
> > + abort ();
>
> makecontext cannot report errors, so we can make this an
> error in setcontext/swapcontext (not sure if that's
> better in practice, but it is more compatible with the
> posix api).
>
> > + }
> > +
> > + ucp->__ssp[0] = ucp->__ssp[1] + ucp->__ssp[2] - 8;
> > + /* Call __push___start_context to push __start_context onto the new
> > + stack as well as the new shadow stack. */
> > __push___start_context (ucp);
> ...
> > --- a/sysdeps/x86_64/nptl/tls.h
> > +++ b/sysdeps/x86_64/nptl/tls.h
> > @@ -60,7 +60,7 @@ typedef struct
> > void *__private_tm[4];
> > /* GCC split stack support. */
> > void *__private_ss;
> > - /* The lowest address of shadow stack, */
> > + /* The marker for the current shadow stack. */
> > unsigned long long int ssp_base;
>
> is this abi between libc/unwinder or compiler or something else?
This is used by ucontext functions internally in glibc.
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-11 16:44 ` H.J. Lu
@ 2023-12-12 18:02 ` Szabolcs Nagy
2023-12-12 18:39 ` H.J. Lu
0 siblings, 1 reply; 42+ messages in thread
From: Szabolcs Nagy @ 2023-12-12 18:02 UTC (permalink / raw)
To: H.J. Lu; +Cc: libc-alpha, rick.p.edgecombe
The 12/11/2023 08:44, H.J. Lu wrote:
> On Mon, Dec 11, 2023 at 3:34 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > The 12/06/2023 09:20, H.J. Lu wrote:
> > > Sync with Linux kernel 6.6 shadow stack interface. Since only x86-64 is
> > > supported, i386 shadow stack codes are unchanged and CET shouldn't be
> > > enabled for i386.
> > >
> > > 1. When the shadow stack base in TCB is unset, the default shadow stack
> > > is in use. Use the current shadow stack pointer as the marker for the
> > > default shadow stack.
> >
> > what is the role of ssp_base in the tcb?
>
> It is used to identify if the current stack is the same as the target
> shadow stack when switching ucontexts. If yes, INCSSP will
> be used to unwind shadow stack. Otherwise, shadow stack
> restore token will be used.
i would like to support stack switching in longjmp too
(for aarch64 gcs) not just in setcontext/swapcontext.
if you assume that the target shadow stack always ends in a
restore token when a jump is switching stack, then scanning
the shadow stack until the token or current ssp is found works.
(tcb ssp_base is not needed.)
the linear scanning affects longjmp performance but it seems
the overhead is amortized by the creation of the stack frames.
if you don't want this in longjmp, then code using it for task
scheduling across worker threads have to be patched to use
*context (means signal mask saving syscall overhead so some
projects may not like this) or marked as non-shadow-stack compat.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-12 18:02 ` Szabolcs Nagy
@ 2023-12-12 18:39 ` H.J. Lu
2023-12-13 10:48 ` Szabolcs Nagy
0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-12 18:39 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, rick.p.edgecombe
On Tue, Dec 12, 2023 at 10:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 12/11/2023 08:44, H.J. Lu wrote:
> > On Mon, Dec 11, 2023 at 3:34 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > The 12/06/2023 09:20, H.J. Lu wrote:
> > > > Sync with Linux kernel 6.6 shadow stack interface. Since only x86-64 is
> > > > supported, i386 shadow stack codes are unchanged and CET shouldn't be
> > > > enabled for i386.
> > > >
> > > > 1. When the shadow stack base in TCB is unset, the default shadow stack
> > > > is in use. Use the current shadow stack pointer as the marker for the
> > > > default shadow stack.
> > >
> > > what is the role of ssp_base in the tcb?
> >
> > It is used to identify if the current stack is the same as the target
> > shadow stack when switching ucontexts. If yes, INCSSP will
> > be used to unwind shadow stack. Otherwise, shadow stack
> > restore token will be used.
>
> i would like to support stack switching in longjmp too
> (for aarch64 gcs) not just in setcontext/swapcontext.
>
> if you assume that the target shadow stack always ends in a
> restore token when a jump is switching stack, then scanning
> the shadow stack until the token or current ssp is found works.
> (tcb ssp_base is not needed.)
We can't put a restore token on shadow stack for setjmp since
longjmp is optional, not required. If longjmp isn't called, there
will be an extra restore token on shadow stack. One way to
switch arbitrary shadow stack is to allow write shadow stack,
which will reduce security.
> the linear scanning affects longjmp performance but it seems
> the overhead is amortized by the creation of the stack frames.
>
> if you don't want this in longjmp, then code using it for task
> scheduling across worker threads have to be patched to use
> *context (means signal mask saving syscall overhead so some
> projects may not like this) or marked as non-shadow-stack compat.
>
--
H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-12 18:39 ` H.J. Lu
@ 2023-12-13 10:48 ` Szabolcs Nagy
2023-12-13 22:45 ` H.J. Lu
0 siblings, 1 reply; 42+ messages in thread
From: Szabolcs Nagy @ 2023-12-13 10:48 UTC (permalink / raw)
To: H.J. Lu; +Cc: libc-alpha, rick.p.edgecombe
The 12/12/2023 10:39, H.J. Lu wrote:
> On Tue, Dec 12, 2023 at 10:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 12/11/2023 08:44, H.J. Lu wrote:
> > > On Mon, Dec 11, 2023 at 3:34 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > The 12/06/2023 09:20, H.J. Lu wrote:
> > > > > Sync with Linux kernel 6.6 shadow stack interface. Since only x86-64 is
> > > > > supported, i386 shadow stack codes are unchanged and CET shouldn't be
> > > > > enabled for i386.
> > > > >
> > > > > 1. When the shadow stack base in TCB is unset, the default shadow stack
> > > > > is in use. Use the current shadow stack pointer as the marker for the
> > > > > default shadow stack.
> > > >
> > > > what is the role of ssp_base in the tcb?
> > >
> > > It is used to identify if the current stack is the same as the target
> > > shadow stack when switching ucontexts. If yes, INCSSP will
> > > be used to unwind shadow stack. Otherwise, shadow stack
> > > restore token will be used.
> >
> > i would like to support stack switching in longjmp too
> > (for aarch64 gcs) not just in setcontext/swapcontext.
> >
> > if you assume that the target shadow stack always ends in a
> > restore token when a jump is switching stack, then scanning
> > the shadow stack until the token or current ssp is found works.
> > (tcb ssp_base is not needed.)
>
> We can't put a restore token on shadow stack for setjmp since
> longjmp is optional, not required. If longjmp isn't called, there
> will be an extra restore token on shadow stack. One way to
> switch arbitrary shadow stack is to allow write shadow stack,
> which will reduce security.
>
setjmp never switches stacks, so it must not place a token.
only stack switching operations need to place tokens.
longjmp cannot target arbitrary setjmp that happened on another
stack: if that stack is still in use by another thread then the
two threads clobber each other's stack. it can only work if that
stack is switched away from and at that point a token was placed.
we can make this the abi: you can only longjmp to a stack which
is switched away from such that a token is placed on it.
in terms of conformance i belive we only need the incssp case,
anything else is qoi extension, but such extensions are used in
practice, e.g. for longjmp between stacks created by makecontext.
> > the linear scanning affects longjmp performance but it seems
> > the overhead is amortized by the creation of the stack frames.
> >
> > if you don't want this in longjmp, then code using it for task
> > scheduling across worker threads have to be patched to use
> > *context (means signal mask saving syscall overhead so some
> > projects may not like this) or marked as non-shadow-stack compat.
> >
>
>
> --
> H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-13 10:48 ` Szabolcs Nagy
@ 2023-12-13 22:45 ` H.J. Lu
2023-12-13 23:54 ` Edgecombe, Rick P
0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-13 22:45 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, rick.p.edgecombe
On Wed, Dec 13, 2023 at 2:49 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 12/12/2023 10:39, H.J. Lu wrote:
> > On Tue, Dec 12, 2023 at 10:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > >
> > > The 12/11/2023 08:44, H.J. Lu wrote:
> > > > On Mon, Dec 11, 2023 at 3:34 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > The 12/06/2023 09:20, H.J. Lu wrote:
> > > > > > Sync with Linux kernel 6.6 shadow stack interface. Since only x86-64 is
> > > > > > supported, i386 shadow stack codes are unchanged and CET shouldn't be
> > > > > > enabled for i386.
> > > > > >
> > > > > > 1. When the shadow stack base in TCB is unset, the default shadow stack
> > > > > > is in use. Use the current shadow stack pointer as the marker for the
> > > > > > default shadow stack.
> > > > >
> > > > > what is the role of ssp_base in the tcb?
> > > >
> > > > It is used to identify if the current stack is the same as the target
> > > > shadow stack when switching ucontexts. If yes, INCSSP will
> > > > be used to unwind shadow stack. Otherwise, shadow stack
> > > > restore token will be used.
> > >
> > > i would like to support stack switching in longjmp too
> > > (for aarch64 gcs) not just in setcontext/swapcontext.
> > >
> > > if you assume that the target shadow stack always ends in a
> > > restore token when a jump is switching stack, then scanning
> > > the shadow stack until the token or current ssp is found works.
> > > (tcb ssp_base is not needed.)
> >
> > We can't put a restore token on shadow stack for setjmp since
> > longjmp is optional, not required. If longjmp isn't called, there
> > will be an extra restore token on shadow stack. One way to
> > switch arbitrary shadow stack is to allow write shadow stack,
> > which will reduce security.
> >
>
> setjmp never switches stacks, so it must not place a token.
>
> only stack switching operations need to place tokens.
>
> longjmp cannot target arbitrary setjmp that happened on another
> stack: if that stack is still in use by another thread then the
> two threads clobber each other's stack. it can only work if that
> stack is switched away from and at that point a token was placed.
> we can make this the abi: you can only longjmp to a stack which
> is switched away from such that a token is placed on it.
The restore token is put on shadow stack by setcontext and
swapcontext. When longjmp is called, it doesn't know if there
is a token or not. If longjmp keeps searching for the token and
doesn't find it, it will run out of shadow stack. Rick, can we
assume that the shadow stack entries are 0, if they aren't in
use?
> in terms of conformance i belive we only need the incssp case,
> anything else is qoi extension, but such extensions are used in
> practice, e.g. for longjmp between stacks created by makecontext.
>
> > > the linear scanning affects longjmp performance but it seems
> > > the overhead is amortized by the creation of the stack frames.
> > >
> > > if you don't want this in longjmp, then code using it for task
> > > scheduling across worker threads have to be patched to use
> > > *context (means signal mask saving syscall overhead so some
> > > projects may not like this) or marked as non-shadow-stack compat.
> > >
> >
> >
> > --
> > H.J.
--
H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-13 22:45 ` H.J. Lu
@ 2023-12-13 23:54 ` Edgecombe, Rick P
2023-12-14 0:20 ` H.J. Lu
0 siblings, 1 reply; 42+ messages in thread
From: Edgecombe, Rick P @ 2023-12-13 23:54 UTC (permalink / raw)
To: szabolcs.nagy, hjl.tools; +Cc: libc-alpha
On Wed, 2023-12-13 at 14:45 -0800, H.J. Lu wrote:
> > longjmp cannot target arbitrary setjmp that happened on another
> > stack: if that stack is still in use by another thread then the
> > two threads clobber each other's stack. it can only work if that
> > stack is switched away from and at that point a token was placed.
> > we can make this the abi: you can only longjmp to a stack which
> > is switched away from such that a token is placed on it.
>
> The restore token is put on shadow stack by setcontext and
> swapcontext. When longjmp is called, it doesn't know if there
> is a token or not. If longjmp keeps searching for the token and
> doesn't find it, it will run out of shadow stack. Rick, can we
> assume that the shadow stack entries are 0, if they aren't in
> use?
Not sure what you mean by entries not in use. You mean the restore
token? The shadow stack entries are not zeroed as they are popped by
RET.
longjmp() doesn't return an error code, so is a crash actually ok here?
I'm remembering another issue that came up when this idea was discussed
before. Apps might not call SAVEPREVSSP after they RSTORSSP. You can
just just swap away and not leave a token. So setjmp() cannot be
guaranteed to work with custom stack switching code. It has to be one
of the rules, at least for x86.
I need to go dig through the mails, but I thought there were some more
limitations (that could also be rules) that we ran into.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-13 23:54 ` Edgecombe, Rick P
@ 2023-12-14 0:20 ` H.J. Lu
2023-12-14 2:21 ` H.J. Lu
0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-14 0:20 UTC (permalink / raw)
To: Edgecombe, Rick P; +Cc: szabolcs.nagy, libc-alpha
On Wed, Dec 13, 2023 at 3:54 PM Edgecombe, Rick P
<rick.p.edgecombe@intel.com> wrote:
>
> On Wed, 2023-12-13 at 14:45 -0800, H.J. Lu wrote:
> > > longjmp cannot target arbitrary setjmp that happened on another
> > > stack: if that stack is still in use by another thread then the
> > > two threads clobber each other's stack. it can only work if that
> > > stack is switched away from and at that point a token was placed.
> > > we can make this the abi: you can only longjmp to a stack which
> > > is switched away from such that a token is placed on it.
> >
> > The restore token is put on shadow stack by setcontext and
> > swapcontext. When longjmp is called, it doesn't know if there
> > is a token or not. If longjmp keeps searching for the token and
> > doesn't find it, it will run out of shadow stack. Rick, can we
> > assume that the shadow stack entries are 0, if they aren't in
> > use?
>
> Not sure what you mean by entries not in use. You mean the restore
> token? The shadow stack entries are not zeroed as they are popped by
> RET.
>
longjmp needs to know when to stop searching the token since there
won't be one if setcontext and swapcontext aren't called on the shadow
stack where setjmp is called. Will checking for zero shadow stack entry
value work here? It is OK if the value isn't zeroed by RET as long as
zero value can be checked to avoid going beyond the shadow stack boundary.
> longjmp() doesn't return an error code, so is a crash actually ok here?
>
>
> I'm remembering another issue that came up when this idea was discussed
> before. Apps might not call SAVEPREVSSP after they RSTORSSP. You can
> just just swap away and not leave a token. So setjmp() cannot be
> guaranteed to work with custom stack switching code. It has to be one
> of the rules, at least for x86.
>
> I need to go dig through the mails, but I thought there were some more
> limitations (that could also be rules) that we ran into.
--
H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-14 0:20 ` H.J. Lu
@ 2023-12-14 2:21 ` H.J. Lu
2023-12-14 17:13 ` szabolcs.nagy
0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-14 2:21 UTC (permalink / raw)
To: Edgecombe, Rick P; +Cc: szabolcs.nagy, libc-alpha
[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]
On Wed, Dec 13, 2023 at 4:20 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Dec 13, 2023 at 3:54 PM Edgecombe, Rick P
> <rick.p.edgecombe@intel.com> wrote:
> >
> > On Wed, 2023-12-13 at 14:45 -0800, H.J. Lu wrote:
> > > > longjmp cannot target arbitrary setjmp that happened on another
> > > > stack: if that stack is still in use by another thread then the
> > > > two threads clobber each other's stack. it can only work if that
> > > > stack is switched away from and at that point a token was placed.
> > > > we can make this the abi: you can only longjmp to a stack which
> > > > is switched away from such that a token is placed on it.
> > >
> > > The restore token is put on shadow stack by setcontext and
> > > swapcontext. When longjmp is called, it doesn't know if there
> > > is a token or not. If longjmp keeps searching for the token and
> > > doesn't find it, it will run out of shadow stack. Rick, can we
> > > assume that the shadow stack entries are 0, if they aren't in
> > > use?
> >
> > Not sure what you mean by entries not in use. You mean the restore
> > token? The shadow stack entries are not zeroed as they are popped by
> > RET.
> >
>
> longjmp needs to know when to stop searching the token since there
> won't be one if setcontext and swapcontext aren't called on the shadow
> stack where setjmp is called. Will checking for zero shadow stack entry
> value work here? It is OK if the value isn't zeroed by RET as long as
> zero value can be checked to avoid going beyond the shadow stack boundary.
Here are 2 patches:
1. Add a test for longjmp from a user context.
2. Check the restore token in longjmp.
Do they make sense?
> > longjmp() doesn't return an error code, so is a crash actually ok here?
> >
> >
> > I'm remembering another issue that came up when this idea was discussed
> > before. Apps might not call SAVEPREVSSP after they RSTORSSP. You can
> > just just swap away and not leave a token. So setjmp() cannot be
> > guaranteed to work with custom stack switching code. It has to be one
> > of the rules, at least for x86.
> >
> > I need to go dig through the mails, but I thought there were some more
> > limitations (that could also be rules) that we ran into.
>
>
>
> --
> H.J.
--
H.J.
[-- Attachment #2: 0002-x86-64-cet-Check-the-restore-token-in-longjmp.patch --]
[-- Type: text/x-patch, Size: 2137 bytes --]
From ce1045ecfe9ff40fdc8c7c8cf766175229d475b7 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 13 Dec 2023 17:50:47 -0800
Subject: [PATCH 2/2] x86-64/cet: Check the restore token in longjmp
setcontext and swapcontext put a restore token on the old shadow stack
so that they switch to a different shadow stack when switching user
contexts. When longjmp from a user context, the target shadow stack
can be different from the current shadow stack and INCSSP can't be
used to restore the shadow stack pointer to the target shadow stack.
Update longjmp to search for a restore token. If found, use the token
to restore the shadow stack pointer before using INCSSP to pop the
shadow stack. Stop the token search if the shadow stack entry value
is zero.
NB: The token search will segfault if there is no restore token and all
shadow stack entries are non-zero.
---
sysdeps/x86_64/__longjmp.S | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/sysdeps/x86_64/__longjmp.S b/sysdeps/x86_64/__longjmp.S
index 9ac075e0a8..0d37e91af5 100644
--- a/sysdeps/x86_64/__longjmp.S
+++ b/sysdeps/x86_64/__longjmp.S
@@ -64,8 +64,31 @@ ENTRY(__longjmp)
/* Get the current ssp. */
rdsspq %rax
/* And compare it with the saved ssp value. */
- subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
+ movq SHADOW_STACK_POINTER_OFFSET(%rdi), %rcx
+ subq %rcx, %rax
je L(skip_ssp)
+
+L(find_restore_token_loop):
+ /* Look for a restore token. */
+ movq -8(%rcx), %rbx
+ andq $-8, %rbx
+ /* Stop if the shadow stack entry value is zero. */
+ jz L(no_shadow_stack_token)
+ cmpq %rcx, %rbx
+ /* Find the restore token. */
+ je L(restore_shadow_stack)
+
+ /* Try the next slot. */
+ subq $8, %rcx
+ jmp L(find_restore_token_loop)
+
+L(restore_shadow_stack):
+ /* Restore the target shadow stack. */
+ rstorssp -8(%rcx)
+ rdsspq %rax
+ subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
+
+L(no_shadow_stack_token):
/* Count the number of frames to adjust and adjust it
with incssp instruction. The instruction can adjust
the ssp by [0..255] value only thus use a loop if
--
2.43.0
[-- Attachment #3: 0001-Add-a-test-for-longjmp-from-user-context.patch --]
[-- Type: text/x-patch, Size: 3025 bytes --]
From bee9c6265f6eb1754c82464755af265e2587c2c9 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 13 Dec 2023 11:13:16 -0800
Subject: [PATCH 1/2] Add a test for longjmp from user context
Verify that longjmp works correctly after setcontext is called to switch
to a user context.
---
stdlib/Makefile | 1 +
stdlib/tst-setcontext10.c | 87 +++++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+)
create mode 100644 stdlib/tst-setcontext10.c
diff --git a/stdlib/Makefile b/stdlib/Makefile
index 0b154e57c5..8c6249aab4 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -234,6 +234,7 @@ tests := \
tst-setcontext7 \
tst-setcontext8 \
tst-setcontext9 \
+ tst-setcontext10 \
tst-strfmon_l \
tst-strfrom \
tst-strfrom-locale \
diff --git a/stdlib/tst-setcontext10.c b/stdlib/tst-setcontext10.c
new file mode 100644
index 0000000000..a311d9f783
--- /dev/null
+++ b/stdlib/tst-setcontext10.c
@@ -0,0 +1,87 @@
+/* Check longjmp from user context.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <setjmp.h>
+#include <ucontext.h>
+#include <unistd.h>
+
+static jmp_buf jmpbuf;
+static ucontext_t ctx;
+
+static void f2 (void);
+
+static void
+__attribute__ ((noinline, noclone))
+f1 (void)
+{
+ printf ("start f1\n");
+ f2 ();
+}
+
+static void
+__attribute__ ((noinline, noclone))
+f2 (void)
+{
+ printf ("start f2\n");
+ if (setcontext (&ctx) != 0)
+ {
+ printf ("%s: setcontext: %m\n", __FUNCTION__);
+ exit (EXIT_FAILURE);
+ }
+}
+
+static void
+f3 (void)
+{
+ printf ("start f3\n");
+ longjmp (jmpbuf, 1);
+}
+
+static int
+__attribute__ ((noinline, noclone))
+do_test_1 (void)
+{
+ char st1[32768];
+
+ if (setjmp (jmpbuf) != 0)
+ return 0;
+
+ puts ("making contexts");
+ if (getcontext (&ctx) != 0)
+ {
+ printf ("%s: getcontext: %m\n", __FUNCTION__);
+ exit (EXIT_FAILURE);
+ }
+ ctx.uc_stack.ss_sp = st1;
+ ctx.uc_stack.ss_size = sizeof st1;
+ ctx.uc_link = NULL;
+ makecontext (&ctx, (void (*) (void)) f3, 0);
+ f1 ();
+ puts ("FAIL: returned from f1 ()");
+ exit (EXIT_FAILURE);
+}
+
+static int
+do_test (void)
+{
+ return do_test_1 ();
+}
+
+#include <support/test-driver.c>
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-14 2:21 ` H.J. Lu
@ 2023-12-14 17:13 ` szabolcs.nagy
2023-12-14 17:40 ` H.J. Lu
0 siblings, 1 reply; 42+ messages in thread
From: szabolcs.nagy @ 2023-12-14 17:13 UTC (permalink / raw)
To: H.J. Lu, Edgecombe, Rick P; +Cc: libc-alpha
The 12/13/2023 18:21, H.J. Lu wrote:
> On Wed, Dec 13, 2023 at 4:20 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Wed, Dec 13, 2023 at 3:54 PM Edgecombe, Rick P
> > <rick.p.edgecombe@intel.com> wrote:
> > >
> > > On Wed, 2023-12-13 at 14:45 -0800, H.J. Lu wrote:
> > > > > longjmp cannot target arbitrary setjmp that happened on another
> > > > > stack: if that stack is still in use by another thread then the
> > > > > two threads clobber each other's stack. it can only work if that
> > > > > stack is switched away from and at that point a token was placed.
> > > > > we can make this the abi: you can only longjmp to a stack which
> > > > > is switched away from such that a token is placed on it.
> > > >
> > > > The restore token is put on shadow stack by setcontext and
> > > > swapcontext. When longjmp is called, it doesn't know if there
> > > > is a token or not. If longjmp keeps searching for the token and
> > > > doesn't find it, it will run out of shadow stack. Rick, can we
> > > > assume that the shadow stack entries are 0, if they aren't in
> > > > use?
> > >
> > > Not sure what you mean by entries not in use. You mean the restore
> > > token? The shadow stack entries are not zeroed as they are popped by
> > > RET.
> > >
> >
> > longjmp needs to know when to stop searching the token since there
> > won't be one if setcontext and swapcontext aren't called on the shadow
> > stack where setjmp is called. Will checking for zero shadow stack entry
> > value work here? It is OK if the value isn't zeroed by RET as long as
> > zero value can be checked to avoid going beyond the shadow stack boundary.
>
> Here are 2 patches:
>
> 1. Add a test for longjmp from a user context.
> 2. Check the restore token in longjmp.
>
> Do they make sense?
>
> > > longjmp() doesn't return an error code, so is a crash actually ok here?
> > >
> > >
> > > I'm remembering another issue that came up when this idea was discussed
> > > before. Apps might not call SAVEPREVSSP after they RSTORSSP. You can
> > > just just swap away and not leave a token. So setjmp() cannot be
> > > guaranteed to work with custom stack switching code. It has to be one
> > > of the rules, at least for x86.
> > >
> > > I need to go dig through the mails, but I thought there were some more
> > > limitations (that could also be rules) that we ran into.
so the rule can be that if a user wants a stack to be resumable
then a restore token must be placed on that stack when switching
away from it, if the token is not there then longjmp is ub and
may crash. (so no need to check for 0, such longjmp is a user error)
custom shadow stack switch code has to take this into account.
> From ce1045ecfe9ff40fdc8c7c8cf766175229d475b7 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Wed, 13 Dec 2023 17:50:47 -0800
> Subject: [PATCH 2/2] x86-64/cet: Check the restore token in longjmp
>
> setcontext and swapcontext put a restore token on the old shadow stack
> so that they switch to a different shadow stack when switching user
> contexts. When longjmp from a user context, the target shadow stack
> can be different from the current shadow stack and INCSSP can't be
> used to restore the shadow stack pointer to the target shadow stack.
> Update longjmp to search for a restore token. If found, use the token
> to restore the shadow stack pointer before using INCSSP to pop the
> shadow stack. Stop the token search if the shadow stack entry value
> is zero.
>
> NB: The token search will segfault if there is no restore token and all
> shadow stack entries are non-zero.
this is reasonable except for a comment below.
> ---
> sysdeps/x86_64/__longjmp.S | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/x86_64/__longjmp.S b/sysdeps/x86_64/__longjmp.S
> index 9ac075e0a8..0d37e91af5 100644
> --- a/sysdeps/x86_64/__longjmp.S
> +++ b/sysdeps/x86_64/__longjmp.S
> @@ -64,8 +64,31 @@ ENTRY(__longjmp)
> /* Get the current ssp. */
> rdsspq %rax
> /* And compare it with the saved ssp value. */
> - subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> + movq SHADOW_STACK_POINTER_OFFSET(%rdi), %rcx
> + subq %rcx, %rax
> je L(skip_ssp)
> +
> +L(find_restore_token_loop):
> + /* Look for a restore token. */
> + movq -8(%rcx), %rbx
> + andq $-8, %rbx
> + /* Stop if the shadow stack entry value is zero. */
> + jz L(no_shadow_stack_token)
> + cmpq %rcx, %rbx
> + /* Find the restore token. */
> + je L(restore_shadow_stack)
> +
> + /* Try the next slot. */
> + subq $8, %rcx
> + jmp L(find_restore_token_loop)
> +
> +L(restore_shadow_stack):
> + /* Restore the target shadow stack. */
> + rstorssp -8(%rcx)
i think a restore token is needed on the old shadow stack,
so if there was a setjmp on that stack one can resume to it.
(essentially longjmp behaves like setcontext when it is used
for stack switching: it leaves behind a resumable stack)
> + rdsspq %rax
> + subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> +
> +L(no_shadow_stack_token):
> /* Count the number of frames to adjust and adjust it
> with incssp instruction. The instruction can adjust
> the ssp by [0..255] value only thus use a loop if
> --
> 2.43.0
>
> From bee9c6265f6eb1754c82464755af265e2587c2c9 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Wed, 13 Dec 2023 11:13:16 -0800
> Subject: [PATCH 1/2] Add a test for longjmp from user context
>
> Verify that longjmp works correctly after setcontext is called to switch
> to a user context.
> ---
> stdlib/Makefile | 1 +
> stdlib/tst-setcontext10.c | 87 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 88 insertions(+)
> create mode 100644 stdlib/tst-setcontext10.c
>
> diff --git a/stdlib/Makefile b/stdlib/Makefile
> index 0b154e57c5..8c6249aab4 100644
> --- a/stdlib/Makefile
> +++ b/stdlib/Makefile
> @@ -234,6 +234,7 @@ tests := \
> tst-setcontext7 \
> tst-setcontext8 \
> tst-setcontext9 \
> + tst-setcontext10 \
> tst-strfmon_l \
> tst-strfrom \
> tst-strfrom-locale \
> diff --git a/stdlib/tst-setcontext10.c b/stdlib/tst-setcontext10.c
> new file mode 100644
> index 0000000000..a311d9f783
> --- /dev/null
> +++ b/stdlib/tst-setcontext10.c
> @@ -0,0 +1,87 @@
> +/* Check longjmp from user context.
> + Copyright (C) 2023 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <setjmp.h>
> +#include <ucontext.h>
> +#include <unistd.h>
> +
> +static jmp_buf jmpbuf;
> +static ucontext_t ctx;
> +
> +static void f2 (void);
> +
> +static void
> +__attribute__ ((noinline, noclone))
> +f1 (void)
> +{
> + printf ("start f1\n");
> + f2 ();
> +}
> +
> +static void
> +__attribute__ ((noinline, noclone))
> +f2 (void)
> +{
> + printf ("start f2\n");
> + if (setcontext (&ctx) != 0)
> + {
> + printf ("%s: setcontext: %m\n", __FUNCTION__);
> + exit (EXIT_FAILURE);
> + }
> +}
> +
> +static void
> +f3 (void)
> +{
> + printf ("start f3\n");
> + longjmp (jmpbuf, 1);
> +}
i would do a setjmp here before longjmp and jump back to that
point from the main stack to see if longjmp left behind a
resumable stack.
> +
> +static int
> +__attribute__ ((noinline, noclone))
> +do_test_1 (void)
> +{
> + char st1[32768];
> +
> + if (setjmp (jmpbuf) != 0)
> + return 0;
> +
> + puts ("making contexts");
> + if (getcontext (&ctx) != 0)
> + {
> + printf ("%s: getcontext: %m\n", __FUNCTION__);
> + exit (EXIT_FAILURE);
> + }
> + ctx.uc_stack.ss_sp = st1;
> + ctx.uc_stack.ss_size = sizeof st1;
> + ctx.uc_link = NULL;
> + makecontext (&ctx, (void (*) (void)) f3, 0);
> + f1 ();
> + puts ("FAIL: returned from f1 ()");
> + exit (EXIT_FAILURE);
> +}
> +
> +static int
> +do_test (void)
> +{
> + return do_test_1 ();
> +}
> +
> +#include <support/test-driver.c>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-14 17:13 ` szabolcs.nagy
@ 2023-12-14 17:40 ` H.J. Lu
2023-12-14 23:00 ` Edgecombe, Rick P
2023-12-15 9:23 ` szabolcs.nagy
0 siblings, 2 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-14 17:40 UTC (permalink / raw)
To: szabolcs.nagy; +Cc: Edgecombe, Rick P, libc-alpha
On Thu, Dec 14, 2023 at 9:13 AM szabolcs.nagy@arm.com
<szabolcs.nagy@arm.com> wrote:
>
> The 12/13/2023 18:21, H.J. Lu wrote:
> > On Wed, Dec 13, 2023 at 4:20 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Wed, Dec 13, 2023 at 3:54 PM Edgecombe, Rick P
> > > <rick.p.edgecombe@intel.com> wrote:
> > > >
> > > > On Wed, 2023-12-13 at 14:45 -0800, H.J. Lu wrote:
> > > > > > longjmp cannot target arbitrary setjmp that happened on another
> > > > > > stack: if that stack is still in use by another thread then the
> > > > > > two threads clobber each other's stack. it can only work if that
> > > > > > stack is switched away from and at that point a token was placed.
> > > > > > we can make this the abi: you can only longjmp to a stack which
> > > > > > is switched away from such that a token is placed on it.
> > > > >
> > > > > The restore token is put on shadow stack by setcontext and
> > > > > swapcontext. When longjmp is called, it doesn't know if there
> > > > > is a token or not. If longjmp keeps searching for the token and
> > > > > doesn't find it, it will run out of shadow stack. Rick, can we
> > > > > assume that the shadow stack entries are 0, if they aren't in
> > > > > use?
> > > >
> > > > Not sure what you mean by entries not in use. You mean the restore
> > > > token? The shadow stack entries are not zeroed as they are popped by
> > > > RET.
> > > >
> > >
> > > longjmp needs to know when to stop searching the token since there
> > > won't be one if setcontext and swapcontext aren't called on the shadow
> > > stack where setjmp is called. Will checking for zero shadow stack entry
> > > value work here? It is OK if the value isn't zeroed by RET as long as
> > > zero value can be checked to avoid going beyond the shadow stack boundary.
> >
> > Here are 2 patches:
> >
> > 1. Add a test for longjmp from a user context.
> > 2. Check the restore token in longjmp.
> >
> > Do they make sense?
> >
> > > > longjmp() doesn't return an error code, so is a crash actually ok here?
> > > >
> > > >
> > > > I'm remembering another issue that came up when this idea was discussed
> > > > before. Apps might not call SAVEPREVSSP after they RSTORSSP. You can
> > > > just just swap away and not leave a token. So setjmp() cannot be
> > > > guaranteed to work with custom stack switching code. It has to be one
> > > > of the rules, at least for x86.
> > > >
> > > > I need to go dig through the mails, but I thought there were some more
> > > > limitations (that could also be rules) that we ran into.
>
> so the rule can be that if a user wants a stack to be resumable
> then a restore token must be placed on that stack when switching
That is fine.
> away from it, if the token is not there then longjmp is ub and
> may crash. (so no need to check for 0, such longjmp is a user error)
We need to check for 0. Otherwise, all current setjmp tests segfault
since there is no restore token on the target shadow stack.
> custom shadow stack switch code has to take this into account.
>
>
> > From ce1045ecfe9ff40fdc8c7c8cf766175229d475b7 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Wed, 13 Dec 2023 17:50:47 -0800
> > Subject: [PATCH 2/2] x86-64/cet: Check the restore token in longjmp
> >
> > setcontext and swapcontext put a restore token on the old shadow stack
> > so that they switch to a different shadow stack when switching user
> > contexts. When longjmp from a user context, the target shadow stack
> > can be different from the current shadow stack and INCSSP can't be
> > used to restore the shadow stack pointer to the target shadow stack.
> > Update longjmp to search for a restore token. If found, use the token
> > to restore the shadow stack pointer before using INCSSP to pop the
> > shadow stack. Stop the token search if the shadow stack entry value
> > is zero.
> >
> > NB: The token search will segfault if there is no restore token and all
> > shadow stack entries are non-zero.
>
> this is reasonable except for a comment below.
>
> > ---
> > sysdeps/x86_64/__longjmp.S | 25 ++++++++++++++++++++++++-
> > 1 file changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/sysdeps/x86_64/__longjmp.S b/sysdeps/x86_64/__longjmp.S
> > index 9ac075e0a8..0d37e91af5 100644
> > --- a/sysdeps/x86_64/__longjmp.S
> > +++ b/sysdeps/x86_64/__longjmp.S
> > @@ -64,8 +64,31 @@ ENTRY(__longjmp)
> > /* Get the current ssp. */
> > rdsspq %rax
> > /* And compare it with the saved ssp value. */
> > - subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> > + movq SHADOW_STACK_POINTER_OFFSET(%rdi), %rcx
> > + subq %rcx, %rax
> > je L(skip_ssp)
> > +
> > +L(find_restore_token_loop):
> > + /* Look for a restore token. */
> > + movq -8(%rcx), %rbx
> > + andq $-8, %rbx
> > + /* Stop if the shadow stack entry value is zero. */
> > + jz L(no_shadow_stack_token)
> > + cmpq %rcx, %rbx
> > + /* Find the restore token. */
> > + je L(restore_shadow_stack)
> > +
> > + /* Try the next slot. */
> > + subq $8, %rcx
> > + jmp L(find_restore_token_loop)
> > +
> > +L(restore_shadow_stack):
> > + /* Restore the target shadow stack. */
> > + rstorssp -8(%rcx)
>
> i think a restore token is needed on the old shadow stack,
> so if there was a setjmp on that stack one can resume to it.
Who will put a restore token on shadow stack when user context
isn't used? If kernel does it, CALL will override the restore token
when the end of shadow stack is reached.
> (essentially longjmp behaves like setcontext when it is used
> for stack switching: it leaves behind a resumable stack)
>
> > + rdsspq %rax
> > + subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> > +
> > +L(no_shadow_stack_token):
> > /* Count the number of frames to adjust and adjust it
> > with incssp instruction. The instruction can adjust
> > the ssp by [0..255] value only thus use a loop if
> > --
> > 2.43.0
> >
>
> > From bee9c6265f6eb1754c82464755af265e2587c2c9 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Wed, 13 Dec 2023 11:13:16 -0800
> > Subject: [PATCH 1/2] Add a test for longjmp from user context
> >
> > Verify that longjmp works correctly after setcontext is called to switch
> > to a user context.
> > ---
> > stdlib/Makefile | 1 +
> > stdlib/tst-setcontext10.c | 87 +++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 88 insertions(+)
> > create mode 100644 stdlib/tst-setcontext10.c
> >
> > diff --git a/stdlib/Makefile b/stdlib/Makefile
> > index 0b154e57c5..8c6249aab4 100644
> > --- a/stdlib/Makefile
> > +++ b/stdlib/Makefile
> > @@ -234,6 +234,7 @@ tests := \
> > tst-setcontext7 \
> > tst-setcontext8 \
> > tst-setcontext9 \
> > + tst-setcontext10 \
> > tst-strfmon_l \
> > tst-strfrom \
> > tst-strfrom-locale \
> > diff --git a/stdlib/tst-setcontext10.c b/stdlib/tst-setcontext10.c
> > new file mode 100644
> > index 0000000000..a311d9f783
> > --- /dev/null
> > +++ b/stdlib/tst-setcontext10.c
> > @@ -0,0 +1,87 @@
> > +/* Check longjmp from user context.
> > + Copyright (C) 2023 Free Software Foundation, Inc.
> > + This file is part of the GNU C Library.
> > +
> > + The GNU C Library is free software; you can redistribute it and/or
> > + modify it under the terms of the GNU Lesser General Public
> > + License as published by the Free Software Foundation; either
> > + version 2.1 of the License, or (at your option) any later version.
> > +
> > + The GNU C Library is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + Lesser General Public License for more details.
> > +
> > + You should have received a copy of the GNU Lesser General Public
> > + License along with the GNU C Library; if not, see
> > + <https://www.gnu.org/licenses/>. */
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <setjmp.h>
> > +#include <ucontext.h>
> > +#include <unistd.h>
> > +
> > +static jmp_buf jmpbuf;
> > +static ucontext_t ctx;
> > +
> > +static void f2 (void);
> > +
> > +static void
> > +__attribute__ ((noinline, noclone))
> > +f1 (void)
> > +{
> > + printf ("start f1\n");
> > + f2 ();
> > +}
> > +
> > +static void
> > +__attribute__ ((noinline, noclone))
> > +f2 (void)
> > +{
> > + printf ("start f2\n");
> > + if (setcontext (&ctx) != 0)
> > + {
> > + printf ("%s: setcontext: %m\n", __FUNCTION__);
> > + exit (EXIT_FAILURE);
> > + }
> > +}
> > +
> > +static void
> > +f3 (void)
> > +{
> > + printf ("start f3\n");
> > + longjmp (jmpbuf, 1);
> > +}
>
> i would do a setjmp here before longjmp and jump back to that
> point from the main stack to see if longjmp left behind a
> resumable stack.
This is a different test. I will add it.
> > +
> > +static int
> > +__attribute__ ((noinline, noclone))
> > +do_test_1 (void)
> > +{
> > + char st1[32768];
> > +
> > + if (setjmp (jmpbuf) != 0)
> > + return 0;
> > +
> > + puts ("making contexts");
> > + if (getcontext (&ctx) != 0)
> > + {
> > + printf ("%s: getcontext: %m\n", __FUNCTION__);
> > + exit (EXIT_FAILURE);
> > + }
> > + ctx.uc_stack.ss_sp = st1;
> > + ctx.uc_stack.ss_size = sizeof st1;
> > + ctx.uc_link = NULL;
> > + makecontext (&ctx, (void (*) (void)) f3, 0);
> > + f1 ();
> > + puts ("FAIL: returned from f1 ()");
> > + exit (EXIT_FAILURE);
> > +}
> > +
> > +static int
> > +do_test (void)
> > +{
> > + return do_test_1 ();
> > +}
> > +
> > +#include <support/test-driver.c>
> > --
> > 2.43.0
> >
>
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-14 17:40 ` H.J. Lu
@ 2023-12-14 23:00 ` Edgecombe, Rick P
2023-12-14 23:47 ` H.J. Lu
2023-12-15 9:23 ` szabolcs.nagy
1 sibling, 1 reply; 42+ messages in thread
From: Edgecombe, Rick P @ 2023-12-14 23:00 UTC (permalink / raw)
To: szabolcs.nagy, hjl.tools; +Cc: libc-alpha, x86
+x86 folks
Thread:
https://public-inbox.org/libc-alpha/CAMe9rOoU2Ows-zd=hx1qpf-vMA7q=yoNYnty2NgY5T5zn58bWw@mail.gmail.com/
On Thu, 2023-12-14 at 09:40 -0800, H.J. Lu wrote:
> > so the rule can be that if a user wants a stack to be resumable
> > then a restore token must be placed on that stack when switching
>
> That is fine.
>
> > away from it, if the token is not there then longjmp is ub and
> > may crash. (so no need to check for 0, such longjmp is a user
> > error)
>
> We need to check for 0. Otherwise, all current setjmp tests segfault
> since there is no restore token on the target shadow stack.
A couple thoughts on this.
I guess the problem is: how does longjmp() know if the target ssp
(setjmp() point) is already on the same stack as the current ssp?
Should it INCSSP back to it, or search for a token the other direction?
To determine this, the proposed algorithm is to search up the shadow
stack from the target ssp for a token or zero. If none are found,
incssp back from the current ssp to get the the target ssp.
One problem is, a zero is not guaranteed to be there because the whole
shadow stack could have been used in the past. In the case of no zero
frame to be found, valid longjmp()s on the same shadow stack could
fail. So this fixes some longjmp() between stacks scenarios, but at the
cost of making longjmp() on the same stack unreliable.
You could do the opposite order: search backwards (INCSSP loop) first,
until you get to the begging of the shadow stack stack, then try to
search for the token. But then you needs to be able to find the start
of the stack. A start token would be a bit more unusual to get
overwritten. An app would have to INCSSP back to the start of the
stack, and then overwrite it with CALLs.
Another idea would be to have a RO, non-shadow stack page at the start
of the stack instead of the unmapped guard page. It would serve the
same purpose as the guard page, but could have a non-overwritable zero
value to search for. Since this would be the "zero page", it actually
wouldn't use any extra memory over an unmapped area, since all zero
guard pages could use the same physical memory. This might be doable
today from userspace, with MAP_FIXED, but it could get tricky to clean
it up and need kernel assistance.
But this doesn't solve the problem of longjmp() from an alt shadow
stack. If glibc ends up using WRSS for that, then this all seems overly
complex. I also wonder how much all this searching of the entire stack
will hurt the performance of apps using longjmp() for speed.
What about the rule being: No longjmp() between shadow stacks, unless
you enable WRSS. Glibc could detect WRSS and take advantage whenever
present. We would have to figure out how to do a WRSS elf bit in a way
that makes sense. So most apps don't need to enable WRSS, but the few
ones that longjmp() between stacks can just flip a switch and
get support with similar performance characteristics.
And then a secure-longjmp() mode could be a lockdown mode added later
if there is demand. I don't know, what do you guys think?
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-14 23:00 ` Edgecombe, Rick P
@ 2023-12-14 23:47 ` H.J. Lu
2023-12-15 1:00 ` Edgecombe, Rick P
0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-14 23:47 UTC (permalink / raw)
To: Edgecombe, Rick P; +Cc: szabolcs.nagy, libc-alpha, x86
On Thu, Dec 14, 2023 at 3:01 PM Edgecombe, Rick P
<rick.p.edgecombe@intel.com> wrote:
>
> +x86 folks
>
> Thread:
> https://public-inbox.org/libc-alpha/CAMe9rOoU2Ows-zd=hx1qpf-vMA7q=yoNYnty2NgY5T5zn58bWw@mail.gmail.com/
>
>
> On Thu, 2023-12-14 at 09:40 -0800, H.J. Lu wrote:
> > > so the rule can be that if a user wants a stack to be resumable
> > > then a restore token must be placed on that stack when switching
> >
> > That is fine.
> >
> > > away from it, if the token is not there then longjmp is ub and
> > > may crash. (so no need to check for 0, such longjmp is a user
> > > error)
> >
> > We need to check for 0. Otherwise, all current setjmp tests segfault
> > since there is no restore token on the target shadow stack.
>
> A couple thoughts on this.
>
> I guess the problem is: how does longjmp() know if the target ssp
> (setjmp() point) is already on the same stack as the current ssp?
> Should it INCSSP back to it, or search for a token the other direction?
> To determine this, the proposed algorithm is to search up the shadow
> stack from the target ssp for a token or zero. If none are found,
> incssp back from the current ssp to get the the target ssp.
>
> One problem is, a zero is not guaranteed to be there because the whole
> shadow stack could have been used in the past. In the case of no zero
> frame to be found, valid longjmp()s on the same shadow stack could
> fail. So this fixes some longjmp() between stacks scenarios, but at the
> cost of making longjmp() on the same stack unreliable.
When the whole shadow stack has been used, one more CALL
will also overflow shadow stack. This is the same as longjmp segfault.
> You could do the opposite order: search backwards (INCSSP loop) first,
> until you get to the begging of the shadow stack stack, then try to
> search for the token. But then you needs to be able to find the start
> of the stack. A start token would be a bit more unusual to get
> overwritten. An app would have to INCSSP back to the start of the
> stack, and then overwrite it with CALLs.
>
> Another idea would be to have a RO, non-shadow stack page at the start
> of the stack instead of the unmapped guard page. It would serve the
> same purpose as the guard page, but could have a non-overwritable zero
> value to search for. Since this would be the "zero page", it actually
> wouldn't use any extra memory over an unmapped area, since all zero
> guard pages could use the same physical memory. This might be doable
> today from userspace, with MAP_FIXED, but it could get tricky to clean
> it up and need kernel assistance.
>
> But this doesn't solve the problem of longjmp() from an alt shadow
> stack. If glibc ends up using WRSS for that, then this all seems overly
> complex. I also wonder how much all this searching of the entire stack
> will hurt the performance of apps using longjmp() for speed.
>
> What about the rule being: No longjmp() between shadow stacks, unless
> you enable WRSS. Glibc could detect WRSS and take advantage whenever
> present. We would have to figure out how to do a WRSS elf bit in a way
> that makes sense. So most apps don't need to enable WRSS, but the few
> ones that longjmp() between stacks can just flip a switch and
> get support with similar performance characteristics.
>
> And then a secure-longjmp() mode could be a lockdown mode added later
> if there is demand. I don't know, what do you guys think?
>
--
H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-14 23:47 ` H.J. Lu
@ 2023-12-15 1:00 ` Edgecombe, Rick P
0 siblings, 0 replies; 42+ messages in thread
From: Edgecombe, Rick P @ 2023-12-15 1:00 UTC (permalink / raw)
To: hjl.tools; +Cc: libc-alpha, szabolcs.nagy, x86
On Thu, 2023-12-14 at 15:47 -0800, H.J. Lu wrote:
> > A couple thoughts on this.
> >
> > I guess the problem is: how does longjmp() know if the target ssp
> > (setjmp() point) is already on the same stack as the current ssp?
> > Should it INCSSP back to it, or search for a token the other
> > direction?
> > To determine this, the proposed algorithm is to search up the
> > shadow
> > stack from the target ssp for a token or zero. If none are found,
> > incssp back from the current ssp to get the the target ssp.
> >
> > One problem is, a zero is not guaranteed to be there because the
> > whole
> > shadow stack could have been used in the past. In the case of no
> > zero
> > frame to be found, valid longjmp()s on the same shadow stack could
> > fail. So this fixes some longjmp() between stacks scenarios, but at
> > the
> > cost of making longjmp() on the same stack unreliable.
>
> When the whole shadow stack has been used, one more CALL
> will also overflow shadow stack. This is the same as longjmp
> segfault.
Just chatted with HJ. We discussed and agreed this method is not
reliable. My preference would be to not have a solution that works
halfway. Either the pattern should be not supported (the current
situation), or work deterministically (WRSS, guard gap manipulation
ideas, etc).
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-14 17:40 ` H.J. Lu
2023-12-14 23:00 ` Edgecombe, Rick P
@ 2023-12-15 9:23 ` szabolcs.nagy
2023-12-15 17:08 ` H.J. Lu
1 sibling, 1 reply; 42+ messages in thread
From: szabolcs.nagy @ 2023-12-15 9:23 UTC (permalink / raw)
To: H.J. Lu; +Cc: Edgecombe, Rick P, libc-alpha
The 12/14/2023 09:40, H.J. Lu wrote:
> On Thu, Dec 14, 2023 at 9:13 AM szabolcs.nagy@arm.com
> > so the rule can be that if a user wants a stack to be resumable
> > then a restore token must be placed on that stack when switching
>
> That is fine.
>
> > away from it, if the token is not there then longjmp is ub and
> > may crash. (so no need to check for 0, such longjmp is a user error)
>
> We need to check for 0. Otherwise, all current setjmp tests segfault
> since there is no restore token on the target shadow stack.
i don't understand this claim.
any longjmp target ssp is either
- on the same stack: you can check targetssp == ssp
- or different stack: you can check *targetssp == restore token
as you scan the target ssp.
the second case relies on the described stack switch abi rule.
> > > +L(restore_shadow_stack):
> > > + /* Restore the target shadow stack. */
> > > + rstorssp -8(%rcx)
> >
> > i think a restore token is needed on the old shadow stack,
> > so if there was a setjmp on that stack one can resume to it.
>
> Who will put a restore token on shadow stack when user context
> isn't used? If kernel does it, CALL will override the restore token
> when the end of shadow stack is reached.
anybody who switches away from the stack has to place a token,
this is simply the abi.
if no token is placed that means all the old stack frames are
not available for switching. (e.g. if the kernel does not place
a token on syscall entry, then longjmp is not allowed to jump
to the stack of another thread that happens to be in a syscall,
which is totally reasonable and it's a good thing that we prevent
such broken longjmp usage imo)
you could design a different abi (e.g. one that relies on wrss),
but i don't see the problem with the abi i'm describing.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-15 9:23 ` szabolcs.nagy
@ 2023-12-15 17:08 ` H.J. Lu
2023-12-18 10:53 ` szabolcs.nagy
0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-15 17:08 UTC (permalink / raw)
To: szabolcs.nagy; +Cc: Edgecombe, Rick P, libc-alpha
On Fri, Dec 15, 2023 at 1:23 AM szabolcs.nagy@arm.com
<szabolcs.nagy@arm.com> wrote:
>
> The 12/14/2023 09:40, H.J. Lu wrote:
> > On Thu, Dec 14, 2023 at 9:13 AM szabolcs.nagy@arm.com
> > > so the rule can be that if a user wants a stack to be resumable
> > > then a restore token must be placed on that stack when switching
> >
> > That is fine.
> >
> > > away from it, if the token is not there then longjmp is ub and
> > > may crash. (so no need to check for 0, such longjmp is a user error)
> >
> > We need to check for 0. Otherwise, all current setjmp tests segfault
> > since there is no restore token on the target shadow stack.
>
> i don't understand this claim.
>
> any longjmp target ssp is either
> - on the same stack: you can check targetssp == ssp
> - or different stack: you can check *targetssp == restore token
> as you scan the target ssp.
longjmp doesn't know if the target SSP is on the same shadow stack.
> the second case relies on the described stack switch abi rule.
>
> > > > +L(restore_shadow_stack):
> > > > + /* Restore the target shadow stack. */
> > > > + rstorssp -8(%rcx)
> > >
> > > i think a restore token is needed on the old shadow stack,
> > > so if there was a setjmp on that stack one can resume to it.
> >
> > Who will put a restore token on shadow stack when user context
> > isn't used? If kernel does it, CALL will override the restore token
> > when the end of shadow stack is reached.
>
> anybody who switches away from the stack has to place a token,
> this is simply the abi.
>
> if no token is placed that means all the old stack frames are
> not available for switching. (e.g. if the kernel does not place
> a token on syscall entry, then longjmp is not allowed to jump
> to the stack of another thread that happens to be in a syscall,
> which is totally reasonable and it's a good thing that we prevent
> such broken longjmp usage imo)
>
> you could design a different abi (e.g. one that relies on wrss),
> but i don't see the problem with the abi i'm describing.
>
>
--
H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-15 17:08 ` H.J. Lu
@ 2023-12-18 10:53 ` szabolcs.nagy
2023-12-18 19:06 ` H.J. Lu
0 siblings, 1 reply; 42+ messages in thread
From: szabolcs.nagy @ 2023-12-18 10:53 UTC (permalink / raw)
To: H.J. Lu; +Cc: Edgecombe, Rick P, libc-alpha
The 12/15/2023 09:08, H.J. Lu wrote:
> On Fri, Dec 15, 2023 at 1:23 AM szabolcs.nagy@arm.com
> <szabolcs.nagy@arm.com> wrote:
> >
> > The 12/14/2023 09:40, H.J. Lu wrote:
> > > On Thu, Dec 14, 2023 at 9:13 AM szabolcs.nagy@arm.com
> > > > so the rule can be that if a user wants a stack to be resumable
> > > > then a restore token must be placed on that stack when switching
> > >
> > > That is fine.
> > >
> > > > away from it, if the token is not there then longjmp is ub and
> > > > may crash. (so no need to check for 0, such longjmp is a user error)
> > >
> > > We need to check for 0. Otherwise, all current setjmp tests segfault
> > > since there is no restore token on the target shadow stack.
> >
> > i don't understand this claim.
> >
> > any longjmp target ssp is either
> > - on the same stack: you can check targetssp == ssp
> > - or different stack: you can check *targetssp == restore token
> > as you scan the target ssp.
>
> longjmp doesn't know if the target SSP is on the same shadow stack.
>
it does if it scans.
for (;; targetssp--) {
if (targetssp == ssp) do_samestack();
if (*targetssp == restoretoken) do_differentstack();
}
the only problem i see is if the target shadow stack is
different from the current one and does not end in a restore
token. but i think that is a user error.
if we plan to introduce altshadowstack then this does not
work in case of shadow stack overflow because the overflowed
shadow stack cannot be jumped to even though in practice we
want that to work.
> > the second case relies on the described stack switch abi rule.
> >
> > > > > +L(restore_shadow_stack):
> > > > > + /* Restore the target shadow stack. */
> > > > > + rstorssp -8(%rcx)
> > > >
> > > > i think a restore token is needed on the old shadow stack,
> > > > so if there was a setjmp on that stack one can resume to it.
> > >
> > > Who will put a restore token on shadow stack when user context
> > > isn't used? If kernel does it, CALL will override the restore token
> > > when the end of shadow stack is reached.
> >
> > anybody who switches away from the stack has to place a token,
> > this is simply the abi.
> >
> > if no token is placed that means all the old stack frames are
> > not available for switching. (e.g. if the kernel does not place
> > a token on syscall entry, then longjmp is not allowed to jump
> > to the stack of another thread that happens to be in a syscall,
> > which is totally reasonable and it's a good thing that we prevent
> > such broken longjmp usage imo)
> >
> > you could design a different abi (e.g. one that relies on wrss),
> > but i don't see the problem with the abi i'm describing.
> >
> >
>
>
> --
> H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-18 10:53 ` szabolcs.nagy
@ 2023-12-18 19:06 ` H.J. Lu
2023-12-19 17:15 ` szabolcs.nagy
0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2023-12-18 19:06 UTC (permalink / raw)
To: szabolcs.nagy; +Cc: Edgecombe, Rick P, libc-alpha
[-- Attachment #1: Type: text/plain, Size: 2207 bytes --]
On Mon, Dec 18, 2023 at 2:54 AM szabolcs.nagy@arm.com
<szabolcs.nagy@arm.com> wrote:
>
> The 12/15/2023 09:08, H.J. Lu wrote:
> > On Fri, Dec 15, 2023 at 1:23 AM szabolcs.nagy@arm.com
> > <szabolcs.nagy@arm.com> wrote:
> > >
> > > The 12/14/2023 09:40, H.J. Lu wrote:
> > > > On Thu, Dec 14, 2023 at 9:13 AM szabolcs.nagy@arm.com
> > > > > so the rule can be that if a user wants a stack to be resumable
> > > > > then a restore token must be placed on that stack when switching
> > > >
> > > > That is fine.
> > > >
> > > > > away from it, if the token is not there then longjmp is ub and
> > > > > may crash. (so no need to check for 0, such longjmp is a user error)
> > > >
> > > > We need to check for 0. Otherwise, all current setjmp tests segfault
> > > > since there is no restore token on the target shadow stack.
> > >
> > > i don't understand this claim.
> > >
> > > any longjmp target ssp is either
> > > - on the same stack: you can check targetssp == ssp
> > > - or different stack: you can check *targetssp == restore token
> > > as you scan the target ssp.
> >
> > longjmp doesn't know if the target SSP is on the same shadow stack.
> >
>
> it does if it scans.
>
> for (;; targetssp--) {
> if (targetssp == ssp) do_samestack();
> if (*targetssp == restoretoken) do_differentstack();
> }
>
> the only problem i see is if the target shadow stack is
> different from the current one and does not end in a restore
> token. but i think that is a user error.
Yes, it works. But it is hard to tell its performance overhead.
> if we plan to introduce altshadowstack then this does not
> work in case of shadow stack overflow because the overflowed
> shadow stack cannot be jumped to even though in practice we
> want that to work.
>
I'd like to support shadow stack in glibc 2.39. Since my patch
doesn't enable shadow stack by default, it doesn't have any
functionality impact on users. It allows us to evaluate shadow
stack support in all packages. We may need to use WRUSS
to enable shadow stack for some packages. But users and
developers can't see how shadow stack works if shadow stack
can't be turned on.
--
H.J.
[-- Attachment #2: 0001-x86-64-cet-Check-the-restore-token-in-longjmp.patch --]
[-- Type: text/x-patch, Size: 2301 bytes --]
From 3dba6d876db6a47b66a680bb4aa85b1db63aa3f8 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 13 Dec 2023 17:50:47 -0800
Subject: [PATCH] x86-64/cet: Check the restore token in longjmp
setcontext and swapcontext put a restore token on the old shadow stack
so that they switch to a different shadow stack when switching user
contexts. When longjmp from a user context, the target shadow stack
can be different from the current shadow stack and INCSSP can't be
used to restore the shadow stack pointer to the target shadow stack.
Update longjmp to search for a restore token. If found, use the token
to restore the shadow stack pointer before using INCSSP to pop the
shadow stack. Stop the token search and use INCSSP if the shadow stack
entry value is the same as the current shadow stack pointer.
It is a user error if there is a shadow stack switch without leaving a
restore token on the old shadow stack.
---
sysdeps/x86_64/__longjmp.S | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/sysdeps/x86_64/__longjmp.S b/sysdeps/x86_64/__longjmp.S
index 9ac075e0a8..b106affdcd 100644
--- a/sysdeps/x86_64/__longjmp.S
+++ b/sysdeps/x86_64/__longjmp.S
@@ -63,9 +63,35 @@ ENTRY(__longjmp)
/* Check and adjust the Shadow-Stack-Pointer. */
/* Get the current ssp. */
rdsspq %rax
+ /* Save the current ssp. */
+ movq %rax, %r10
/* And compare it with the saved ssp value. */
- subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
+ movq SHADOW_STACK_POINTER_OFFSET(%rdi), %rcx
+ subq %rcx, %rax
je L(skip_ssp)
+
+L(find_restore_token_loop):
+ /* Look for a restore token. */
+ movq -8(%rcx), %rbx
+ andq $-8, %rbx
+ cmpq %rcx, %rbx
+ /* Find the restore token. */
+ je L(restore_shadow_stack)
+
+ /* Try the next slot. */
+ subq $8, %rcx
+ /* Stop if the current ssp is found. */
+ cmpq %rcx, %r10
+ je L(no_shadow_stack_token)
+ jmp L(find_restore_token_loop)
+
+L(restore_shadow_stack):
+ /* Restore the target shadow stack. */
+ rstorssp -8(%rcx)
+ rdsspq %rax
+ subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
+
+L(no_shadow_stack_token):
/* Count the number of frames to adjust and adjust it
with incssp instruction. The instruction can adjust
the ssp by [0..255] value only thus use a loop if
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-18 19:06 ` H.J. Lu
@ 2023-12-19 17:15 ` szabolcs.nagy
2023-12-19 18:12 ` H.J. Lu
0 siblings, 1 reply; 42+ messages in thread
From: szabolcs.nagy @ 2023-12-19 17:15 UTC (permalink / raw)
To: H.J. Lu; +Cc: Edgecombe, Rick P, libc-alpha
The 12/18/2023 11:06, H.J. Lu wrote:
> On Mon, Dec 18, 2023 at 2:54 AM szabolcs.nagy@arm.com
> <szabolcs.nagy@arm.com> wrote:
> > it does if it scans.
> >
> > for (;; targetssp--) {
> > if (targetssp == ssp) do_samestack();
> > if (*targetssp == restoretoken) do_differentstack();
> > }
> >
> > the only problem i see is if the target shadow stack is
> > different from the current one and does not end in a restore
> > token. but i think that is a user error.
>
> Yes, it works. But it is hard to tell its performance overhead.
>
> > if we plan to introduce altshadowstack then this does not
> > work in case of shadow stack overflow because the overflowed
> > shadow stack cannot be jumped to even though in practice we
> > want that to work.
> >
>
> I'd like to support shadow stack in glibc 2.39. Since my patch
> doesn't enable shadow stack by default, it doesn't have any
> functionality impact on users. It allows us to evaluate shadow
> stack support in all packages. We may need to use WRUSS
> to enable shadow stack for some packages. But users and
> developers can't see how shadow stack works if shadow stack
> can't be turned on.
ok.
> From 3dba6d876db6a47b66a680bb4aa85b1db63aa3f8 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Wed, 13 Dec 2023 17:50:47 -0800
> Subject: [PATCH] x86-64/cet: Check the restore token in longjmp
>
> setcontext and swapcontext put a restore token on the old shadow stack
> so that they switch to a different shadow stack when switching user
> contexts. When longjmp from a user context, the target shadow stack
> can be different from the current shadow stack and INCSSP can't be
> used to restore the shadow stack pointer to the target shadow stack.
> Update longjmp to search for a restore token. If found, use the token
> to restore the shadow stack pointer before using INCSSP to pop the
> shadow stack. Stop the token search and use INCSSP if the shadow stack
> entry value is the same as the current shadow stack pointer.
>
> It is a user error if there is a shadow stack switch without leaving a
> restore token on the old shadow stack.
looks good except missing saveprevssp below
> ---
> sysdeps/x86_64/__longjmp.S | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/x86_64/__longjmp.S b/sysdeps/x86_64/__longjmp.S
> index 9ac075e0a8..b106affdcd 100644
> --- a/sysdeps/x86_64/__longjmp.S
> +++ b/sysdeps/x86_64/__longjmp.S
> @@ -63,9 +63,35 @@ ENTRY(__longjmp)
> /* Check and adjust the Shadow-Stack-Pointer. */
> /* Get the current ssp. */
> rdsspq %rax
> + /* Save the current ssp. */
> + movq %rax, %r10
> /* And compare it with the saved ssp value. */
> - subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> + movq SHADOW_STACK_POINTER_OFFSET(%rdi), %rcx
> + subq %rcx, %rax
> je L(skip_ssp)
> +
> +L(find_restore_token_loop):
> + /* Look for a restore token. */
> + movq -8(%rcx), %rbx
> + andq $-8, %rbx
> + cmpq %rcx, %rbx
> + /* Find the restore token. */
> + je L(restore_shadow_stack)
> +
> + /* Try the next slot. */
> + subq $8, %rcx
> + /* Stop if the current ssp is found. */
> + cmpq %rcx, %r10
> + je L(no_shadow_stack_token)
> + jmp L(find_restore_token_loop)
> +
> +L(restore_shadow_stack):
> + /* Restore the target shadow stack. */
> + rstorssp -8(%rcx)
> + rdsspq %rax
> + subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> +
i'd use saveprevssp in this case so the old shadow stack
remains resumable with a later longjmp.
> +L(no_shadow_stack_token):
> /* Count the number of frames to adjust and adjust it
> with incssp instruction. The instruction can adjust
> the ssp by [0..255] value only thus use a loop if
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
2023-12-19 17:15 ` szabolcs.nagy
@ 2023-12-19 18:12 ` H.J. Lu
0 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-19 18:12 UTC (permalink / raw)
To: szabolcs.nagy; +Cc: Edgecombe, Rick P, libc-alpha
On Tue, Dec 19, 2023 at 9:16 AM szabolcs.nagy@arm.com
<szabolcs.nagy@arm.com> wrote:
>
> The 12/18/2023 11:06, H.J. Lu wrote:
> > On Mon, Dec 18, 2023 at 2:54 AM szabolcs.nagy@arm.com
> > <szabolcs.nagy@arm.com> wrote:
> > > it does if it scans.
> > >
> > > for (;; targetssp--) {
> > > if (targetssp == ssp) do_samestack();
> > > if (*targetssp == restoretoken) do_differentstack();
> > > }
> > >
> > > the only problem i see is if the target shadow stack is
> > > different from the current one and does not end in a restore
> > > token. but i think that is a user error.
> >
> > Yes, it works. But it is hard to tell its performance overhead.
> >
> > > if we plan to introduce altshadowstack then this does not
> > > work in case of shadow stack overflow because the overflowed
> > > shadow stack cannot be jumped to even though in practice we
> > > want that to work.
> > >
> >
> > I'd like to support shadow stack in glibc 2.39. Since my patch
> > doesn't enable shadow stack by default, it doesn't have any
> > functionality impact on users. It allows us to evaluate shadow
> > stack support in all packages. We may need to use WRUSS
> > to enable shadow stack for some packages. But users and
> > developers can't see how shadow stack works if shadow stack
> > can't be turned on.
>
> ok.
>
> > From 3dba6d876db6a47b66a680bb4aa85b1db63aa3f8 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Wed, 13 Dec 2023 17:50:47 -0800
> > Subject: [PATCH] x86-64/cet: Check the restore token in longjmp
> >
> > setcontext and swapcontext put a restore token on the old shadow stack
> > so that they switch to a different shadow stack when switching user
> > contexts. When longjmp from a user context, the target shadow stack
> > can be different from the current shadow stack and INCSSP can't be
> > used to restore the shadow stack pointer to the target shadow stack.
> > Update longjmp to search for a restore token. If found, use the token
> > to restore the shadow stack pointer before using INCSSP to pop the
> > shadow stack. Stop the token search and use INCSSP if the shadow stack
> > entry value is the same as the current shadow stack pointer.
> >
> > It is a user error if there is a shadow stack switch without leaving a
> > restore token on the old shadow stack.
>
> looks good except missing saveprevssp below
>
> > ---
> > sysdeps/x86_64/__longjmp.S | 28 +++++++++++++++++++++++++++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/sysdeps/x86_64/__longjmp.S b/sysdeps/x86_64/__longjmp.S
> > index 9ac075e0a8..b106affdcd 100644
> > --- a/sysdeps/x86_64/__longjmp.S
> > +++ b/sysdeps/x86_64/__longjmp.S
> > @@ -63,9 +63,35 @@ ENTRY(__longjmp)
> > /* Check and adjust the Shadow-Stack-Pointer. */
> > /* Get the current ssp. */
> > rdsspq %rax
> > + /* Save the current ssp. */
> > + movq %rax, %r10
> > /* And compare it with the saved ssp value. */
> > - subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> > + movq SHADOW_STACK_POINTER_OFFSET(%rdi), %rcx
> > + subq %rcx, %rax
> > je L(skip_ssp)
> > +
> > +L(find_restore_token_loop):
> > + /* Look for a restore token. */
> > + movq -8(%rcx), %rbx
> > + andq $-8, %rbx
> > + cmpq %rcx, %rbx
> > + /* Find the restore token. */
> > + je L(restore_shadow_stack)
> > +
> > + /* Try the next slot. */
> > + subq $8, %rcx
> > + /* Stop if the current ssp is found. */
> > + cmpq %rcx, %r10
> > + je L(no_shadow_stack_token)
> > + jmp L(find_restore_token_loop)
> > +
> > +L(restore_shadow_stack):
> > + /* Restore the target shadow stack. */
> > + rstorssp -8(%rcx)
> > + rdsspq %rax
> > + subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> > +
>
> i'd use saveprevssp in this case so the old shadow stack
> remains resumable with a later longjmp.
Good idea. Will add it. I will submit it as a separate patch
after my current shadow stack patches have been merged.
Thanks.
> > +L(no_shadow_stack_token):
> > /* Count the number of frames to adjust and adjust it
> > with incssp instruction. The instruction can adjust
> > the ssp by [0..255] value only thus use a loop if
> > --
> > 2.43.0
> >
>
--
H.J.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 12/17] elf: Always provide _dl_get_dl_main_map in libc.a
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (10 preceding siblings ...)
2023-12-06 17:20 ` [PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
2023-12-06 17:20 ` [PATCH 13/17] x86/cet: Enable shadow stack during startup H.J. Lu
` (4 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Always provide _dl_get_dl_main_map in libc.a. It will be used by x86
to process PT_GNU_PROPERTY segment.
---
elf/dl-support.c | 2 --
sysdeps/generic/ldsodefs.h | 8 ++++----
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/elf/dl-support.c b/elf/dl-support.c
index 837fa1c836..70c5b3599a 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -344,7 +344,6 @@ _dl_non_dynamic_init (void)
DL_SYSINFO_IMPLEMENTATION
#endif
-#if ENABLE_STATIC_PIE
/* Since relocation to hidden _dl_main_map causes relocation overflow on
aarch64, a function is used to get the address of _dl_main_map. */
@@ -353,7 +352,6 @@ _dl_get_dl_main_map (void)
{
return &_dl_main_map;
}
-#endif
/* This is used by _dl_runtime_profile, not used on static code. */
void
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 9b50ddd09f..0e8a008a49 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1172,10 +1172,6 @@ void __libc_setup_tls (void);
# if ENABLE_STATIC_PIE
/* Relocate static executable with PIE. */
extern void _dl_relocate_static_pie (void) attribute_hidden;
-
-/* Get a pointer to _dl_main_map. */
-extern struct link_map * _dl_get_dl_main_map (void)
- __attribute__ ((visibility ("hidden")));
# else
# define _dl_relocate_static_pie()
# endif
@@ -1217,6 +1213,10 @@ rtld_hidden_proto (_dl_deallocate_tls)
extern void _dl_nothread_init_static_tls (struct link_map *) attribute_hidden;
+/* Get a pointer to _dl_main_map. */
+extern struct link_map * _dl_get_dl_main_map (void)
+ __attribute__ ((visibility ("hidden")));
+
/* Find origin of the executable. */
extern const char *_dl_get_origin (void) attribute_hidden;
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 13/17] x86/cet: Enable shadow stack during startup
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (11 preceding siblings ...)
2023-12-06 17:20 ` [PATCH 12/17] elf: Always provide _dl_get_dl_main_map in libc.a H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
2023-12-06 17:20 ` [PATCH 14/17] x86/cet: Check feature_1 in TCB for active IBT and SHSTK H.J. Lu
` (3 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Previously, CET was enabled by kernel before passing control to user
space and the startup code must disable CET if applications or shared
libraries aren't CET enabled. Since the current kernel only supports
shadow stack and won't enable shadow stack before passing control to
user space, we need to enable shadow stack during startup if the
application and all shared library are shadow stack enabled. There
is no need to disable shadow stack at startup. Shadow stack can only
be enabled in a function which will never return. Otherwise, shadow
stack will underflow at the function return.
1. GL(dl_x86_feature_1) is set to the CET features which are supported
by the processor and are not disabled by the tunable. Only non-zero
features in GL(dl_x86_feature_1) should be enabled. After enabling
shadow stack with ARCH_SHSTK_ENABLE, ARCH_SHSTK_STATUS is used to check
if shadow stack is really enabled.
2. Use ARCH_SHSTK_ENABLE in RTLD_START in dynamic executable. It is
safe since RTLD_START never returns.
3. Call arch_prctl (ARCH_SHSTK_ENABLE) from ARCH_SETUP_TLS in static
executable. Since the start function using ARCH_SETUP_TLS never returns,
it is safe to enable shadow stack in ARCH_SETUP_TLS.
---
sysdeps/unix/sysv/linux/x86/cpu-features.c | 49 --------------
sysdeps/unix/sysv/linux/x86/dl-cet.h | 23 +++++++
sysdeps/unix/sysv/linux/x86_64/dl-cet.h | 47 +++++++++++++
sysdeps/x86/cpu-features-offsets.sym | 1 +
sysdeps/x86/cpu-features.c | 51 --------------
sysdeps/x86/dl-cet.c | 77 +++++++++++-----------
sysdeps/x86/get-cpuid-feature-leaf.c | 2 +-
sysdeps/x86/include/cpu-features.h | 3 +
sysdeps/x86/libc-start.h | 54 ++++++++++++++-
sysdeps/x86_64/dl-machine.h | 12 +++-
10 files changed, 175 insertions(+), 144 deletions(-)
delete mode 100644 sysdeps/unix/sysv/linux/x86/cpu-features.c
create mode 100644 sysdeps/unix/sysv/linux/x86_64/dl-cet.h
diff --git a/sysdeps/unix/sysv/linux/x86/cpu-features.c b/sysdeps/unix/sysv/linux/x86/cpu-features.c
deleted file mode 100644
index 0e6e2bf855..0000000000
--- a/sysdeps/unix/sysv/linux/x86/cpu-features.c
+++ /dev/null
@@ -1,49 +0,0 @@
-/* Initialize CPU feature data for Linux/x86.
- This file is part of the GNU C Library.
- Copyright (C) 2018-2023 Free Software Foundation, Inc.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <https://www.gnu.org/licenses/>. */
-
-#if CET_ENABLED
-# include <sys/prctl.h>
-# include <asm/prctl.h>
-
-static inline int __attribute__ ((always_inline))
-get_cet_status (void)
-{
- unsigned long long kernel_feature;
- unsigned int status = 0;
- if (INTERNAL_SYSCALL_CALL (arch_prctl, ARCH_SHSTK_STATUS,
- &kernel_feature) == 0)
- {
- if ((kernel_feature & ARCH_SHSTK_SHSTK) != 0)
- status = GNU_PROPERTY_X86_FEATURE_1_SHSTK;
- }
- return status;
-}
-
-# ifndef SHARED
-static inline void
-x86_setup_tls (void)
-{
- __libc_setup_tls ();
- THREAD_SETMEM (THREAD_SELF, header.feature_1, GL(dl_x86_feature_1));
-}
-
-# define ARCH_SETUP_TLS() x86_setup_tls ()
-# endif
-#endif
-
-#include <sysdeps/x86/cpu-features.c>
diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h b/sysdeps/unix/sysv/linux/x86/dl-cet.h
index da220ac627..634c885d33 100644
--- a/sysdeps/unix/sysv/linux/x86/dl-cet.h
+++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h
@@ -38,3 +38,26 @@ dl_cet_lock_cet (unsigned int cet_feature)
return (int) INTERNAL_SYSCALL_CALL (arch_prctl, ARCH_SHSTK_LOCK,
kernel_feature);
}
+
+static inline unsigned int __attribute__ ((always_inline))
+dl_cet_get_cet_status (void)
+{
+ unsigned long long kernel_feature;
+ unsigned int status = 0;
+ if (INTERNAL_SYSCALL_CALL (arch_prctl, ARCH_SHSTK_STATUS,
+ &kernel_feature) == 0)
+ {
+ if ((kernel_feature & ARCH_SHSTK_SHSTK) != 0)
+ status = GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+ }
+ return status;
+}
+
+/* Enable shadow stack with a macro to avoid shadow stack underflow. */
+#define ENABLE_X86_CET(cet_feature) \
+ if ((cet_feature & GNU_PROPERTY_X86_FEATURE_1_SHSTK)) \
+ { \
+ long long int kernel_feature = ARCH_SHSTK_SHSTK; \
+ INTERNAL_SYSCALL_CALL (arch_prctl, ARCH_SHSTK_ENABLE, \
+ kernel_feature); \
+ }
diff --git a/sysdeps/unix/sysv/linux/x86_64/dl-cet.h b/sysdeps/unix/sysv/linux/x86_64/dl-cet.h
new file mode 100644
index 0000000000..e23e05c6b8
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86_64/dl-cet.h
@@ -0,0 +1,47 @@
+/* Linux/x86-64 CET initializers function.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <cpu-features-offsets.h>
+#include_next <dl-cet.h>
+
+#define X86_STRINGIFY_1(x) #x
+#define X86_STRINGIFY(x) X86_STRINGIFY_1 (x)
+
+/* Enable shadow stack before calling _dl_init if it is enabled in
+ GL(dl_x86_feature_1). Call _dl_setup_x86_features to setup shadow
+ stack. */
+#define RTLD_START_ENABLE_X86_FEATURES \
+"\
+ # Check if shadow stack is enabled in GL(dl_x86_feature_1).\n\
+ movl _rtld_local+" X86_STRINGIFY (RTLD_GLOBAL_DL_X86_FEATURE_1_OFFSET) "(%rip), %edx\n\
+ testl $" X86_STRINGIFY (X86_FEATURE_1_SHSTK) ", %edx\n\
+ jz 1f\n\
+ # Enable shadow stack if enabled in GL(dl_x86_feature_1).\n\
+ movl $" X86_STRINGIFY (ARCH_SHSTK_SHSTK) ", %esi\n\
+ movl $" X86_STRINGIFY (ARCH_SHSTK_ENABLE) ", %edi\n\
+ movl $" X86_STRINGIFY (__NR_arch_prctl) ", %eax\n\
+ syscall\n\
+1:\n\
+ # Pass GL(dl_x86_feature_1) to _dl_cet_setup_features.\n\
+ movl %edx, %edi\n\
+ # Align stack for the _dl_cet_setup_features call.\n\
+ andq $-16, %rsp\n\
+ call _dl_cet_setup_features\n\
+ # Restore %rax and %rsp from %r12 and %r13.\n\
+ movq %r12, %rax\n\
+ movq %r13, %rsp\n\
+"
diff --git a/sysdeps/x86/cpu-features-offsets.sym b/sysdeps/x86/cpu-features-offsets.sym
index 6d03cea8e8..5429f60632 100644
--- a/sysdeps/x86/cpu-features-offsets.sym
+++ b/sysdeps/x86/cpu-features-offsets.sym
@@ -4,3 +4,4 @@
RTLD_GLOBAL_RO_DL_X86_CPU_FEATURES_OFFSET offsetof (struct rtld_global_ro, _dl_x86_cpu_features)
XSAVE_STATE_SIZE_OFFSET offsetof (struct cpu_features, xsave_state_size)
+RTLD_GLOBAL_DL_X86_FEATURE_1_OFFSET offsetof (struct rtld_global, _dl_x86_feature_1)
diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index f180f0d9a4..097868c1d9 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -1106,57 +1106,6 @@ no_cpuid:
TUNABLE_CALLBACK (set_x86_ibt));
TUNABLE_GET (x86_shstk, tunable_val_t *,
TUNABLE_CALLBACK (set_x86_shstk));
-
- /* Check CET status. */
- unsigned int cet_status = get_cet_status ();
-
- if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
- CPU_FEATURE_UNSET (cpu_features, IBT)
- if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
- CPU_FEATURE_UNSET (cpu_features, SHSTK)
-
- if (cet_status)
- {
- GL(dl_x86_feature_1) = cet_status;
-
-# ifndef SHARED
- /* Check if IBT and SHSTK are enabled by kernel. */
- if ((cet_status
- & (GNU_PROPERTY_X86_FEATURE_1_IBT
- | GNU_PROPERTY_X86_FEATURE_1_SHSTK)))
- {
- /* Disable IBT and/or SHSTK if they are enabled by kernel, but
- disabled by environment variable:
-
- GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
- */
- unsigned int cet_feature = 0;
- if (!CPU_FEATURE_USABLE (IBT))
- cet_feature |= (cet_status
- & GNU_PROPERTY_X86_FEATURE_1_IBT);
- if (!CPU_FEATURE_USABLE (SHSTK))
- cet_feature |= (cet_status
- & GNU_PROPERTY_X86_FEATURE_1_SHSTK);
-
- if (cet_feature)
- {
- int res = dl_cet_disable_cet (cet_feature);
-
- /* Clear the disabled bits in dl_x86_feature_1. */
- if (res == 0)
- GL(dl_x86_feature_1) &= ~cet_feature;
- }
-
- /* Lock CET if IBT or SHSTK is enabled in executable. Don't
- lock CET if IBT or SHSTK is enabled permissively. */
- if (GL(dl_x86_feature_control).ibt != cet_permissive
- && GL(dl_x86_feature_control).shstk != cet_permissive)
- dl_cet_lock_cet (GL(dl_x86_feature_1)
- & (GNU_PROPERTY_X86_FEATURE_1_IBT
- | GNU_PROPERTY_X86_FEATURE_1_SHSTK));
- }
-# endif
- }
#endif
#ifndef SHARED
diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
index 8b911fd931..7f37244d37 100644
--- a/sysdeps/x86/dl-cet.c
+++ b/sysdeps/x86/dl-cet.c
@@ -172,40 +172,11 @@ dl_cet_check_startup (struct link_map *m, struct dl_cet_info *info)
= info->enable_feature_1 ^ info->feature_1_enabled;
if (disable_feature_1 != 0)
{
- /* Disable features in the kernel because of legacy objects or
- cet_always_off. */
- if (dl_cet_disable_cet (disable_feature_1) != 0)
- _dl_fatal_printf ("%s: can't disable x86 Features\n",
- info->program);
-
/* Clear the disabled bits. Sync dl_x86_feature_1 and
info->feature_1_enabled with info->enable_feature_1. */
info->feature_1_enabled = info->enable_feature_1;
GL(dl_x86_feature_1) = info->enable_feature_1;
}
-
- if (HAS_CPU_FEATURE (IBT) || HAS_CPU_FEATURE (SHSTK))
- {
- /* Lock CET features only if IBT or SHSTK are enabled and are not
- enabled permissively. */
- unsigned int feature_1_lock = 0;
-
- if (((info->feature_1_enabled & GNU_PROPERTY_X86_FEATURE_1_IBT)
- != 0)
- && info->enable_ibt_type != cet_permissive)
- feature_1_lock |= GNU_PROPERTY_X86_FEATURE_1_IBT;
-
- if (((info->feature_1_enabled & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
- != 0)
- && info->enable_shstk_type != cet_permissive)
- feature_1_lock |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
-
- if (feature_1_lock != 0
- && dl_cet_lock_cet (feature_1_lock) != 0)
- _dl_fatal_printf ("%s: can't lock CET\n", info->program);
- }
-
- THREAD_SETMEM (THREAD_SELF, header.feature_1, GL(dl_x86_feature_1));
}
#endif
@@ -291,6 +262,15 @@ dl_cet_check (struct link_map *m, const char *program)
{
struct dl_cet_info info;
+ /* CET is enabled only if RTLD_START_ENABLE_X86_FEATURES is defined. */
+#if defined SHARED && defined RTLD_START_ENABLE_X86_FEATURES
+ /* Set dl_x86_feature_1 to features enabled in the executable. */
+ if (program != NULL)
+ GL(dl_x86_feature_1) = (m->l_x86_feature_1_and
+ & (X86_FEATURE_1_IBT
+ | X86_FEATURE_1_SHSTK));
+#endif
+
/* Check how IBT and SHSTK should be enabled. */
info.enable_ibt_type = GL(dl_x86_feature_control).ibt;
info.enable_shstk_type = GL(dl_x86_feature_control).shstk;
@@ -300,17 +280,9 @@ dl_cet_check (struct link_map *m, const char *program)
/* No legacy object check if IBT and SHSTK are always on. */
if (info.enable_ibt_type == cet_always_on
&& info.enable_shstk_type == cet_always_on)
- {
-#ifdef SHARED
- /* Set it only during startup. */
- if (program != NULL)
- THREAD_SETMEM (THREAD_SELF, header.feature_1,
- info.feature_1_enabled);
-#endif
- return;
- }
+ return;
- /* Check if IBT and SHSTK were enabled by kernel. */
+ /* Check if IBT and SHSTK were enabled. */
if (info.feature_1_enabled == 0)
return;
@@ -344,6 +316,33 @@ _dl_cet_open_check (struct link_map *l)
dl_cet_check (l, NULL);
}
+/* Set GL(dl_x86_feature_1) to the enabled features and clear the
+ active bits of the disabled features. */
+
+attribute_hidden
+void
+_dl_cet_setup_features (unsigned int cet_feature)
+{
+ /* NB: cet_feature == GL(dl_x86_feature_1) which is set to features
+ enabled from executable, not necessarily supported by kernel. */
+ if (cet_feature)
+ {
+ cet_feature = dl_cet_get_cet_status ();
+ /* Sync GL(dl_x86_feature_1) with kernel. */
+ GL(dl_x86_feature_1) = cet_feature;
+ if (cet_feature)
+ {
+ THREAD_SETMEM (THREAD_SELF, header.feature_1, cet_feature);
+
+ /* Lock CET if IBT or SHSTK is enabled in executable. Don't
+ lock CET if IBT or SHSTK is enabled permissively. */
+ if (GL(dl_x86_feature_control).ibt != cet_permissive
+ && (GL(dl_x86_feature_control).shstk != cet_permissive))
+ dl_cet_lock_cet (cet_feature);
+ }
+ }
+}
+
#ifdef SHARED
# ifndef LINKAGE
diff --git a/sysdeps/x86/get-cpuid-feature-leaf.c b/sysdeps/x86/get-cpuid-feature-leaf.c
index 40a46cc79c..9317a6b494 100644
--- a/sysdeps/x86/get-cpuid-feature-leaf.c
+++ b/sysdeps/x86/get-cpuid-feature-leaf.c
@@ -24,7 +24,7 @@ __x86_get_cpuid_feature_leaf (unsigned int leaf)
static const struct cpuid_feature feature = {};
if (leaf < CPUID_INDEX_MAX)
return ((const struct cpuid_feature *)
- &GLRO(dl_x86_cpu_features).features[leaf]);
+ &GLRO(dl_x86_cpu_features).features[leaf]);
else
return &feature;
}
diff --git a/sysdeps/x86/include/cpu-features.h b/sysdeps/x86/include/cpu-features.h
index 2d7427a6c0..23bd8146a2 100644
--- a/sysdeps/x86/include/cpu-features.h
+++ b/sysdeps/x86/include/cpu-features.h
@@ -990,6 +990,9 @@ extern const struct cpu_features *_dl_x86_get_cpu_features (void)
# define INIT_ARCH()
# define _dl_x86_get_cpu_features() (&GLRO(dl_x86_cpu_features))
extern void _dl_x86_init_cpu_features (void) attribute_hidden;
+
+extern void _dl_cet_setup_features (unsigned int)
+ attribute_hidden;
#endif
#ifdef __x86_64__
diff --git a/sysdeps/x86/libc-start.h b/sysdeps/x86/libc-start.h
index e93da6ef3d..856230daeb 100644
--- a/sysdeps/x86/libc-start.h
+++ b/sysdeps/x86/libc-start.h
@@ -19,7 +19,57 @@
#ifndef SHARED
# define ARCH_SETUP_IREL() apply_irel ()
# define ARCH_APPLY_IREL()
-# ifndef ARCH_SETUP_TLS
-# define ARCH_SETUP_TLS() __libc_setup_tls ()
+# ifdef __CET__
+/* Get CET features enabled in the static executable. */
+
+static inline unsigned int
+get_cet_feature (void)
+{
+ /* Check if CET is supported and not disabled by tunables. */
+ struct cpu_features *cpu_features
+ = (struct cpu_features *) __get_cpu_features ();
+ unsigned int cet_feature = 0;
+ if (CPU_FEATURE_USABLE_P (cpu_features, IBT))
+ cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT;
+ if (CPU_FEATURE_USABLE_P (cpu_features, SHSTK))
+ cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+ if (!cet_feature)
+ return cet_feature;
+
+ struct link_map *main_map = _dl_get_dl_main_map ();
+
+ /* Scan program headers backward to check PT_GNU_PROPERTY early for
+ x86 feature bits on static executable. */
+ const ElfW(Phdr) *phdr = GL(dl_phdr);
+ const ElfW(Phdr) *ph;
+ for (ph = phdr + GL(dl_phnum); ph != phdr; ph--)
+ if (ph[-1].p_type == PT_GNU_PROPERTY)
+ {
+ _dl_process_pt_gnu_property (main_map, -1, &ph[-1]);
+ /* Enable IBT and SHSTK only if they are enabled on static
+ executable. */
+ cet_feature &= (main_map->l_x86_feature_1_and
+ & (GNU_PROPERTY_X86_FEATURE_1_IBT
+ | GNU_PROPERTY_X86_FEATURE_1_SHSTK));
+ /* Set GL(dl_x86_feature_1) to the enabled CET features. */
+ GL(dl_x86_feature_1) = cet_feature;
+ break;
+ }
+
+ return cet_feature;
+}
+
+/* The function using this macro to enable shadow stack must not return
+ to avoid shadow stack underflow. */
+# define ARCH_SETUP_TLS() \
+ { \
+ __libc_setup_tls (); \
+ \
+ unsigned int cet_feature = get_cet_feature (); \
+ ENABLE_X86_CET (cet_feature); \
+ _dl_cet_setup_features (cet_feature); \
+ }
+# else
+# define ARCH_SETUP_TLS() __libc_setup_tls ()
# endif
#endif /* !SHARED */
diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
index 581a2f1a9e..faeae723cb 100644
--- a/sysdeps/x86_64/dl-machine.h
+++ b/sysdeps/x86_64/dl-machine.h
@@ -29,6 +29,11 @@
#include <dl-static-tls.h>
#include <dl-machine-rel.h>
#include <isa-level.h>
+#ifdef __CET__
+# include <dl-cet.h>
+#else
+# define RTLD_START_ENABLE_X86_FEATURES
+#endif
/* Return nonzero iff ELF header is compatible with the running host. */
static inline int __attribute__ ((unused))
@@ -146,13 +151,16 @@ _start:\n\
_dl_start_user:\n\
# Save the user entry point address in %r12.\n\
movq %rax, %r12\n\
+ # Save %rsp value in %r13.\n\
+ movq %rsp, %r13\n\
+"\
+ RTLD_START_ENABLE_X86_FEATURES \
+"\
# Read the original argument count.\n\
movq (%rsp), %rdx\n\
# Call _dl_init (struct link_map *main_map, int argc, char **argv, char **env)\n\
# argc -> rsi\n\
movq %rdx, %rsi\n\
- # Save %rsp value in %r13.\n\
- movq %rsp, %r13\n\
# And align stack for the _dl_init call. \n\
andq $-16, %rsp\n\
# _dl_loaded -> rdi\n\
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 14/17] x86/cet: Check feature_1 in TCB for active IBT and SHSTK
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (12 preceding siblings ...)
2023-12-06 17:20 ` [PATCH 13/17] x86/cet: Enable shadow stack during startup H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
2023-12-06 17:20 ` [PATCH 15/17] x86/cet: Don't disable CET if not single threaded H.J. Lu
` (2 subsequent siblings)
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Initially, IBT and SHSTK are marked as active when CPU supports them
and CET are enabled in glibc. They can be disabled early by tunables
before relocation. Since after relocation, GLRO(dl_x86_cpu_features)
becomes read-only, we can't update GLRO(dl_x86_cpu_features) to mark
IBT and SHSTK as inactive. Instead, check the feature_1 field in TCB
to decide if IBT and SHST are active.
---
sysdeps/x86/bits/platform/x86.h | 8 ++++++++
sysdeps/x86/get-cpuid-feature-leaf.c | 11 ++++++++++-
sysdeps/x86/sys/platform/x86.h | 17 +++++++++++++++++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/sysdeps/x86/bits/platform/x86.h b/sysdeps/x86/bits/platform/x86.h
index 1e23d53ba2..1575ae53fb 100644
--- a/sysdeps/x86/bits/platform/x86.h
+++ b/sysdeps/x86/bits/platform/x86.h
@@ -337,3 +337,11 @@ enum
x86_cpu_AVX10_YMM = x86_cpu_index_24_ecx_0_ebx + 17,
x86_cpu_AVX10_ZMM = x86_cpu_index_24_ecx_0_ebx + 18,
};
+
+/* Bits in the feature_1 field in TCB. */
+
+enum
+{
+ x86_feature_1_ibt = 1U << 0,
+ x86_feature_1_shstk = 1U << 1
+};
diff --git a/sysdeps/x86/get-cpuid-feature-leaf.c b/sysdeps/x86/get-cpuid-feature-leaf.c
index 9317a6b494..f69936b31e 100644
--- a/sysdeps/x86/get-cpuid-feature-leaf.c
+++ b/sysdeps/x86/get-cpuid-feature-leaf.c
@@ -15,9 +15,18 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-
+#include <assert.h>
+#include <tcb-offsets.h>
#include <ldsodefs.h>
+#ifdef __x86_64__
+# ifdef __LP64__
+_Static_assert (FEATURE_1_OFFSET == 72, "FEATURE_1_OFFSET != 72");
+# else
+_Static_assert (FEATURE_1_OFFSET == 40, "FEATURE_1_OFFSET != 40");
+# endif
+#endif
+
const struct cpuid_feature *
__x86_get_cpuid_feature_leaf (unsigned int leaf)
{
diff --git a/sysdeps/x86/sys/platform/x86.h b/sysdeps/x86/sys/platform/x86.h
index 1ea2c5fc0b..89b1b16f22 100644
--- a/sysdeps/x86/sys/platform/x86.h
+++ b/sysdeps/x86/sys/platform/x86.h
@@ -45,6 +45,23 @@ x86_cpu_present (unsigned int __index)
static __inline__ _Bool
x86_cpu_active (unsigned int __index)
{
+ if (__index == x86_cpu_IBT || __index == x86_cpu_SHSTK)
+ {
+#ifdef __x86_64__
+ unsigned int __feature_1;
+# ifdef __LP64__
+ __asm__ ("mov %%fs:72, %0" : "=r" (__feature_1));
+# else
+ __asm__ ("mov %%fs:40, %0" : "=r" (__feature_1));
+# endif
+ if (__index == x86_cpu_IBT)
+ return __feature_1 & x86_feature_1_ibt;
+ else
+ return __feature_1 & x86_feature_1_shstk;
+#else
+ return false;
+#endif
+ }
const struct cpuid_feature *__ptr = __x86_get_cpuid_feature_leaf
(__index / (8 * sizeof (unsigned int) * 4));
unsigned int __reg
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 15/17] x86/cet: Don't disable CET if not single threaded
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (13 preceding siblings ...)
2023-12-06 17:20 ` [PATCH 14/17] x86/cet: Check feature_1 in TCB for active IBT and SHSTK H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
2023-12-06 17:20 ` [PATCH 16/17] x86/cet: Don't set CET active by default H.J. Lu
2023-12-06 17:20 ` [PATCH 17/17] x86/cet: Run some CET tests with shadow stack H.J. Lu
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
In permissive mode, don't disable IBT nor SHSTK when dlopening a legacy
shared library if not single threaded since IBT and SHSTK may be still
enabled in other threads. Other threads with IBT or SHSTK enabled will
crash when calling functions in the legacy shared library. Instead, an
error will be issued.
---
sysdeps/x86/dl-cet.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
index 7f37244d37..c6c407d7f7 100644
--- a/sysdeps/x86/dl-cet.c
+++ b/sysdeps/x86/dl-cet.c
@@ -20,6 +20,7 @@
#include <libintl.h>
#include <ldsodefs.h>
#include <dl-cet.h>
+#include <sys/single_threaded.h>
/* GNU_PROPERTY_X86_FEATURE_1_IBT and GNU_PROPERTY_X86_FEATURE_1_SHSTK
are defined in <elf.h>, which are only available for C sources.
@@ -204,7 +205,10 @@ dl_cet_check_dlopen (struct link_map *m, struct dl_cet_info *info)
&& (info->feature_1_legacy
& GNU_PROPERTY_X86_FEATURE_1_IBT) != 0)
{
- if (info->enable_ibt_type != cet_permissive)
+ /* Don't disable IBT if not single threaded since IBT may be still
+ enabled in other threads. */
+ if (info->enable_ibt_type != cet_permissive
+ || !SINGLE_THREAD_P)
{
legacy_obj = info->feature_1_legacy_ibt;
msg = N_("rebuild shared object with IBT support enabled");
@@ -220,7 +224,10 @@ dl_cet_check_dlopen (struct link_map *m, struct dl_cet_info *info)
&& (info->feature_1_legacy
& GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0)
{
- if (info->enable_shstk_type != cet_permissive)
+ /* Don't disable SHSTK if not single threaded since SHSTK may be
+ still enabled in other threads. */
+ if (info->enable_shstk_type != cet_permissive
+ || !SINGLE_THREAD_P)
{
legacy_obj = info->feature_1_legacy_shstk;
msg = N_("rebuild shared object with SHSTK support enabled");
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 16/17] x86/cet: Don't set CET active by default
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (14 preceding siblings ...)
2023-12-06 17:20 ` [PATCH 15/17] x86/cet: Don't disable CET if not single threaded H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
2023-12-06 17:20 ` [PATCH 17/17] x86/cet: Run some CET tests with shadow stack H.J. Lu
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
Not all CET enabled applications and libraries have been properly tested
in CET enabled environments. Some CET enabled applications or libraries
will crash or misbehave when CET is enabled. Don't set CET active by
default so that all applications and libraries will run normally regardless
of whether CET is active or not. Shadow stack can be enabled by
$ export GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
at run-time if shadow stack can be enabled by kernel.
NB: This commit can be reverted if it is OK to enable CET by default for
all applications and libraries.
---
sysdeps/x86/cpu-features.c | 2 +-
sysdeps/x86/cpu-tunables.c | 17 ++++++++++++++++-
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index 097868c1d9..80a07ac589 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -110,7 +110,7 @@ update_active (struct cpu_features *cpu_features)
if (!CPU_FEATURES_CPU_P (cpu_features, RTM_ALWAYS_ABORT))
CPU_FEATURE_SET_ACTIVE (cpu_features, RTM);
-#if CET_ENABLED
+#if CET_ENABLED && 0
CPU_FEATURE_SET_ACTIVE (cpu_features, IBT);
CPU_FEATURE_SET_ACTIVE (cpu_features, SHSTK);
#endif
diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c
index 5697885226..8f4f25efb0 100644
--- a/sysdeps/x86/cpu-tunables.c
+++ b/sysdeps/x86/cpu-tunables.c
@@ -34,6 +34,18 @@
break; \
}
+#define CHECK_GLIBC_IFUNC_CPU_BOTH(f, cpu_features, name, \
+ disable, len) \
+ _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
+ if (memcmp (f, #name, len) == 0) \
+ { \
+ if (disable) \
+ CPU_FEATURE_UNSET (cpu_features, name) \
+ else \
+ CPU_FEATURE_SET_ACTIVE (cpu_features, name) \
+ break; \
+ }
+
/* Disable a preferred feature NAME. We don't enable a preferred feature
which isn't available. */
#define CHECK_GLIBC_IFUNC_PREFERRED_OFF(f, cpu_features, name, len) \
@@ -149,11 +161,14 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
}
break;
case 5:
+ {
+ CHECK_GLIBC_IFUNC_CPU_BOTH (n, cpu_features, SHSTK, disable,
+ 5);
+ }
if (disable)
{
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, LZCNT, 5);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, MOVBE, 5);
- CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SHSTK, 5);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SSSE3, 5);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, XSAVE, 5);
}
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 17/17] x86/cet: Run some CET tests with shadow stack
2023-12-06 17:19 [PATCH 00/17] x86/cet: Update CET kernel interface H.J. Lu
` (15 preceding siblings ...)
2023-12-06 17:20 ` [PATCH 16/17] x86/cet: Don't set CET active by default H.J. Lu
@ 2023-12-06 17:20 ` H.J. Lu
16 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2023-12-06 17:20 UTC (permalink / raw)
To: libc-alpha; +Cc: rick.p.edgecombe
When CET is disabled by default, run some CET tests with shadow stack
enabled using
$ export GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
---
sysdeps/x86/Makefile | 14 ++++++++++++++
sysdeps/x86/tst-shstk-legacy-1e-static.sh | 1 +
sysdeps/x86/tst-shstk-legacy-1e.sh | 1 +
sysdeps/x86/tst-shstk-legacy-1g.sh | 1 +
4 files changed, 17 insertions(+)
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 6ceefe16c7..906f158d69 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -202,6 +202,13 @@ CFLAGS-tst-cet-legacy-10-static.c += -mshstk
CFLAGS-tst-cet-legacy-10a.c += -fcf-protection=none
CFLAGS-tst-cet-legacy-10a-static.c += -fcf-protection=none
+tst-cet-legacy-4-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
+tst-cet-legacy-6-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
+tst-cet-legacy-10-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
+tst-cet-legacy-10-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
+tst-cet-legacy-10a-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
+tst-cet-legacy-10a-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
+
CFLAGS-tst-shstk-legacy-1a.c += -fcf-protection=none
CFLAGS-tst-shstk-legacy-1a-static.c += -fcf-protection=none
CFLAGS-tst-shstk-legacy-1d.c += -fcf-protection=none
@@ -241,14 +248,20 @@ tst-cet-legacy-6b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
tst-cet-legacy-9-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
tst-cet-legacy-9-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
+tst-shstk-legacy-1a-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
+tst-shstk-legacy-1a-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
$(objpfx)tst-shstk-legacy-1a: $(objpfx)tst-shstk-legacy-1-extra.o
$(objpfx)tst-shstk-legacy-1a-static: $(objpfx)tst-shstk-legacy-1-extra.o
+tst-shstk-legacy-1b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
+tst-shstk-legacy-1b-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
$(objpfx)tst-shstk-legacy-1b: $(objpfx)tst-shstk-legacy-1-extra.o
$(objpfx)tst-shstk-legacy-1b-static: $(objpfx)tst-shstk-legacy-1-extra.o
tst-shstk-legacy-1c-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK
tst-shstk-legacy-1c-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK
$(objpfx)tst-shstk-legacy-1c: $(objpfx)tst-shstk-legacy-1-extra.o
$(objpfx)tst-shstk-legacy-1c-static: $(objpfx)tst-shstk-legacy-1-extra.o
+tst-shstk-legacy-1d-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
+tst-shstk-legacy-1d-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
$(objpfx)tst-shstk-legacy-1d: $(objpfx)tst-shstk-legacy-1-extra.o
$(objpfx)tst-shstk-legacy-1d-static: $(objpfx)tst-shstk-legacy-1-extra.o
$(objpfx)tst-shstk-legacy-1e: $(objpfx)tst-shstk-legacy-1-extra.o
@@ -262,6 +275,7 @@ $(objpfx)tst-shstk-legacy-1e-static.out: \
$(objpfx)tst-shstk-legacy-1e-static
$(SHELL) $< $(common-objpfx) 2> $@; \
$(evaluate-test)
+tst-shstk-legacy-1f-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK
$(objpfx)tst-shstk-legacy-1f: $(objpfx)tst-shstk-legacy-mod-1.so
$(objpfx)tst-shstk-legacy-mod-1.so: \
$(objpfx)tst-shstk-legacy-mod-1.os \
diff --git a/sysdeps/x86/tst-shstk-legacy-1e-static.sh b/sysdeps/x86/tst-shstk-legacy-1e-static.sh
index e943aec70e..008c50dae3 100755
--- a/sysdeps/x86/tst-shstk-legacy-1e-static.sh
+++ b/sysdeps/x86/tst-shstk-legacy-1e-static.sh
@@ -20,6 +20,7 @@
common_objpfx=$1; shift
+GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK \
${common_objpfx}elf/tst-shstk-legacy-1e-static
# The exit status should only be unsupported (77) or segfault (139).
status=$?
diff --git a/sysdeps/x86/tst-shstk-legacy-1e.sh b/sysdeps/x86/tst-shstk-legacy-1e.sh
index b0467aa899..82f2acbf75 100755
--- a/sysdeps/x86/tst-shstk-legacy-1e.sh
+++ b/sysdeps/x86/tst-shstk-legacy-1e.sh
@@ -21,6 +21,7 @@
common_objpfx=$1; shift
test_program_prefix=$1; shift
+GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK \
${test_program_prefix} \
${common_objpfx}elf/tst-shstk-legacy-1e
# The exit status should only be unsupported (77) or segfault (139).
diff --git a/sysdeps/x86/tst-shstk-legacy-1g.sh b/sysdeps/x86/tst-shstk-legacy-1g.sh
index c112bf6d8d..261eef7cac 100755
--- a/sysdeps/x86/tst-shstk-legacy-1g.sh
+++ b/sysdeps/x86/tst-shstk-legacy-1g.sh
@@ -21,6 +21,7 @@
common_objpfx=$1; shift
test_program_prefix=$1; shift
+GLIBC_TUNABLES=glibc.cpu.hwcaps=SHSTK \
${test_program_prefix} \
${common_objpfx}elf/tst-shstk-legacy-1g
# The exit status should only be unsupported (77) or segfault (139).
--
2.43.0
^ permalink raw reply [flat|nested] 42+ messages in thread