Some of the Gcc compiled clone tests in restricted mode face stack corruption. This occurs as the run-time permission settings (executive or restricted mode) in the PCC may be lost if function pointers are materialised from the GOT entries. This further leads to selecting an incorrect stack pointer(CSP instead of RCSP) and hence the crash.

As a workaround, materialise the function pointer clone_base_fn explicitly by building branch address from the PCC. Here it is assumed that the bounds and permissions of clone_base_fn are same as the PCC , which is not an issue in practice (being in the same translation unit).

Signed-off-by: Amit Daniel Kachhap amit.kachhap@arm.com --- tools/testing/selftests/arm64/morello/clone.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/arm64/morello/clone.c b/tools/testing/selftests/arm64/morello/clone.c index 69189287144a..a511054f90fa 100644 --- a/tools/testing/selftests/arm64/morello/clone.c +++ b/tools/testing/selftests/arm64/morello/clone.c @@ -109,6 +109,21 @@ void clone_single(struct test_fixture *data)

int clone_flags = CLONE_VM | CLONE_PARENT_SETTID | CLONE_CHILD_SETTID;

+ int (*clone_fn_ptr)(void *) = clone_base_fn; + + /* + * Function pointers are materialised by the compiler by either + * computing from the PCC (ADRP + ADD + SEAL) or loading from GOT. The + * run-time permission settings (executive or restrictive) in PCC may + * not match with the GOT entries. Hence, materialise the function + * pointers explicitly to avoid this mismatch issue. + */ + if (in_restricted() && + (cheri_perms_get(clone_fn_ptr) & ARM_CAP_PERMISSION_EXECUTIVE)) { + clone_fn_ptr = cheri_address_set(cheri_pcc_get(), (ptraddr_t)clone_fn_ptr); + clone_fn_ptr = cheri_sentry_create(clone_fn_ptr); + } + ASSERT_NE(new_stack, NULL); /* For stack probing .... */ data->sp = new_stack + STACK_SIZE; @@ -119,7 +134,7 @@ void clone_single(struct test_fixture *data)

EXPECT_TRUE(!(data->flags & CLONE_TH_RESTRICTED) || in_restricted());

- result = __clone(clone_base_fn, (uintcap_t)new_stack + STACK_SIZE, + result = __clone(clone_fn_ptr, (uintcap_t)new_stack + STACK_SIZE, clone_flags, data, &ppid, tls, &cpid);

EXPECT_GT(result, 0) {

Some of the clone tests use loop check on status flag to determine if the child exited normally. However, this may cause infinite looping if the data handshake does not happen properly between parent and child due to compiler optimization as observed with Morello Gcc.

Although this can be fixed by setting the flag to volatile but here we replace the busy loop with the waitpid() syscall to wait for the child to exit before parent is executed. This approach allows us to get rid of item status and result from struct test_fixture.

Signed-off-by: Amit Daniel Kachhap amit.kachhap@arm.com --- tools/testing/selftests/arm64/morello/clone.c | 36 +++++++------------ .../selftests/arm64/morello/freestanding.h | 7 ++++ 2 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/tools/testing/selftests/arm64/morello/clone.c b/tools/testing/selftests/arm64/morello/clone.c index 60514fa5f869..68d32261c8de 100644 --- a/tools/testing/selftests/arm64/morello/clone.c +++ b/tools/testing/selftests/arm64/morello/clone.c @@ -18,16 +18,13 @@ #define in_restricted() \ (!(cheri_perms_get(cheri_pcc_get()) & ARM_CAP_PERMISSION_EXECUTIVE))

- -#define MAGIC_NR 0x3562 /* whatever really ..... */ - #ifndef MAX_PID_NS_LEVEL #define MAX_PID_NS_LEVEL 32 #endif

/* Cloned thread result */ -#define CTR_SUCCESS 1 -#define CTR_FAILED -1 +#define CTR_SUCCESS 0 +#define CTR_FAILED 1

#define BIT(nr) ((1U) << (nr)) /* Test flags */ @@ -36,10 +33,8 @@ #define CLONE_TH_RUSAGE BIT(3)

struct test_fixture { - int status; int flags; void *sp; - int result; };

#define PROBE_INTERVAL (1 << 12) @@ -76,27 +71,21 @@ static int clone_base_fn(void *data)

asm("mrs %0, ctpidr_el0" : "=C" (tls));

- if (!tls) { - __data->result = CTR_FAILED; - goto done; - } + if (!tls) + return CTR_FAILED;

probe_addr_range(tls, STACK_SIZE >> 10, 64); }

/* If things didn't explode by now .... */ - __data->result = - !!(__data->flags & CLONE_TH_RESTRICTED) != in_restricted() ? - CTR_FAILED : CTR_SUCCESS; -done: - __data->status = MAGIC_NR; - return 0; + return !!(__data->flags & CLONE_TH_RESTRICTED) != in_restricted() ? + CTR_FAILED : CTR_SUCCESS; }

static inline __attribute__((always_inline)) void clone_single(struct test_fixture *data) { - int ppid = 0, cpid = 0; + int ppid = 0, cpid = 0, wstatus; int result = -EINVAL;

void *new_stack = mmap_verified(NULL, STACK_SIZE, PROT_READ | PROT_WRITE, @@ -107,7 +96,7 @@ void clone_single(struct test_fixture *data) CAP_LOAD_PERMS | CAP_STORE_PERMS) : NULL;

- int clone_flags = CLONE_VM | CLONE_PARENT_SETTID | CLONE_CHILD_SETTID; + int clone_flags = CLONE_VM | CLONE_PARENT_SETTID | CLONE_CHILD_SETTID | SIGCHLD;

int (*clone_fn_ptr)(void *) = clone_base_fn;

@@ -142,10 +131,9 @@ void clone_single(struct test_fixture *data) goto leave; }

- /* Wait substitute ... */ - while (data->status != MAGIC_NR) { - asm(""); - } + /* Wait for the child to exit */ + result = waitpid(cpid, &wstatus, 0); + ASSERT_EQ(result, cpid); /* * CLONE_CHILD_SETTID sets child's thread ID to provided child's * memory but as VM is being shared, it's all good at this point. @@ -153,7 +141,7 @@ void clone_single(struct test_fixture *data) * Either way if this point has been reached - all went 'supposedly' * well. */ - ASSERT_EQ(data->result, CTR_SUCCESS); + ASSERT_TRUE(WIFEXITED(wstatus) && (WEXITSTATUS(wstatus) == CTR_SUCCESS)); leave: munmap(new_stack, STACK_SIZE); if (tls) diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 0b0de05d46bc..2beb52eafae1 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -31,6 +31,13 @@ typedef __uintcap_t uintcap_t;

#define EXIT_SUCCESS 0

+#ifndef WIFEXITED +#define WIFEXITED(status) (((status) & 0x7f) == 0) +#endif +#ifndef WEXITSTATUS +#define WEXITSTATUS(status) (((status) & 0xff00) >> 8) +#endif + struct __test_meta { int message; };