[linux-morello] Re: [Linux-morello] [PATCH 5/5] kselftests/arm64/morello: Test for new aux entries and init registers

On 02/09/2022 11:45, Sherwin da Cruz wrote:

Amend an existing morello selftest to check that new capability aux entries are valid and unsealed, as well as check that registers c0-c3 match argc/argv/envp/auxv as viewed on the stack.

Signed-off-by: Sherwin da Cruz sherwin.dacruz@arm.com

.../selftests/arm64/morello/bootstrap.c | 58 ++++++++++++++----- .../arm64/morello/freestanding_start.S | 12 +++- 2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/arm64/morello/bootstrap.c b/tools/testing/selftests/arm64/morello/bootstrap.c index 92816a82f9e3..b272806a9c2c 100644 --- a/tools/testing/selftests/arm64/morello/bootstrap.c +++ b/tools/testing/selftests/arm64/morello/bootstrap.c @@ -43,6 +43,15 @@ struct morello_auxv { uintcap_t a_val; }; +struct stack_items_t {

Naming: structs are in their own namespace so we don't use an _t suffix (this is only used for typedefs). Also I'd suggest "initial_data" as name as those things don't have to be on the stack.

• int argc;
• char **argv;
• char **envp;
• struct morello_auxv *auxv;

+};

+static struct stack_items_t stack_items;

Maybe "initial_data_from_regs" to make it very clear what the difference is with the data parsed from the stack in test_stack?

• int clear_child_tid;

void verify_string(char *str) @@ -70,6 +79,9 @@ int verify_auxval(struct morello_auxv *auxv) switch (auxv->a_type) { case AT_NULL: return 0;

• case AT_CHERI_EXEC_RW_CAP:
• case AT_CHERI_INTERP_RW_CAP:
• case AT_CHERI_INTERP_RX_CAP: case AT_BASE: /* Fall through if not null, abi allows it */ if ((void *)auxv->a_val == NULL)

@@ -79,6 +91,12 @@ int verify_auxval(struct morello_auxv *auxv) case AT_PLATFORM: case AT_RANDOM: case AT_PHDR:

• case AT_CHERI_EXEC_RX_CAP:
• case AT_CHERI_STACK_CAP:
• case AT_CHERI_SEAL_CAP:
• case AT_CHERI_CID_CAP:
• case AT_ARGV:
• case AT_ENVP: /* valid and unsealed */ ASSERT_TRUE(cheri_tag_get(auxv->a_val)) TH_LOG("auxv (%ld) value is invalid", auxv->a_type);

@@ -131,48 +149,57 @@ TEST(test_c64) TEST(test_stack) {

• int argc_reg = stack_items.argc;
• char **argv_reg = stack_items.argv;
• char **envp_reg = stack_items.envp;
• struct morello_auxv *auxv_reg = stack_items.auxv;

Considering you use each of these only once, I don't see the point in copying them in local variables. Just access each member of stack_items directly where you need it.

Kevin

• /* copy the pointer so we can modify it */ char **stack = stack_from_kernel;

• int argc;

• int argc_stack; /* start with an argc check */ VERIFY_CAP(stack, sizeof(void *), STACK_PERMS, "argc");

/* dereference and go past argc */

• argc = *(int *)stack;

• argc_stack = *(int *)stack;
• ASSERT_EQ(argc_stack, argc_reg); stack += 1;

/* argv + null */

• VERIFY_CAP(stack, sizeof(void *) * (argc + 1), STACK_PERMS, "argv");

• VERIFY_CAP(stack, sizeof(void *) * (argc_stack + 1), STACK_PERMS, "argv_stack");
• ASSERT_CAP_EQ(argv_reg, stack);

/* we are clear to dereference all argv */

• for (int i = 0; i < argc; i++) {

• for (int i = 0; i < argc_stack; i++) { char *arg = *(stack + i); verify_string(arg); }

/* go past argv */

• stack += argc;
• ASSERT_NULL(*stack) TH_LOG("argv was not null terminated");

• stack += argc_stack;
• ASSERT_NULL(*stack) TH_LOG("argv was not null terminated on stack"); /* go past the null terminator */ stack += 1;

/* progressively check bounds for envp and dereference */

• ASSERT_CAP_EQ(envp_reg, stack); while (1) {

• char *envp = *stack;
  

• char *envp_stack = *stack;
  

VERIFY_CAP(stack, sizeof(void *), STACK_PERMS, "envp"); stack += 1;

• if (envp == NULL)
  

• if (envp_stack == NULL)
  break;
  

• verify_string(envp);
  

• verify_string(envp_stack);
  

  }

/* finally, go through auxv */

• ASSERT_CAP_EQ(auxv_reg, stack); while (1) {

• struct morello_auxv *auxv = ((struct morello_auxv *) stack);
  

• struct morello_auxv *auxv_stack = ((struct morello_auxv *) stack);
  

• VERIFY_CAP(auxv, sizeof(struct morello_auxv), STACK_PERMS, "auxv");
  

• if (verify_auxval(auxv) == 0)
  

• VERIFY_CAP(auxv_stack, sizeof(struct morello_auxv), STACK_PERMS, "auxv");
  

• if (verify_auxval(auxv_stack) == 0)
  break;
  

  stack += 2; }

@@ -192,8 +219,13 @@ TEST(test_set_tid_address_initial)

• exit test. We assume exit() works and we progressively build a known working
• test environment.

*/ -int main(void) +int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) {

• stack_items.argc = argc;
• stack_items.argv = argv;
• stack_items.envp = envp;
• stack_items.auxv = auxv;
• test_write(); /* from now on write and exit work, so go wild */ test_c64();

diff --git a/tools/testing/selftests/arm64/morello/freestanding_start.S b/tools/testing/selftests/arm64/morello/freestanding_start.S index 9eccac44d623..66dfe2bab676 100644 --- a/tools/testing/selftests/arm64/morello/freestanding_start.S +++ b/tools/testing/selftests/arm64/morello/freestanding_start.S @@ -66,6 +66,12 @@ VARIABLE(__cur_test, 16) .text FUNCTION_START(_start)

• /* save x0, c1-c3 which has argc/argv/envp/auxv */
• mov x20, x0
• mov c21, c1
• mov c22, c2
• mov c23, c3
• bl __morello_init_globals

/* save the initial kernel stack to a global variable */ @@ -84,7 +90,11 @@ FUNCTION_START(_start) mov csp, c0 #endif

• /* call int main(void) */

• /* call int main(int argc, char **argv, char **envp, char **auxv) */
• mov x0, x20
• mov c1, c21
• mov c2, c22
• mov c3, c23 bl main

/* exit. Argument is in x0 already */