The __morello_thread_init_user helper requires setting values in several system control registers. This is currently achieved by a series of inline assembly instructions; this patch defines the read_sysreg_c, write_sysreg_c, and clear_sysreg_c helpers to simplify the __morello_thread_init_user helper, and replaces the original inline assembly with calls to these helpers.

Signed-off-by: Akram Ahmad Akram.Ahmad@arm.com --- arch/arm64/include/asm/morello.h | 16 ++++++++++++++++ arch/arm64/kernel/morello.c | 18 ++++++++++++------ 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/morello.h b/arch/arm64/include/asm/morello.h index 357f403d073d..4cc7822fb77a 100644 --- a/arch/arm64/include/asm/morello.h +++ b/arch/arm64/include/asm/morello.h @@ -27,6 +27,22 @@ struct morello_state { unsigned long cctlr; };

+#define read_sysreg_c(r) ({ \ + uintcap_t __cap; \ + asm volatile("mrs %0, " __stringify(r) : "=r" (__cap)); \ + __cap; \ +}) + +#define write_sysreg_c(v, r) do { \ + uintcap_t __cap = (uintcap_t)(v); \ + asm volatile("msr " __stringify(r) ", %x0" \ + : : "rZ" (__cap)); \ +} while (0) + +#define clear_sysreg_c(r) ({ \ + write_sysreg_c(0, r); \ +}) + void morello_cap_get_val_tag(uintcap_t cap, __uint128_t *val, u8 *tag);

/* diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index 644edfd44b46..a32780f9af4d 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -20,6 +20,13 @@ #include <asm/morello.h> #include <asm/ptrace.h>

+#define CID_EL0 cid_el0 +#define DDC_EL0 ddc_el0 +#define RDDC_EL0 rddc_el0 +#define CTPIDR_EL0 ctpidr_el0 +#define RCTPIDR_EL0 rctpidr_el0 +#define CCTLR_EL0 cctlr_el0 + void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val, u8 *tag) { @@ -53,12 +60,11 @@ void __morello_thread_init_user(struct task_struct *tsk, uintcap_t ddc) * RDDC is set to a null capability as processes are always started in * Executive. */ - asm("msr rctpidr_el0, czr\n\t" - "msr ddc_el0, %0\n\t" - "msr rddc_el0, czr\n\t" - "msr cid_el0, czr\n\t" - "msr cctlr_el0, xzr" - : /* outputs */ :/* inputs */ "r"(ddc) : /*clobbers */); + clear_sysreg_c(RCTPIDR_EL0); + clear_sysreg_c(CID_EL0); + clear_sysreg_c(RDDC_EL0); + write_sysreg_c(ddc, DDC_EL0); + write_sysreg(0, CCTLR_EL0); }

static uintcap_t morello_sentry_unsealcap __ro_after_init;

On 12/02/2024 16:54, Akram Ahmad wrote:

Many of the Morello helper functions are implemented in assembly due to historical reasons. This patch rewrites the following helpers in C to improve their readability:

• __morello_cap_lo_hi_tag
• __morello_cap_has_executive
• __morello_merge_c_x
• __morello_thread_init_user

The split between patch 1 and 3 currently seems to be that patch 1 takes care of private helpers and patch 3 of the rest. It makes some sense, but it doesn't improve readability much compared to just one patch. A better logical split might be between general helpers and those that context-switch / initialise morello_user_state, since the latter are pretty similar to one another (and the most complex).

Signed-off-by: Akram Ahmad Akram.Ahmad@arm.com

arch/arm64/kernel/morello.c | 44 ++++++++++++++++++++++++---- arch/arm64/lib/morello.S | 58 ------------------------------------- 2 files changed, 39 insertions(+), 63 deletions(-)

diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index e3f732f4c52d..644edfd44b46 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -20,12 +20,46 @@ #include <asm/morello.h> #include <asm/ptrace.h> -/* Private functions implemented in morello.S */ void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val,

Helpers that are only used inside morello.c should now be static. Until now they were implemented in a different translation unit so they had to be declared global.

