On Tue, Aug 06, 2024 at 10:08:44PM -0700, Kees Cook wrote:

On Tue, Aug 06, 2024 at 04:10:02PM +0100, Mark Brown wrote:

quoted

quoted

  # Running test 'Shadow stack with no token'

It took me a while to figure out where a thread switches shstk (even
without this series):

kernel_clone, copy_process, copy_thread, fpu_clone, update_fpu_shstk
(and shstk_alloc_thread_stack is called just before update_fpu_shstk).

I don't understand the token consumption in arch_shstk_post_fork(). This
wasn't needed before with the fixed-size new shstk, why is it needed
now?

Concerns were raised on earlier rounds of review that since instead of
allocating the shadow stack as part of creating the new thread we are
using a previously allocated shadow stack someone could use this as part
of an exploit.  You could just jump on top of any existing shadow stack
and cause writes to it.

Anyway, my attempt to trace the shstk changes for the test:

write(1, "TAP version 13\n", 15)        = 15
write(1, "1..2\n", 5)                   = 5
clone3({flags=0, exit_signal=18446744073709551615, stack=NULL, stack_size=0}, 104) = -1 EINVAL (Invalid argument)
write(1, "# clone3() syscall supported\n", 29) = 29
map_shadow_stack(NULL, 4096, 0)         = 125837480497152
write(1, "# Shadow stack supportd\n", 24) = 24
write(1, "# Running test 'Shadow stack wit"..., 44) = 44
getpid()                                = 4943
write(1, "# [4943] Trying clone3() with fl"..., 51) = 51
map_shadow_stack(NULL, 4096, 0)         = 125837480488960
clone3({flags=CLONE_VM, exit_signal=SIGCHLD, stack=NULL, stack_size=0, /* bytes 88..103 */ "\x00\xf0\x52\xd2\x72\x72\x00\x00\x00\x10\x00\x00\x00\x00\x00\x00"} => {/* bytes 88..103 */ "\x00\xf0\x52\xd2\x72\x72\x00\x00\x00\x10\x00\x00\x00\x00\x00\x00"}, 104) = 4944
getpid()                                = 4943
write(1, "# I am the parent (4943). My chi"..., 49strace: Process 4944 attached
) = 49
[pid  4944] --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_CPERR, si_addr=NULL} ---
[pid  4943] wait4(-1,  <unfinished ...>
[pid  4944] +++ killed by SIGSEGV (core dumped) +++

So we created the thread, then before we get to the wait4() in the
parent we start delivering a SEGV_CPERR to the child.  The flow for the
child is as expected.

Then the parent gets an ordinary segfault, not a shadow stack specific
one, like some memory got deallocated underneath it or a pointer got
corrupted.

[  569.153288] shstk_setup: clone3[4943] ssp:7272d2200000
[  569.153998] process: copy_thread: clone3[4943] new_ssp:7272d2530000
[  569.154002] update_fpu_shstk: clone3[4943] ssp:7272d2530000
[  569.154008] shstk_post_fork: clone3[4944]
[  569.154011] shstk_post_fork: clone3[4944] sending SIGSEGV post fork

I don't see an update_fpu_shstk for 4944? Should I with this test?

I'd only expect to see one update, my understanding is that that update
is for the child but happening in the context of the parent as the hild
is not yet started.

Does this help:

diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
index 27acbdf44c5f..d7005974aff5 100644
--- a/arch/x86/kernel/shstk.c
+++ b/arch/x86/kernel/shstk.c

@@ -258,6 +258,8 @@ unsigned long shstk_alloc_thread_stack(struct task_struct *tsk,
 	if (args->shadow_stack) {
 		addr = args->shadow_stack;
 		size = args->shadow_stack_size;
+		shstk->base = 0;
+		shstk->size = 0;
 	} else {
 		/*
 		 * For CLONE_VFORK the child will share the parents