linux-kernel - Re: [PATCH RFT v7 9/9] selftests/clone3: Test shadow stack support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <19ee6fc9-94d7-4420-abd3-7cfdf612df0c@sirena.org.uk>
Date: Tue, 6 Aug 2024 16:10:02 +0100
From: Mark Brown <broonie@...nel.org>
To: Kees Cook <kees@...nel.org>
Cc: "Rick P. Edgecombe" <rick.p.edgecombe@...el.com>,
	Deepak Gupta <debug@...osinc.com>,
	Szabolcs Nagy <Szabolcs.Nagy@....com>,
	"H.J. Lu" <hjl.tools@...il.com>,
	Florian Weimer <fweimer@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Juri Lelli <juri.lelli@...hat.com>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
	Valentin Schneider <vschneid@...hat.com>,
	Christian Brauner <brauner@...nel.org>,
	Shuah Khan <shuah@...nel.org>, linux-kernel@...r.kernel.org,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>, jannh@...gle.com,
	linux-kselftest@...r.kernel.org, linux-api@...r.kernel.org
Subject: Re: [PATCH RFT v7 9/9] selftests/clone3: Test shadow stack support

On Mon, Aug 05, 2024 at 08:54:54PM -0700, Kees Cook wrote:
> On Wed, Jul 31, 2024 at 01:14:15PM +0100, Mark Brown wrote:

> > +	case CLONE3_ARGS_SHADOW_STACK:
> > +		/* We need to specify a normal stack too to avoid corruption */
> > +		args.shadow_stack = get_shadow_stack_page(SHADOW_STACK_SET_TOKEN);
> > +		args.shadow_stack_size = getpagesize();
> > +		break;

>   # Running test 'Shadow stack on system with shadow stack'
>   # [5496] Trying clone3() with flags 0 (size 0)
>   # I am the parent (5496). My child's pid is 5505
>   # Child exited with signal 11
>   # [5496] clone3() with flags says: 11 expected 0
>   # [5496] Result (11) is different than expected (0)
>   not ok 20 Shadow stack on system with shadow stack

> The child segfaults immediately, it seems?

That's what I'd expect if we either didn't manage to create the shadow
stack token in the page we mapped or we messed up in
arch_shstk_post_fork() somehow, probably getting the wrong pointer for
the token or not mapping things correctly.  I'll have done something
silly, it'll doubtless be very obvious and embarrassing when I see it
but I'm not seeing it right now...

> > +	case CLONE3_ARGS_SHADOW_STACK_NO_POINTER:
> > +		args.shadow_stack_size = getpagesize();
> > +		break;

>   # Running test 'Shadow stack with no pointer'
>   # [5496] Trying clone3() with flags 0 (size 0)
>   # Invalid argument - Failed to create new process
>   # [5496] clone3() with flags says: -22 expected -22
>   ok 21 Shadow stack with no pointer

> This seems like it misses the failure and reports ok

No, this is testing to make sure we get the failure - if we have
arguments that can't possibly be valid then we should reject them with
an error code during validation prior to trying to create the new
thread.  The "expected -22" in the output says it's looking for an
error.  Same for the other similar expected error code.

> > +	case CLONE3_ARGS_SHADOW_STACK_NO_TOKEN:
> > +		args.shadow_stack = get_shadow_stack_page(0);
> > +		args.shadow_stack_size = getpagesize();
> > +		break;

> 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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)