[linux-morello] Re: [PATCH v2 09/11] kselftests/arm64: morello: Process the dynamic relocations

On 2/6/23 21:19, Kevin Brodsky wrote:

On 02/02/2023 10:26, Amit Daniel Kachhap wrote:

...

Gcc emits only dynamic relocations and not cap relocations like Clang toolchain. Hence the existing kselftests compiled in static and standalone mode needs to process the dynamic relocations and fill the entries to relevant capability details.

The code changes are similar to the start up code of the recently released Arm GNU Toolchain for Morello.

Signed-off-by: Amit Daniel Kachhap amit.kachhap@arm.com

.../arm64/morello/freestanding_init_globals.c | 113 +++++++++++++++++- .../arm64/morello/freestanding_start.S | 5 +- 2 files changed, 111 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/arm64/morello/freestanding_init_globals.c b/tools/testing/selftests/arm64/morello/freestanding_init_globals.c index 6139de724f24..76d8968adf0b 100644 --- a/tools/testing/selftests/arm64/morello/freestanding_init_globals.c +++ b/tools/testing/selftests/arm64/morello/freestanding_init_globals.c @@ -5,8 +5,19 @@ #include <stdbool.h> #include <stddef.h> #include <stdint.h> +#include <linux/auxvec.h> +#include <linux/elf.h>

• #include "freestanding.h"

+#define DRELOC_FRAG_EXEC 4 +#define DRELOC_FRAG_RWDATA 2 +#define DRELOC_FRAG_RODATA 1

+#ifndef R_MORELLO_RELATIVE +#define R_MORELLO_RELATIVE 59395 +#endif

