linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Dave Martin <Dave.Martin@arm.com>
To: Cristian Marussi <cristian.marussi@arm.com>
Cc: andreyknvl@google.com, shuah@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v3 05/11] kselftest: arm64: mangle_pstate_ssbs_regs
Date: Tue, 13 Aug 2019 17:25:09 +0100	[thread overview]
Message-ID: <20190813162509.GC10425@arm.com> (raw)
In-Reply-To: <20190802170300.20662-6-cristian.marussi@arm.com>

On Fri, Aug 02, 2019 at 06:02:54PM +0100, Cristian Marussi wrote:
> Added a simple mangle testcase which messes with the ucontext_t

Add

> from within the sig_handler, trying to toggle PSTATE SSBS bit.

signal handler

> Expect SIGILL if SSBS feature unsupported or that the value set in
> PSTATE.SSBS is preserved on test PASS.

The test doesn't set PSTATE.SSBS directly.

Maybe something like: "Expect SIGILL if the SSBS feature is unsupported.
Otherwise, expect sigreturn to set PSTATE.SSBS from the corresponding
bit in pstate in the signal frame."

> This commit also introduces a new common utility function:
> get_current_context() which can be used to grab a ucontext without
> the help of libc, and detect if such ucontext has been actively used
> to jump back into it.
> 
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
>  .../selftests/arm64/signal/test_signals.h     |  4 +
>  .../arm64/signal/test_signals_utils.c         | 93 +++++++++++++++++++
>  .../arm64/signal/test_signals_utils.h         |  2 +
>  .../arm64/signal/testcases/.gitignore         |  1 +
>  .../testcases/mangle_pstate_ssbs_regs.c       | 56 +++++++++++
>  5 files changed, 156 insertions(+)
>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c
> 
> diff --git a/tools/testing/selftests/arm64/signal/test_signals.h b/tools/testing/selftests/arm64/signal/test_signals.h
> index 85db3ac44b32..37bed0590226 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals.h
> +++ b/tools/testing/selftests/arm64/signal/test_signals.h
> @@ -116,6 +116,10 @@ struct tdescr {
>  	/* optional sa_flags for the installed handler */
>  	int		sa_flags;
>  	ucontext_t	saved_uc;
> +	/* used by get_current_ctx() */
> +	size_t		live_sz;
> +	ucontext_t	*live_uc;
> +	volatile bool	live_uc_valid;
>  
>  	/* a setup function to be called before test starts */
>  	int (*setup)(struct tdescr *td);
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.c b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> index ac0055f6340b..faf55ba99d58 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals_utils.c
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> @@ -11,12 +11,16 @@
>  #include <linux/auxvec.h>
>  #include <ucontext.h>
>  
> +#include <asm/unistd.h>
> +
>  #include "test_signals.h"
>  #include "test_signals_utils.h"
>  #include "testcases/testcases.h"
>  
>  extern struct tdescr *current;
>  
> +static int sig_copyctx = SIGUSR2;
> +
>  static char *feats_store[FMAX_END] = {
>  	"SSBS",
>  	"PAN",
> @@ -37,6 +41,85 @@ static inline char *feats_to_string(unsigned long feats)
>  	return feats_string;
>  }
>  
> +/*
> + * Obtaining a valid and full-blown ucontext_t from userspace is tricky:
> + * libc getcontext does() not save all the regs and messes with some of
> + * them (pstate value in particular is not reliable).
> + * Here we use a service signal to grab the ucontext_t from inside a
> + * dedicated signal handler, since there, it is populated by Kernel
> + * itself in setup_sigframe(). The grabbed context is then stored and
> + * made available in td->live_uc.
> + *
> + * Anyway this function really serves a dual purpose:
> + *
> + * 1. grab a valid sigcontext into td->live_uc for result analysis: in
> + * such case it returns 1.
> + *
> + * 2. detect if somehow a previously grabbed live_uc context has been
> + * used actively with a sigreturn: in such a case the execution would have
> + * magically resumed in the middle of the function itself (seen_already==1):
> + * in such a case return 0, since in fact we have not just simply grabbed
> + * the context.
> + *
> + * This latter case is useful to detect when a fake_sigreturn test-case has
> + * unexpectedly survived without hittig a SEGV.
> + */
> +bool get_current_context(struct tdescr *td, ucontext_t *dest_uc)
> +{
> +	static volatile sig_atomic_t seen_already;
> +
> +	if (!td || !dest_uc) {
> +		fprintf(stdout, "Signal-based Context dumping NOT available\n");

Should this ever happen inless there is a test bug?

Maybe this should just be an assert.

> +		return 0;
> +	}
> +
> +	/* it's a genuine invokation..reinit */
> +	seen_already = 0;
> +	td->live_uc_valid = 0;
> +	td->live_sz = sizeof(*dest_uc);
> +	memset(dest_uc, 0x00, td->live_sz);

Eventually we will need to examine the signal frame to determine its
size, but for now this is fine.

It will start to matter for SVE.

> +	td->live_uc = dest_uc;
> +	/*
> +	 * Grab ucontext_t triggering a signal...
> +	 * ASM equivalent of raise(sig_copyctx);
> +	 *
> +	 * Note that:
> +	 * - live_uc_valid is declared volatile in struct tdescr
> +	 *   since it will be changed inside the sig_copyctx handler.
> +	 * - the kill() syscall invocation returns only after any possible
> +	 *   registered sig_handler for the invoked signal has returned,

sig_handler looks like the name of some function of variable, but I
can't find it.  Did I miss something?

> +	 *   so that live_uc_valid flag is surely up to date when this
> +	 *   function return it.
> +	 * - the additional 'memory' clobber is there to avoid possible
> +	 *   compiler's assumption on the content pointed by dest_uc, which
> +	 *   is changed inside the handler, but not referenced here anyway.
> +	 */
> +	asm volatile ("mov x8, %0\n\t"
> +		      "svc #0\n\t"
> +		      "mov x1, %1\n\t"
> +		      "mov x8, %2\n\t"
> +		      "svc #0"
> +		      :
> +		      : "i" (__NR_getpid),
> +		        "r" (sig_copyctx),
> +			"i" (__NR_kill)
> +		      : "x1","x8","x0","memory");
> +	/*
> +	 * If we get here with seen_already==1 it implies the td->live_uc
> +	 * context has been used to get back here....this probably means
> +	 * a test has failed to cause a SEGV...anyway the live_uc has not
> +	 * just been acquired...so return 0
> +	 */
> +	if (seen_already) {
> +		fprintf(stdout,
> +			"Successful sigreturn detected: live_uc is stale !\n");
> +		return 0;
> +	}
> +	seen_already = 1;
> +
> +	return td->live_uc_valid;
> +}
> +
>  static void unblock_signal(int signum)
>  {
>  	sigset_t sset;
> @@ -112,6 +195,12 @@ static void default_handler(int signum, siginfo_t *si, void *uc)
>  		 * to terminate immediately exiting straight away
>  		 */
>  		default_result(current, 1);
> +	} else if (signum == sig_copyctx && current->live_uc) {
> +		memcpy(current->live_uc, uc, current->live_sz);
> +		ASSERT_GOOD_CONTEXT(current->live_uc);
> +		current->live_uc_valid = 1;
> +		fprintf(stderr,
> +			"GOOD CONTEXT grabbed from sig_copyctx handler\n");
>  	} else {
>  		if (signum == current->sig_unsupp && !are_feats_ok(current)) {
>  			fprintf(stderr, "-- RX SIG_UNSUPP on unsupported feature...OK\n");
> @@ -214,6 +303,10 @@ static int test_init(struct tdescr *td)
>  			!feats_ok ? "NOT " : "");
>  	}
>  
> +	if (td->sig_trig == sig_copyctx)
> +		sig_copyctx = SIGUSR1;

What's this for?  What if we have the same signal for sig_trig and
sig_copyctx?

> +	unblock_signal(sig_copyctx);
> +
>  	td->initialized = 1;
>  	return 1;
>  }
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.h b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> index 8658d1a7d4b9..ce35be8ebc8e 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals_utils.h
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> @@ -10,4 +10,6 @@ int test_setup(struct tdescr *td);
>  void test_cleanup(struct tdescr *td);
>  int test_run(struct tdescr *td);
>  void test_result(struct tdescr *td);
> +
> +bool get_current_context(struct tdescr *td, ucontext_t *dest_uc);
>  #endif
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> index 226bb179b673..a48a118b1a1a 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/.gitignore
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -3,3 +3,4 @@ mangle_pstate_invalid_daif_bits
>  mangle_pstate_invalid_mode_el1
>  mangle_pstate_invalid_mode_el2
>  mangle_pstate_invalid_mode_el3
> +mangle_pstate_ssbs_regs
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c
> new file mode 100644
> index 000000000000..a399d9aa40d5
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#include <stdio.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +static int mangle_invalid_pstate_ssbs_run(struct tdescr *td,
> +					  siginfo_t *si, ucontext_t *uc)
> +{
> +	ASSERT_GOOD_CONTEXT(uc);
> +
> +	/* set bit value */

Should we clear SSBS in the test setup (using MSR), to make sure that
sigreturn really succeeds in _changing_ the bit to 1?

> +	uc->uc_mcontext.pstate |= PSR_SSBS_BIT;
> +	fprintf(stderr, "SSBS set to 1 -- PSTATE: 0x%016lX\n",
> +		uc->uc_mcontext.pstate);
> +	/* Save after mangling...it should be preserved */
> +	td->saved_uc = *uc;
> +
> +	return 1;
> +}

