All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: "Rick P. Edgecombe" <rick.p.edgecombe@intel.com>,
	Deepak Gupta <debug@rivosinc.com>,
	Szabolcs Nagy <Szabolcs.Nagy@arm.com>,
	"H.J. Lu" <hjl.tools@gmail.com>,
	Florian Weimer <fweimer@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	Christian Brauner <brauner@kernel.org>,
	Shuah Khan <shuah@kernel.org>,
	linux-kernel@vger.kernel.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	jannh@google.com, linux-kselftest@vger.kernel.org,
	linux-api@vger.kernel.org
Subject: Re: [PATCH RFT v7 9/9] selftests/clone3: Test shadow stack support
Date: Tue, 6 Aug 2024 22:08:44 -0700	[thread overview]
Message-ID: <202408062022.34F3558@keescook> (raw)
In-Reply-To: <19ee6fc9-94d7-4420-abd3-7cfdf612df0c@sirena.org.uk>

On Tue, Aug 06, 2024 at 04:10:02PM +0100, Mark Brown wrote:
> On Mon, Aug 05, 2024 at 08:54:54PM -0700, Kees Cook wrote:
> > This actually segfaults the parent:
> 
> >   # Running test 'Shadow stack with no token'
> >   # [5496] Trying clone3() with flags 0x100 (size 0)
> >   # I am the parent (5496). My child's pid is 5507
> >   Segmentation fault
> 
> Oh dear.  We possibly manage to corrupt the parent's shadow stack
> somehow?  I don't think I managed to do that in my arm64 testing.  This
> should also be something going wrong in arch_shstk_post_fork().
> 
> > Let me know what would be most helpful to dig into more...
> 
> It'll almost certianly be something in arch_shstk_post_fork(), that's
> the bit I couldn't test.  Just making that always return success should
> avoid the first fault, the second ought to not crash but will report a
> fail as we should be rejecting the shadow stack when we try to consume
> the 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?

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) +++
<... wait4 resumed>[{WIFSIGNALED(s) && WTERMSIG(s) == SIGSEGV && WCOREDUMP(s)}], __WALL, NULL) = 4944
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_DUMPED, si_pid=4944, si_uid=0, si_status=SIGSEGV, si_utime=0, si_stime=0} ---
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x7272d21fffe8} ---
+++ killed by SIGSEGV (core dumped) +++

[  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?

And the parent dies with SEGV_MAPERR??

I'll keep looking in the morning ...

-- 
Kees Cook

  reply	other threads:[~2024-08-07  5:08 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-31 12:14 [PATCH RFT v7 0/9] fork: Support shadow stacks in clone3() Mark Brown
2024-07-31 12:14 ` [PATCH RFT v7 1/9] Documentation: userspace-api: Add shadow stack API documentation Mark Brown
2024-07-31 12:14 ` [PATCH RFT v7 2/9] selftests: Provide helper header for shadow stack testing Mark Brown
2024-07-31 12:14 ` [PATCH RFT v7 3/9] mm: Introduce ARCH_HAS_USER_SHADOW_STACK Mark Brown
2024-07-31 12:14 ` [PATCH RFT v7 4/9] fork: Add shadow stack support to clone3() Mark Brown
2024-07-31 12:14 ` [PATCH RFT v7 5/9] selftests/clone3: Remove redundant flushes of output streams Mark Brown
2024-07-31 12:14 ` [PATCH RFT v7 6/9] selftests/clone3: Factor more of main loop into test_clone3() Mark Brown
2024-07-31 12:14 ` [PATCH RFT v7 7/9] selftests/clone3: Explicitly handle child exits due to signals Mark Brown
2024-07-31 12:14 ` [PATCH RFT v7 8/9] selftests/clone3: Allow tests to flag if -E2BIG is a valid error code Mark Brown
2024-07-31 12:14 ` [PATCH RFT v7 9/9] selftests/clone3: Test shadow stack support Mark Brown
2024-08-06  3:54   ` Kees Cook
2024-08-06 15:10     ` Mark Brown
2024-08-07  5:08       ` Kees Cook [this message]
2024-08-07 12:39         ` Mark Brown
2024-08-07 18:23           ` Kees Cook
2024-08-07 19:23           ` Kees Cook
2024-08-07 22:03             ` Mark Brown
2024-08-07 23:22               ` Kees Cook
2024-08-06 20:10     ` Mark Brown
2024-08-06 21:43       ` Kees Cook
2024-08-06 21:57         ` Mark Brown
2024-08-06 22:21           ` Mark Brown
2024-08-05 21:29 ` [PATCH RFT v7 0/9] fork: Support shadow stacks in clone3() Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202408062022.34F3558@keescook \
    --to=kees@kernel.org \
    --cc=Szabolcs.Nagy@arm.com \
    --cc=bp@alien8.de \
    --cc=brauner@kernel.org \
    --cc=broonie@kernel.org \
    --cc=bsegall@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=debug@rivosinc.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=fweimer@redhat.com \
    --cc=hjl.tools@gmail.com \
    --cc=hpa@zytor.com \
    --cc=jannh@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=rostedt@goodmis.org \
    --cc=shuah@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.