Use stack high water mark to optimise zeroing.

This is implemented in Sail but not yet merged:
CHERIoT-Platform/cheriot-sail#8

It can be enabled by setting a board config option
'stack_high_water_mark' to true.

We need to think more about potential attacks based on unexpected
stack pointer bounds or passing pointers to stack.

Includes a fix for #62: don't rely on zero_stack setting \base to \top.

The zero_stack macro in the switcher did not meet the claim in the
comment that the base register would end up pointing to the top (it
was not true if the top was not 32-byte aligned).

Rather than fix this I've removed the claim in the comment and
adjusted the places that call it not rely on this property.

Author: ronorton

sdk/core/loader/boot.cc

@@ -811,13 +811,18 @@ namespace
 			Debug::log("New thread's trusted stack is {}", threadTStack);
 			threadTStack->mepcc = pcc;
 			threadTStack->cgp   = cgp;
-			threadTStack->csp   = allocateStack(config.stackSize);
-			Debug::log("New thread's stack is {}", threadTStack->csp);
+			auto stack          = allocateStack(config.stackSize);
+			threadTStack->csp   = stack;
+			Debug::log("New thread's stack is {}", stack);
 			// Enable previous level interrupts and set the previous exception
 			// level to M mode.
 			threadTStack->mstatus =
 			  (priv::MSTATUS_MPIE |
 			   (priv::MSTATUS_PRV_M << priv::MSTATUS_MPP_SHIFT));
+#ifdef CONFIG_MSHWM
+			threadTStack->mshwm  = stack.top();
+			threadTStack->mshwmb = stack.base();
+#endif
 			threadTStack->frameoffset = offsetof(TrustedStack, frames[1]);
 			threadTStack->frames[0].calleeExportTable =
 			  build(compartment.exportTable);

sdk/core/switcher/entry.S

@@ -6,6 +6,18 @@
 
 .include "assembly-helpers.s"
 
+#  Symbolic names for the satck high water mark registers until
+#  the assembler knows about them.
+
+/**
+ * Machine-mode stack high water mark CSR
+ */
+#define CSR_MSHWM  0xbc1
+/**
+ * Machine mode stack high water mark stack base CSR
+ */
+#define CSR_MSHWMB 0xbc2
+
 #define MAX_FAULTS_PER_COMPARTMENT_CALL 1024
 
 # Global for the sealing key.  Stored in the switcher's code section.
@@ -105,11 +117,10 @@ switcher_scheduler_entry_csp:
 .endm
 
 /**
- * Zero the stack.  The three operands are the base address (modified during
- * this call, will point at the top at the end), the top address, and a scratch
- * register to use.  The base must be a capability but it must be provided
- * without the c prefix because it is used as both a capability and integer
- * register.  Top and scratch are both clobbered.
+ * Zero the stack.  The three operands are the base address, the top address,
+ * and a scratch register to use.  The base must be a capability but it must
+ * be provided without the c prefix because it is used as both a capability
+ * and integer register.  All three registers are clobbered.
  */
 .macro zero_stack base top scratch
 	addi               \scratch, \top, -32
@@ -181,8 +192,24 @@ compartment_switcher_entry:
 	cgetbase           s1, csp
 	csetaddr           csp, csp, s1
 	sub                s1, s0, s1
-	csetboundsexact    csp, csp, s1
-	zero_stack         sp, s0, gp
+	csetboundsexact    ct2, csp, s1
+	csetaddr           csp, ct2, s0
+#ifdef CONFIG_MSHWM
+	// read and align the stack high water mark
+	csrr               gp, CSR_MSHWM
+	and                gp, gp, ~0xf
+	// skip zeroing if high water mark >= stack poitner
+	bge                t2, sp, after_zero
+	// use stack high water mark as base address for zeroing
+	// XXX could be out of bounds / unrepresentable if bad csp?
+	csetaddr           ct2, csp, gp
+#endif
+	zero_stack         t2, s0, gp
+after_zero:
+#ifdef CONFIG_MSHWM
+	// store new stack top as stack high water mark
+	csrw               CSR_MSHWM, sp
+#endif
 #endif // CONFIG_NO_SWITCHER_SAFETY
 .Lout:
 	// Fetch the sealing key
@@ -315,6 +342,12 @@ exception_entry_asm:
 	csc                ct0, TrustedStack_offset_mepcc(csp)
 	csrr               t1, mstatus
 	csw                t1, TrustedStack_offset_mstatus(csp)
+#ifdef CONFIG_MSHWM
+	csrr               t1, CSR_MSHWM
+	csw                t1, TrustedStack_offset_mshwm(csp)
+	csrr               t1, CSR_MSHWMB
+	csw                t1, TrustedStack_offset_mshwmb(csp)
+#endif
 	csrr               t1, mcause
 	csw                t1, TrustedStack_offset_mcause(csp)
 
@@ -394,6 +427,12 @@ exception_entry_asm:
 .Linstall_context:
 	clw                x1, TrustedStack_offset_mstatus(csp)
 	csrw               mstatus, x1