• struct cap_reloc { size_t capability_location; size_t base;

@@ -23,7 +34,7 @@ struct cap_reloc {

• because capability relocations must have already been processed in order to
• refer to such symbols.

*/ -void __morello_init_globals(void) +void __morello_process_cap_relocs(void) { const struct cap_reloc *start_cap_relocs, *end_cap_relocs, *reloc; uintcap_t root_cap; @@ -33,11 +44,17 @@ void __morello_init_globals(void) * not be indirected through the GOT, as this would create a capability * relocation. We need assembly to refer to those directly. */

• asm("adrp %0, __start___cap_relocs\n\t"

• asm(".weak __start___cap_relocs\n\t"

•    ".hidden __start___cap_relocs\n\t"
  

•    "adrp %0, __start___cap_relocs\n\t"
   "add %0, %0, #:lo12:__start___cap_relocs\n\t"
  

•    "adrp %1, __stop___cap_relocs\n\t"
  

•    "add %1, %1, #:lo12:__stop___cap_relocs"
  

•    : "=C"(start_cap_relocs), "=C"(end_cap_relocs));
  

•    : "=C"(start_cap_relocs));
  

• asm(".weak __stop___cap_relocs\n\t"

•    ".hidden __stop___cap_relocs\n\t"
  

•    "adrp %0, __stop___cap_relocs\n\t"
  

•    "add %0, %0, #:lo12:__stop___cap_relocs"
  

•    : "=C"(end_cap_relocs));
  

root_cap = (uintcap_t)cheri_ddc_get(); @@ -63,3 +80,89 @@ void __morello_init_globals(void) *target = cap; } }

+static void get_caps(uintptr_t *cap_rx, uintptr_t *cap_rw, const uintptr_t *auxv) +{

• for (;;) {

• switch ((unsigned long)auxv[0])	{
  

• case AT_NULL:
  

• 	return;
  

• case AT_CHERI_EXEC_RX_CAP:
  

• 	*cap_rx = auxv[1];
  

• 	break;
  

• case AT_CHERI_EXEC_RW_CAP:
  

• 	*cap_rw = auxv[1];
  

• 	break;
  

• }
  

• auxv += 2;
  

• }

+}

+static inline uintptr_t morello_relative(uint64_t base, uintptr_t cap_rx, uintptr_t cap_rw,

• 			 Elf64_Rela *reloc, void *reloc_addr)
  

+{

• uint64_t __attribute__((may_alias)) *u64_reloc_addr = reloc_addr;
• /*

• * Fragment identified by r_offset has the following information:
  

• * | 64-bit: address | 56-bits: length | 8-bits: permissions |
  

• */
  

• unsigned long loc = u64_reloc_addr[0];
• unsigned long len = u64_reloc_addr[1] & ((1UL << 56) - 1);
• unsigned long perm = u64_reloc_addr[1] >> 56;
• uintptr_t value;
• if (perm == DRELOC_FRAG_EXEC)

• value = cheri_address_set(cap_rx, base + loc);
  

• else

• value = cheri_address_set(cap_rw, base + loc);
  

• value = value + reloc->r_addend;
• if (perm == DRELOC_FRAG_EXEC || perm == DRELOC_FRAG_RODATA)

• value = cheri_perms_clear(value, CHERI_PERM_SEAL | CHERI_PERM_STORE |
  

• 			  CHERI_PERM_STORE_CAP | CHERI_PERM_STORE_LOCAL_CAP);
  

The issue with clearing out some permissions instead of setting them explicitly is that it's easy to forget some (Unseal for instance). I think it's better to stick to the explicit permission mask like in v1, my comment there was simply that the base can be just Global.

As discussed offline, I will add base capability with Global permission.

...

• if (perm == DRELOC_FRAG_RWDATA || perm == DRELOC_FRAG_RODATA) {

• value = cheri_perms_clear(value, CHERI_PERM_SEAL | CHERI_PERM_EXECUTE);
  

• value = cheri_bounds_set_exact(value, len);
  

• }
• /* Seal executable capabilities with MORELLO_RB. */
• if (perm == DRELOC_FRAG_EXEC)

• value = cheri_sentry_create(value);
  

Nit: empty line before return.

...

• return value;

+}

+void __morello_process_dynamic_relocs(int argc __attribute__((unused)),

• 		   char **argv __attribute__((unused)),
  

• 		   char **envp __attribute__((unused)),
  

• 		   void *auxv)
  

+{

• Elf64_Rela *rela_dyn_start, *rela_dyn_end, *reloc;
• uintptr_t cap_rx = 0;
• uintptr_t cap_rw = 0;
• asm(".weak __rela_dyn_start\n\t"

•    ".hidden __rela_dyn_start\n\t"
  

•    "adrp %0, __rela_dyn_start\n\t"
  

•    "add %0, %0, #:lo12:__rela_dyn_start\n\t"
  

•    : "=C"(rela_dyn_start));
  

• asm(".weak __rela_dyn_end\n\t"

•    ".hidden __rela_dyn_end\n\t"
  

•    "adrp %0, __rela_dyn_end\n\t"
  

•    "add %0, %0, #:lo12:__rela_dyn_end"
  

•    : "=C"(rela_dyn_end));
  

• get_caps(&cap_rx, &cap_rw, auxv);
• for (reloc = rela_dyn_start; reloc < rela_dyn_end; ++reloc) {

• uintptr_t *reloc_addr, value;
  

• if (reloc->r_info != R_MORELLO_RELATIVE)
  

• 	continue;
  

• reloc_addr = (uintptr_t *)cheri_address_set(cap_rw, reloc->r_offset);
  

• value = morello_relative(0, cap_rx, cap_rw, reloc, reloc_addr);
  

• *reloc_addr = value;
  

• }

+} diff --git a/tools/testing/selftests/arm64/morello/freestanding_start.S b/tools/testing/selftests/arm64/morello/freestanding_start.S index 66dfe2bab676..af500bd715a2 100644 --- a/tools/testing/selftests/arm64/morello/freestanding_start.S +++ b/tools/testing/selftests/arm64/morello/freestanding_start.S @@ -25,7 +25,7 @@

• besides the .space we specify:
• • a space for a capability pointing to it (in the .got section)
• • a capability relocation (in __cap_relocs section) to make the capability

• • valid (see __morello_init_globals() for more details)

• • valid (see __morello_process_cap_relocs() for more details)

Same comment as in v1: I would also remove the parentheses in the line above, and here say e.g. __morello__init_{cap,dynamic}_relocs to make it clear there are two possible forms of relocations.

Yes it makes sense.

Amit

Kevin

...

• We use
• adrp c0, :got:name
• ldr c0, [c0, :got_lo12:name]

@@ -72,7 +72,8 @@ FUNCTION_START(_start) mov c22, c2 mov c23, c3

• bl __morello_init_globals

• bl __morello_process_cap_relocs
• bl __morello_process_dynamic_relocs

/* save the initial kernel stack to a global variable */ got_get_ptr stack_from_kernel, c0