Re: [PATCH v3 05/11] kselftest: arm64: mangle_pstate_ssbs_regs

Hi

On 13/08/2019 17:25, Dave Martin wrote:

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

Ok.

...

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

Ok

...

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.

Yes definitely better.

...

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

Yes this function has been reworked in the SVE signal frame patches to dynamically detect runtime signal frame and be able to optionally grab a sigframe containing SVE material (avoiding the kill() syscall to trigger a signal in favour of a brk instruction to cause a SIGTRAP without passing via the syscall machinery which would zero the SVE sigframe stuff)

Such patch, which depends on this series, is still not published.

...

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

No I'll replace the comment with "signal handler"

...

• *   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?

To avoid that a user defined sig_trig equal to sig_copyctx can fool this function. In SVE signal frame patch (not in this series), I'll anyway switch to use a distinct SIGTRAP sig_trig.

...

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

Yes. I introduced a set_regval() asm helper and added a new .init signal-test framework entry in tdescr to be able to call per-test specific initialization.

I took the chance also to remove the remaining volatile qualifiers from signal handling code to please checkpatch, and add a dsb barrier to ensure the writes by the signal handler.

...

• 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

Cheers

Cristian