+#ifdef CONFIG_MSHWM
+	clw                x1, TrustedStack_offset_mshwm(csp)
+	csrw               CSR_MSHWM, x1
+	clw                x1, TrustedStack_offset_mshwmb(csp)
+	csrw               CSR_MSHWMB, x1
+#endif
 	cspecialw          mepcc, ct2
 	csb                zero, TrustedStack_offset_inForcedUnwind(csp)
 	// c2 is csp, which will be loaded last and will overwrite the trusted
@@ -694,9 +733,21 @@ exception_entry_asm:
 	// Update the current frame offset.
 	csw                t2, TrustedStack_offset_frameoffset(ctp)
 #ifndef CONFIG_NO_SWITCHER_SAFETY
+#ifdef CONFIG_MSHWM
+	// read and align the stack high water mark
+	// we will use this as base address for stack clearing
+	// note that it cannot be greater than stack top as we 
+	// we set it to stack top when we pushed to trusted stack frame
+	csrr               tp, CSR_MSHWM
+	and                tp, tp, ~0xf
+#else
 	cgetbase           tp, csp
+#endif
 	cgetaddr           t1, csp
 	csetaddr           ct2, csp, tp
 	zero_stack         t2, t1, tp
+#ifdef CONFIG_MSHWM
+	csrw               CSR_MSHWM, sp
+#endif
 #endif // CONFIG_NO_SWITCHER_SAFETY
 	cret

sdk/core/switcher/trusted-stack-assembly.h

@@ -21,10 +21,20 @@ EXPORT_ASSEMBLY_OFFSET(TrustedStack, c14, 14 * 8)
 EXPORT_ASSEMBLY_OFFSET(TrustedStack, c15, 15 * 8)
 EXPORT_ASSEMBLY_OFFSET(TrustedStack, mstatus, 16 * 8)
 EXPORT_ASSEMBLY_OFFSET(TrustedStack, mcause, (16 * 8) + 4)
+#ifdef CONFIG_MSHWM
+EXPORT_ASSEMBLY_OFFSET(TrustedStack, mshwm, 17 * 8)
+EXPORT_ASSEMBLY_OFFSET(TrustedStack, mshwmb, (17 * 8) + 4)
+
+// Size of everything up to this point
+#define TSTACK_REGFRAME_SZ (18 * 8)
+// frameoffset, inForcedUnwind and padding
+#define TSTACK_HEADER_SZ 16
+#else
 // Size of everything up to this point
-#define TSTACK_REGFRAME_SZ ((16 * 8) + (2 * 4))
+#define TSTACK_REGFRAME_SZ ((16 * 8) + (2* 4))
 // frameoffset, inForcedUnwind and padding
 #define TSTACK_HEADER_SZ 8
+#endif
 // The basic trusted stack is the size of the save area, 8 bytes of state for
 // unwinding information, and then a single trusted stack frame used for the
 // unwind state of the initial thread. (7 * 8) is the size of TrustedStackFrame

sdk/core/switcher/tstack.h

@@ -44,33 +44,42 @@ struct TrustedStackFrame
 template<size_t NFrames>
 struct TrustedStackGeneric
 {
-	void    *mepcc;
-	void    *c1;
-	void    *csp;
-	void    *cgp;
-	void    *c4;
-	void    *c5;
-	void    *c6;
-	void    *c7;
-	void    *c8;
-	void    *c9;
-	void    *c10;
-	void    *c11;
-	void    *c12;
-	void    *c13;
-	void    *c14;
-	void    *c15;
-	size_t   mstatus;
-	size_t   mcause;
+	void  *mepcc;
+	void  *c1;
+	void  *csp;
+	void  *cgp;
+	void  *c4;
+	void  *c5;
+	void  *c6;
+	void  *c7;
+	void  *c8;
+	void  *c9;
+	void  *c10;
+	void  *c11;
+	void  *c12;
+	void  *c13;
+	void  *c14;
+	void  *c15;
+	size_t mstatus;
+	size_t mcause;
+#ifdef CONFIG_MSHWM
+	uint32_t mshwm;
+	uint32_t mshwmb;
+#endif
 	uint16_t frameoffset;
 	/**
 	 * Flag indicating whether this thread is in the process of a forced
 	 * unwind.  If so, this is one, otherwise it is zero.
 	 */
 	uint8_t inForcedUnwind;
 	// Padding up to multiple of 16-bytes.
-	uint8_t  pad0;
-	uint16_t padding[2];
+	uint8_t padding[
+#ifdef CONFIG_MSHWM
+	  13
+#else
+	  5
+#endif
+	];
 	/**
 	 * The trusted stack.  There is always one frame, describing the entry
 	 * point.  If this is popped then we have run off the stack and the thread

sdk/xmake.lua

@@ -232,6 +232,11 @@ rule("firmware")
 			add_defines("SIMULATION")
 		end
 
+		if board.stack_high_water_mark then
+			print("using stack high water mark")
+			add_defines("CONFIG_MSHWM")
+		end
+
 		-- Build the MMIO space for the board
 		local mmio = ""
 		local mmio_start = 0xffffffff