• 	     u8 *tag);
  

-void __morello_merge_c_x(uintcap_t *creg, u64 xreg); -bool __morello_cap_has_executive(uintcap_t cap); -void __morello_thread_init_user(struct task_struct *tsk, uintcap_t ddc);

• 	     u8 *tag)
  

+{

• *lo_val = cap;

Let's use an explicit cast to u64 for clarity.

• *hi_val = __builtin_cheri_copy_from_high((void __capability *)cap);

The CHERI builtins are normally polymorphic: they accept both void * __capability and uintcap_t, and those returning a capability (e.g. cheri_address_set()) return the same type they are passed. It looks like this particular one still only accepts void * __capability (I've reported that), but it should be possible to remove the casts when calling the other builtins.

Another note: when using __capability explicitly, always use it in postfix position: void * __capability, and not void __capability *. The latter is deprecated and ambiguous in complex cases, it just happens that we need it for __user.

• *tag = cheri_tag_get((void __capability *)&cap);

&cap is a (64-bit) pointer to cap, which will end up being stored on the stack. Casting it to void * __capability creates a capability out of it, derived from the kernel's DDC. Probably not what you meant to do :)

+}

+void __morello_merge_c_x(uintcap_t *creg, u64 xreg) +{

• if (cheri_address_get(*creg) != xreg)

• *creg = (uintcap_t)cheri_address_set(*creg, xreg);
  

No need for a cast when assigning either, as the polymorphic builtins return the same type they are passed (uintcap_t here).

+}

+bool __morello_cap_has_executive(uintcap_t cap) +{

• return cheri_perms_get((void __capability *)cap) & MORELLO_CAP_PERM_EXECUTIVE_MASK;

Let's use the standard permission values from cheriintrin.h, ARM_CAP_PERMISSION_EXECUTIVE here. We still need MORELLO_CAP_PERM_EXECUTIVE_MASK for the assembly macros, but we could move its definition inside the #ifdef __ASSEMBLY__  in morello.h to avoid confusion.

+}

+void __morello_thread_init_user(struct task_struct *tsk, uintcap_t ddc)

This function exists because morello_thread_init_user() needed an assembly helper. Now that we're writing everything in C, we don't need it any more - just fold into morello_thread_init_user().

+{

• tsk->thread.morello_user_state.ddc = ddc;
• tsk->thread.morello_user_state.rddc = (uintcap_t)as_user_ptr(0);

(uintcap_t)0 is good enough, no need to use user pointer helpers here.

• tsk->thread.morello_user_state.cid = (uintcap_t)as_user_ptr(0);
• tsk->thread.morello_user_state.cctlr = 0;

Nit: for all these functions that access a bunch of fields in morello_user_state, it could be a bit more readable with a local pointer:

struct morello_state *morello_state = &tsk->thread.morello_user_state;

That would also be consistent with morello_flush_cap_regs_to_64_regs().

• /*

• * tls_thread_flush() does not touch rctpidr_el0 so this must be zeroed
  

• * here. We do not need to initialise its value in morello_user_state.
  

• * Only the ddc_el0 register must be initialised to the specific value;
  

• * RDDC is set to a null capability as processes are always started in
  

• * Executive.
  

• */
  

• asm("msr rctpidr_el0, czr\n\t"

•    "msr ddc_el0, %0\n\t"
  

•    "msr rddc_el0, czr\n\t"
  

•    "msr cid_el0, czr\n\t"
  

•    "msr cctlr_el0, xzr"
  

•    : /* outputs */ :/* inputs */ "r"(ddc) : /*clobbers */);
  

Nit: I'd rather keep the same order as in the assembly (i.e. intermixing setting registers and morello_user_state).

Kevin

+} static uintcap_t morello_sentry_unsealcap __ro_after_init; diff --git a/arch/arm64/lib/morello.S b/arch/arm64/lib/morello.S index 0800375ef33d..5a4a39badfe2 100644 --- a/arch/arm64/lib/morello.S +++ b/arch/arm64/lib/morello.S @@ -46,38 +46,6 @@ SYM_FUNC_START(morello_build_any_user_cap) ret SYM_FUNC_END(morello_build_any_user_cap) -SYM_FUNC_START(__morello_thread_init_user)

• mov x9, #THREAD_MORELLO_USER_STATE
• add x0, x0, x9 // x0 = tsk->thread.morello_user_state
• /*

• * CTPIDR doesn't need to be initialised explicitly:
  

• * - tls_thread_flush() already zeroes tpidr_el0, zeroing ctpidr_el0 as
  

• *   well
  

• * - The value stored in thread.morello_user_state will be set the next
  

• *   time task_save_user_tls() is called, like thread_struct.uw.tp_value.
  

• *
  

• * tls_thread_flush() does not touch rcsp_el0, so we need to zero it
  

• * here, but its value in morello_user_state does not need to be
  

• * initialised here either.
  

• */
  

• msr rctpidr_el0, czr
• /* DDC: initialised to the specified value */
• msr ddc_el0, c1
• /* RDDC: null capability (processes are always started in Executive) */
• msr rddc_el0, czr
• stp c1, czr, [x0, #MORELLO_STATE_DDC]
• /* CID: null capability */
• msr cid_el0, czr
• str czr, [x0, #MORELLO_STATE_CID]
• /* CCTLR: all bits cleared */
• msr cctlr_el0, xzr
• str xzr, [x0, #MORELLO_STATE_CCTLR]
• ret

-SYM_FUNC_END(__morello_thread_init_user)

SYM_FUNC_START(morello_thread_save_user_state) mov x9, #THREAD_MORELLO_USER_STATE add x0, x0, x9 // x0 = tsk->thread.morello_user_state @@ -177,29 +145,3 @@ SYM_FUNC_START(morello_task_restore_user_tls) ret SYM_FUNC_END(morello_task_restore_user_tls)

-SYM_FUNC_START(__morello_cap_lo_hi_tag)

• str x0, [x1]
• cfhi x4, c0 // Extract upper 64 bits
• str x4, [x2]
• gctag x4, c0
• strb w4, [x3]
• ret

-SYM_FUNC_END(__morello_cap_lo_hi_tag)

-SYM_FUNC_START(__morello_merge_c_x)

• ldr c2, [x0]
• cmp x2, x1
• b.eq 1f
• scvalue c2, c2, x1
• str c2, [x0]

-1:

• ret

-SYM_FUNC_END(__morello_merge_c_x)

-SYM_FUNC_START(__morello_cap_has_executive)

• gcperm x0, c0
• ubfx x0, x0, #MORELLO_CAP_PERM_EXECUTIVE_BIT, #1
• ret

-SYM_FUNC_END(__morello_cap_has_executive)

On 12/02/2024 16:54, Akram Ahmad wrote:

This patch translates the following Morello helpers, which are currently written in assembly in arch/arm64/lib/morello.S, to C functions which are now defined in arch/arm64/kernel/morello.c:

• morello_cap_get_val_tag
• morello_build_any_user_cap
• morello_task_{save, restore}_user_tls

As these were all the remaining assembly functions in morello.S, this file has now been removed and the Makefile edited accordingly.

Signed-off-by: Akram Ahmad Akram.Ahmad@arm.com

arch/arm64/kernel/morello.c | 58 ++++++++++++++ arch/arm64/lib/Makefile | 3 +- arch/arm64/lib/morello.S | 147 ------------------------------------ 3 files changed, 59 insertions(+), 149 deletions(-) delete mode 100644 arch/arm64/lib/morello.S

diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index a32780f9af4d..d4be535c8057 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -75,6 +75,21 @@ static uintcap_t morello_sentry_unsealcap __ro_after_init; #define CAP_OTYPE_FIELD_BITS 15 +void morello_cap_get_val_tag(uintcap_t cap, __uint128_t *val, u8 *tag) +{

• *tag = cheri_tag_get((void __capability *)&cap);
• *val = (__uint128_t)__builtin_cheri_copy_from_high((void __capability *)cap) << 64;
• *val = *val | (__uint128_t)cheri_address_get((void __capability *)cap);

This should indeed be functionally equivalent to the assembly, albeit quite a bit more complicated.

I would however suggest we keep the same approach as in assembly. It may not be obvious what that approach is so let me spell it out: we are not considering val as a pointer to __uint128_t, but as a pointer to an (untagged) capability. This is what __uint128_t is really meant to represent here: the 128-bit "data" part of the capability, without the tag. The compiler doesn't really support that though, so in C we'd have to do something like *((uintcap_t *)val) = cheri_tag_clear(cap). This could potentially violate strict aliasing [1], but 1. this is generally allowed in the kernel, and 2. in practice *val is never accessed as a __uint128_t.

Just a note on why we're using __uint128_t at all: this function (and the next one) are used for manipulating ptrace uapi structs. We do not want to use any CHERI type in these structs, as e.g. a debugger should not need to use CHERI at all itself. Capabilities are not be propagated as-is through ptrace, instead they're split into data and tag. See also [2].

+}

+uintcap_t morello_build_any_user_cap(const __uint128_t *val, u8 tag) +{

• if (!tag)

• return (uintcap_t)cheri_tag_clear(uaddr_to_user_ptr((u64)*val));
  

• return (uintcap_t)__builtin_cheri_copy_to_high(uaddr_to_user_ptr((u64)*val), *val >> 64);

CTHI (copy to high) always clears the tag, because it arbitrarily modifies the metadata and this is fundamentally incompatible with capability integrity.

The instruction sequence should remain pretty much the same as in assembly. As above val should be interpreted as __uintcap_t *, and the special instructions used to rebuild the capability can be generated using those builtins / wrappers [3].

[1] https://en.cppreference.com/w/c/language/object [2] https://git.morello-project.org/morello/kernel/linux/-/blob/morello/next/Doc... [3] https://git.morello-project.org/morello/llvm-project/-/blob/morello/master/c...

+}

static bool is_pure_task(void) { return IS_ENABLED(CONFIG_CHERI_PURECAP_UABI) && !is_compat_task(); @@ -112,6 +127,49 @@ void morello_thread_start(struct pt_regs *regs, unsigned long pc) /* CSP is null-derived in hybrid */ } } +void morello_thread_save_user_state(struct task_struct *tsk) +{

• /* (R)CTPIDR is handled by task_save_user_tls */
• tsk->thread.morello_user_state.ddc = read_sysreg_c(DDC_EL0);
• tsk->thread.morello_user_state.rddc = read_sysreg_c(RDDC_EL0);
• tsk->thread.morello_user_state.cid = read_sysreg_c(CID_EL0);
• tsk->thread.morello_user_state.cctlr = read_sysreg(CCTLR_EL0);

+}

+void morello_thread_restore_user_state(struct task_struct *tsk) +{

• /* (R)CTPIDR is handled by task_restore_user_tls */
• write_sysreg_c(tsk->thread.morello_user_state.ddc, DDC_EL0);
• write_sysreg_c(tsk->thread.morello_user_state.rddc, RDDC_EL0);
• write_sysreg_c(tsk->thread.morello_user_state.cid, CID_EL0);
• write_sysreg(tsk->thread.morello_user_state.cctlr, CCTLR_EL0);

+}

+void morello_task_save_user_tls(struct task_struct *tsk, user_uintptr_t *tp_ptr) +{

• tsk->thread.morello_user_state.ctpidr = read_sysreg_c(CTPIDR_EL0);
• tsk->thread.morello_user_state.rctpidr = read_sysreg_c(RCTPIDR_EL0);
• if (!__morello_cap_has_executive(task_pt_regs(tsk)->pcc))

Nit: with an if / else, it's generally preferred not to negate the condition, as it's easier not to have to think about the double negation in the else case. Not an absolute rule, but it's clearly easier to read without the negation in this case (and below).

• *tp_ptr = read_sysreg_c(RCTPIDR_EL0);
  

Nit: let's cast to user_uintptr_t for clarity. It's superfluous in PCuABI since user_uintptr_t is uintcap_t, but in !PCuABI user_uintptr_t is still u64 and these assignments are actually truncating the capability to just its address.

• else

• *tp_ptr = read_sysreg_c(CTPIDR_EL0);
  

+}

+void morello_task_restore_user_tls(struct task_struct *tsk,

• 		   const user_uintptr_t *tp_ptr)
  

+{

• uintcap_t *ctpidr = &tsk->thread.morello_user_state.ctpidr;
• uintcap_t *rctpidr = &tsk->thread.morello_user_state.rctpidr;
• if (!__morello_cap_has_executive(task_pt_regs(tsk)->pcc))

• *rctpidr = *tp_ptr;
  

• else

• *ctpidr = *tp_ptr;
  

We still need to perform the X/C register merging in !PCuABI, it should be similar to the assembly version (using __morello_merge_c_x()). Unfortunately we can't get rid of the #ifdef here. However we can avoid nesting it inside the if / else by having another variable track the register we need to update:

uintcap_t *active_ctpidr = pcc_has_executive ? ctpidr : rctpidr;

morello_flush_cap_regs_to_64_regs() uses a similar approach.

• write_sysreg_c(*rctpidr, RCTPIDR_EL0);
• write_sysreg_c(*ctpidr, CTPIDR_EL0);

+} void morello_thread_init_user(void) { diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile index 5fa6f0645edc..2e46ed437b69 100644 --- a/arch/arm64/lib/Makefile +++ b/arch/arm64/lib/Makefile @@ -27,8 +27,7 @@ obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o obj-$(CONFIG_ARM64_MTE) += mte.o -obj-$(CONFIG_ARM64_MORELLO) += morello.o \

• 	       copy_from_user_with_captags.o \
  

+obj-$(CONFIG_ARM64_MORELLO) += copy_from_user_with_captags.o \ copy_to_user_with_captags.o obj-$(CONFIG_KASAN_SW_TAGS) += kasan_sw_tags.o diff --git a/arch/arm64/lib/morello.S b/arch/arm64/lib/morello.S deleted file mode 100644 index 5a4a39badfe2..000000000000 --- a/arch/arm64/lib/morello.S +++ /dev/null @@ -1,147 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/*

• • Copyright (C) 2020 Arm Ltd.
• • TODO: most of the functions in this file should be reimplemented in C to
• • improve readability.
• */

-#include <linux/linkage.h>

-#include <asm/asm-uaccess.h> -#include <asm/assembler.h> -#include <asm/morello.h> -#include <asm/sysreg.h>

-SYM_FUNC_START(morello_cap_get_val_tag)

• gctag x3, c0
• clrtag c0, c0 // Store the 128-bit value without the tag
• str c0, [x1]
• strb w3, [x2]
• ret

-SYM_FUNC_END(morello_cap_get_val_tag)

-SYM_FUNC_START(morello_build_any_user_cap)

• ldr c0, [x0]
• cbz w1, 1f
• /*

• * The tag should be set, build a valid capability from the root
  

• * capability.
  

• * In case c0 is sealed (non-zero object type), we need to extract the
  

• * object type first to be able to reseal it after BUILD. The CSEAL
  

• * instruction is used to cover the case where c0 was not sealed.
  

• */
  

• adr_l x3, cheri_user_root_allperms_cap
• ldr c3, [x3]
• cpytype c4, c3, c0
• build c0, c0, c3
• cseal c0, c0, c4
• b 2f

-1:

• /* The tag should be cleared, make sure it is. */
• clrtag c0, c0

-2:

• ret

-SYM_FUNC_END(morello_build_any_user_cap)

-SYM_FUNC_START(morello_thread_save_user_state)

• mov x9, #THREAD_MORELLO_USER_STATE

All users of these offsets are now gone, so we can remove them from arch/arm64/kernel/asm-offsets.c.

• add x0, x0, x9 // x0 = tsk->thread.morello_user_state
• /* (R)CTPIDR is handled by task_save_user_tls */
• mrs c1, ddc_el0
• mrs c2, rddc_el0
• stp c1, c2, [x0, #MORELLO_STATE_DDC]
• mrs c1, cid_el0
• str c1, [x0, #MORELLO_STATE_CID]
• mrs x1, cctlr_el0
• str x1, [x0, #MORELLO_STATE_CCTLR]
• ret

-SYM_FUNC_END(morello_thread_save_user_state)

-SYM_FUNC_START(morello_thread_restore_user_state)

• mov x9, #THREAD_MORELLO_USER_STATE
• add x0, x0, x9 // x0 = tsk->thread.morello_user_state
• /* (R)CTPIDR is handled by task_restore_user_tls */
• ldp c1, c2, [x0, #MORELLO_STATE_DDC]
• msr ddc_el0, c1
• msr rddc_el0, c2
• ldr c1, [x0, #MORELLO_STATE_CID]
• msr cid_el0, c1
• ldr x1, [x0, #MORELLO_STATE_CCTLR]
• msr cctlr_el0, x1
• ret

-SYM_FUNC_END(morello_thread_restore_user_state)

-SYM_FUNC_START(morello_task_save_user_tls)

• get_task_pt_regs x8, x0

get_task_pt_regs was introduced specifically for those so we can remove it from assembler.h as well.

Kevin

• mov x9, #THREAD_MORELLO_USER_STATE
• add x0, x0, x9 // x0 = tsk->thread.morello_user_state
• mrs c2, ctpidr_el0
• mrs c3, rctpidr_el0
• /* Save CTPIDR and RCTPIDR */
• stp c2, c3, [x0, #MORELLO_STATE_CTPIDR]
• ldr c5, [x8, #S_PCC]
• /*

• * If the task is currently running in Restricted, save the lower 64 bits
  

• * of RCTPIDR (RTPIDR) in tsk->thread instead of TPIDR.
  

• */
  

• morello_tst_cap_has_executive c5, x6 // Task running in Executive?

-#ifdef CONFIG_CHERI_PURECAP_UABI

• csel c2, c2, c3, ne // If not, save RTPIDR
• str c2, [x1]

-#else

• csel x2, x2, x3, ne // If not, save RTPIDR
• str x2, [x1]

-#endif

• ret

-SYM_FUNC_END(morello_task_save_user_tls)

-SYM_FUNC_START(morello_task_restore_user_tls)

• get_task_pt_regs x8, x0
• mov x9, #THREAD_MORELLO_USER_STATE
• add x0, x0, x9 // x0 = tsk->thread.morello_user_state
• /* Load CTPIDR, RCTPIDR and the 64-bit TLS pointer */
• ldp c2, c3, [x0, #MORELLO_STATE_CTPIDR]

-#ifdef CONFIG_CHERI_PURECAP_UABI

• ldr c4, [x1]

-#else

• ldr x4, [x1]

-#endif

• /*

• * The 64-bit TLS pointer may have been modified from within the kernel
  

• * (e.g. through ptrace) since morello_task_save_user_tls was called.
  

• * Merge it back into the active capability TLS pointer, that is RCTPIDR
  

• * if the task is running in Restricted and CTPIDR otherwise.
  

• */
  

• ldr c5, [x8, #S_PCC]
• morello_tst_cap_has_executive c5, x6 // Task running in Executive?
• b.eq 1f // If not, merge into RCTPIDR

-#ifdef CONFIG_CHERI_PURECAP_UABI

• mov c2, c4 // Save the TLS pointer into CTPIDR

-#else

• morello_merge_c_x 2, x4 // Merge the TLS pointer into CTPIDR

-#endif

• b 2f

-1: -#ifdef CONFIG_CHERI_PURECAP_UABI

• mov c3, c4 // Save the TLS pointer into RCTPIDR

-#else

• morello_merge_c_x 3, x4 // Merge the TLS pointer into RCTPIDR

-#endif -2:

• msr ctpidr_el0, c2
• msr rctpidr_el0, c3
• ret

-SYM_FUNC_END(morello_task_restore_user_tls)