[...]

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-08-13 16:28 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-02 17:02 [PATCH v3 00/11] Add arm64/signal initial kselftest support Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 01/11] kselftest: arm64: introduce new boilerplate code Cristian Marussi
2019-08-13 16:23   ` Dave Martin
2019-08-27 12:14     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 02/11] kselftest: arm64: adds first test and common utils Cristian Marussi
2019-08-06 15:50   ` Cristian Marussi
2019-08-07 15:42   ` Cristian Marussi
2019-08-09 10:54   ` Cristian Marussi
2019-08-09 11:16     ` Dave Martin
2019-08-09 12:20       ` Cristian Marussi
2019-08-09 12:32         ` Dave Martin
2019-08-12 12:43   ` Amit Kachhap
2019-08-13 13:22     ` Cristian Marussi
2019-08-14 10:22       ` Amit Kachhap
2019-08-27 14:24         ` Cristian Marussi
2019-08-13 16:24   ` Dave Martin
2019-08-28 17:34     ` Cristian Marussi
2019-09-03 15:34       ` Dave Martin
2019-09-03 16:08         ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 03/11] kselftest: arm64: mangle_pstate_invalid_daif_bits Cristian Marussi
2019-08-13 16:24   ` Dave Martin
2019-08-29 10:19     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 04/11] kselftest: arm64: mangle_pstate_invalid_mode_el Cristian Marussi
2019-08-13 16:24   ` Dave Martin
2019-08-29 11:50     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 05/11] kselftest: arm64: mangle_pstate_ssbs_regs Cristian Marussi
2019-08-13 16:25   ` Dave Martin [this message]
2019-08-29 15:35     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 06/11] kselftest: arm64: fake_sigreturn_bad_magic Cristian Marussi
2019-08-13 16:25   ` Dave Martin
2019-08-30 14:29     ` Cristian Marussi
2019-09-04 10:05       ` Dave Martin
2019-09-04 10:37         ` Cristian Marussi
2019-09-04 10:47           ` Dave Martin
2019-08-02 17:02 ` [PATCH v3 07/11] kselftest: arm64: fake_sigreturn_bad_size_for_magic0 Cristian Marussi
2019-08-13 16:25   ` Dave Martin
2019-08-30 14:49     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 08/11] kselftest: arm64: fake_sigreturn_missing_fpsimd Cristian Marussi
2019-08-13 16:26   ` Dave Martin
2019-08-30 14:55     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 09/11] kselftest: arm64: fake_sigreturn_duplicated_fpsimd Cristian Marussi
2019-08-13 16:26   ` Dave Martin
2019-08-30 15:11     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 10/11] kselftest: arm64: fake_sigreturn_bad_size Cristian Marussi
2019-08-13 16:26   ` Dave Martin
2019-08-30 15:21     ` Cristian Marussi
2019-08-02 17:03 ` [PATCH v3 11/11] kselftest: arm64: fake_sigreturn_misaligned_sp Cristian Marussi
2019-08-07 16:04   ` Cristian Marussi
2019-08-13 16:28     ` Dave Martin
2019-08-30 15:22       ` Cristian Marussi
2019-08-13 16:27   ` Dave Martin
2019-08-30 16:33     ` Cristian Marussi
2019-08-13 16:22 ` [PATCH v3 00/11] Add arm64/signal initial kselftest support Dave Martin
2019-08-30 16:40   ` Cristian Marussi
2019-09-02 10:53     ` Dave Martin
2019-09-02 11:30       ` Cristian Marussi

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=20190813162509.GC10425@arm.com \
    --to=dave.martin@arm.com \
    --cc=andreyknvl@google.com \
    --cc=cristian.marussi@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=shuah@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).