Message-ID: <202408052046.00BC7CBC@keescook>
Date: Mon, 5 Aug 2024 20:54:54 -0700
From: Kees Cook <kees@...nel.org>
To: Mark Brown <broonie@...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>,
	Daniel Bristot de Oliveira <bristot@...hat.com>,
	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 Wed, Jul 31, 2024 at 01:14:15PM +0100, Mark Brown wrote:
> Add basic test coverage for specifying the shadow stack for a newly
> created thread via clone3(), including coverage of the newly extended
> argument structure.  We check that a user specified shadow stack can be
> provided, and that invalid combinations of parameters are rejected.
> 
> In order to facilitate testing on systems without userspace shadow stack
> support we manually enable shadow stacks on startup, this is architecture
> specific due to the use of an arch_prctl() on x86. Due to interactions with
> potential userspace locking of features we actually detect support for
> shadow stacks on the running system by attempting to allocate a shadow
> stack page during initialisation using map_shadow_stack(), warning if this
> succeeds when the enable failed.
> 
> In order to allow testing of user configured shadow stacks on
> architectures with that feature we need to ensure that we do not return
> from the function where the clone3() syscall is called in the child
> process, doing so would trigger a shadow stack underflow.  To do this we
> use inline assembly rather than the standard syscall wrapper to call
> clone3().  In order to avoid surprises we also use a syscall rather than
> the libc exit() function., this should be overly cautious.
> 
> Signed-off-by: Mark Brown <broonie@...nel.org>
> ---
>  tools/testing/selftests/clone3/clone3.c           | 134 +++++++++++++++++++++-
>  tools/testing/selftests/clone3/clone3_selftests.h |  38 ++++++
>  2 files changed, 171 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c
> index 26221661e9ae..81c2e8648e8b 100644
> --- a/tools/testing/selftests/clone3/clone3.c
> +++ b/tools/testing/selftests/clone3/clone3.c
> @@ -3,6 +3,7 @@
>  /* Based on Christian Brauner's clone3() example */
>  
>  #define _GNU_SOURCE
> +#include <asm/mman.h>
>  #include <errno.h>
>  #include <inttypes.h>
>  #include <linux/types.h>
> @@ -11,6 +12,7 @@
>  #include <stdint.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <sys/mman.h>
>  #include <sys/syscall.h>
>  #include <sys/types.h>
>  #include <sys/un.h>
> @@ -19,8 +21,12 @@
>  #include <sched.h>
>  
>  #include "../kselftest.h"
> +#include "../ksft_shstk.h"
>  #include "clone3_selftests.h"
>  
> +static bool shadow_stack_supported;
> +static size_t max_supported_args_size;
> +
>  enum test_mode {
>  	CLONE3_ARGS_NO_TEST,
>  	CLONE3_ARGS_ALL_0,
> @@ -28,6 +34,10 @@ enum test_mode {
>  	CLONE3_ARGS_INVAL_EXIT_SIGNAL_NEG,
>  	CLONE3_ARGS_INVAL_EXIT_SIGNAL_CSIG,
>  	CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG,
> +	CLONE3_ARGS_SHADOW_STACK,
> +	CLONE3_ARGS_SHADOW_STACK_NO_SIZE,
> +	CLONE3_ARGS_SHADOW_STACK_NO_POINTER,
> +	CLONE3_ARGS_SHADOW_STACK_NO_TOKEN,
>  };
>  
>  typedef bool (*filter_function)(void);
> @@ -44,6 +54,44 @@ struct test {
>  	filter_function filter;
>  };
>  
> +
> +/*
> + * We check for shadow stack support by attempting to use
> + * map_shadow_stack() since features may have been locked by the
> + * dynamic linker resulting in spurious errors when we attempt to
> + * enable on startup.  We warn if the enable failed.
> + */
> +static void test_shadow_stack_supported(void)
> +{
> +	long ret;
> +
> +	ret = syscall(__NR_map_shadow_stack, 0, getpagesize(), 0);
> +	if (ret == -1) {
> +		ksft_print_msg("map_shadow_stack() not supported\n");
> +	} else if ((void *)ret == MAP_FAILED) {
> +		ksft_print_msg("Failed to map shadow stack\n");
> +	} else {
> +		ksft_print_msg("Shadow stack supportd\n");

typo: supportd -> supported

> +		shadow_stack_supported = true;
> +
> +		if (!shadow_stack_enabled)
> +			ksft_print_msg("Mapped but did not enable shadow stack\n");
> +	}
> +}

On my CET system, this reports:

  ...
  # clone3() syscall supported
  # Shadow stack supportd
  # Running test 'simple clone3()'
  ...

(happily doesn't print "Mapped but did not enable ...").

> +
> +static unsigned long long get_shadow_stack_page(unsigned long flags)
> +{
> +	unsigned long long page;
> +
> +	page = syscall(__NR_map_shadow_stack, 0, getpagesize(), flags);
> +	if ((void *)page == MAP_FAILED) {
> +		ksft_print_msg("map_shadow_stack() failed: %d\n", errno);
> +		return 0;
> +	}
> +
> +	return page;
> +}
> +
>  static int call_clone3(uint64_t flags, size_t size, enum test_mode test_mode)
>  {
>  	struct __clone_args args = {
> @@ -89,6 +137,21 @@ static int call_clone3(uint64_t flags, size_t size, enum test_mode test_mode)
>  	case CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG:
>  		args.exit_signal = 0x00000000000000f0ULL;
>  		break;
> +	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?

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

> +	case CLONE3_ARGS_SHADOW_STACK_NO_SIZE:
> +		args.shadow_stack = get_shadow_stack_page(SHADOW_STACK_SET_TOKEN);
> +		break;

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

Same?

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

Let me know what would be most helpful to dig into more...

-- 
Kees Cook

Powered by blists - more mailing lists