Fix revoker scan regions.

Neither software or hardware revoker were scanning enough of memory,
as revealed by the tests in the previous commit. To fix this we move
all the mutable data (which needs to be swept) into a single contigous
range bounded by __revoker_scan_start and __export_mem_heap_end.

The software revoker is currently set up to scan multiple disjoint
regions (stack, heap etc) but we now only need to use one, which is
not the same as the hardware revoker. This revealed a problem in the
loader where it was not using imprecise set bounds as intended.

sdk/core/loader/boot.cc

@@ -1392,57 +1392,43 @@ extern "C" SchedulerEntryInfo loader_entry_point(const ImgHdr &imgHdr,
 	  csp);
 
 #ifdef SOFTWARE_REVOKER
-	// If we are using a software revoker then we need to provide it with three
+	// If we are using a software revoker then we need to provide it with some
 	// terrifyingly powerful capabilities.  These break some of the rules that
-	// we enforce for everything else, especially the last one, which is a
-	// stack capability that is reachable from a global.  The only code that
+	// we enforce for everything else (e.g. they may point to stacks
+	// but are reachable from a global).  The only code that
 	// accesses these in the revoker is very small and amenable to auditing
 	// (the only memory accesses are a load and a store back at the same
 	// location, with interrupts disabled, to trigger the load barrier).
-	//
-	// We use imprecise set-bounds operations here because we need to ensure
-	// that the regions are completely scanned and scanning slightly more is
-	// not a problem unless the revoker is compromised.  The software revoker
-	// already has a terrifying set of rights, so this doesn't really make
-	// things worse and is another good reason to use a hardware revoker.
-	// Given that hardware revokers are lower power, faster, and more secure,
-	// there's little reason for the software revoker to be used for anything
-	// other than testing.
+
+	// The scary capabilities are stored at the beginning of the software
+	// revoker compartment. At the moment we only need one.
 	auto scaryCapabilities = build<Capability<void>,
 	                               Root::Type::RWStoreL,
 	                               Root::Permissions<Root::Type::RWStoreL>,
-	                               /* Precise: */ false>(
+	                               /* Precise: */ true>(
 	  imgHdr.privilegedCompartments.software_revoker().code.start(),
-	  3 * sizeof(void *));
-	// Read-write capability to all globals.  This is scary because a bug in
-	// the revoker could violate compartment isolation.
+	  sizeof(void *));
 	Debug::log("Writing scary capabilities for software revoker to {}",
 	           scaryCapabilities);
-	scaryCapabilities[0] =
-	  build(LA_ABS(__compart_cgps),
-	        LA_ABS(__compart_cgps_end) - LA_ABS(__compart_cgps));
+	// Construct a capability to all RW globals, stacks and heap.
+	// We use an imprecise set-bounds operation here because we need to ensure
+	// that the region is completely scanned and scanning slightly more is
+	// not a problem unless the revoker is compromised.  The software revoker
+	// already has a terrifying set of rights, so this doesn't really make
+	// things worse and is another good reason to use a hardware revoker.
+	// Given that hardware revokers are lower power, faster, and more secure,
+	// there's little reason for the software revoker to be used for anything
+	// other than testing.
+	scaryCapabilities[0] = build<void,
+	                             Root::Type::RWStoreL,
+	                             Root::Permissions<Root::Type::RWStoreL>,
+	                             /* Precise: */ false>(
+	  LA_ABS(__revoker_scan_start),
+	  LA_ABS(__export_mem_heap_end) - LA_ABS(__revoker_scan_start));
 	scaryCapabilities[0].address() = scaryCapabilities[0].base();
-	Debug::log("Wrote scary capability {}", scaryCapabilities[0]);
-	// Read-write capability to the whole heap.  This is scary because a bug in
-	// the revoker could violate heap safety.
-	scaryCapabilities[1] =
-	  build<void,
-	        Root::Type::RWGlobal,
-	        Root::Permissions<Root::Type::RWGlobal>,
-	        false>(LA_ABS(__export_mem_heap),
-	               LA_ABS(__export_mem_heap_end) - LA_ABS(__export_mem_heap));
-	scaryCapabilities[1].address() = scaryCapabilities[1].base();
-	Debug::log("Wrote scary capability {}", scaryCapabilities[1]);
-	// Read-write capability to the entire stack.  This is scary because a bug
-	// in the revoker could violate thread isolation.
-	scaryCapabilities[2] =
-	  build<void,
-	        Root::Type::RWStoreL,
-	        Root::Permissions<Root::Type::RWStoreL>,
-	        false>(LA_ABS(__stack_space_start),
-	               LA_ABS(__stack_space_end) - LA_ABS(__stack_space_start));
-	scaryCapabilities[2].address() = scaryCapabilities[2].base();
-	Debug::log("Wrote scary capability {}", scaryCapabilities[2]);
+	Debug::log("Wrote scary cap[0]={} requested_start={}",
+	           scaryCapabilities[0],
+	           LA_ABS(__revoker_scan_start));
 #endif
 
 	// Set up the exception entry point

sdk/core/software_revoker/revoker.cc

@@ -11,15 +11,14 @@ using CHERI::Capability;
 using CHERI::Permission;
 
 /**
- * We need an array of the three allocations that provide the globals at the
+ * We need an array of the allocations that provide the globals at the
  * start of our PCC, but the compiler doesn't currently provide a good way of
- * doing this, so do it with an assembly stub for loading the three
- * capabilities.
+ * doing this, so do it with an assembly stub for loading the capabilities.
  */
 __asm__("	.section .text, \"ax\", @progbits\n"
         "	.p2align 3\n"
         "globals:\n"
-        "	.zero 3*8\n"
+        "	.zero 1*8\n"
         "	.globl get_globals\n"
         "get_globals:\n"
         "	sll        a0, a0, 3\n"
@@ -35,7 +34,7 @@ extern "C" volatile void *volatile *get_globals(int idx);
 namespace
 {
 	/**
-	 * The index of the current range to scan.  Must be 0-2 or negative.
+	 * The index of the current range to scan.  Must be 0 or -1.
 	 */
 	int currentRange;
 	/**
@@ -62,18 +61,9 @@ namespace
 		 */
 		NotRunning,
 		/**
-		 * The revoker is scanning globals.
+		 * The revoker is scanning.
 		 */
-		ScanningGlobals,
-		/**
-		 * The revoker is scanning the heap.
-		 */
-		ScanningHeap,
-		/**
-		 * The revoker is scanning stacks (including trusted stacks and the
-		 * register-save areas).
-		 */
-		ScanningStacks
+		Scanning
 	} state;
 
 	/**
@@ -84,12 +74,8 @@ namespace
 		switch (state)
 		{
 			case State::NotRunning:
-				return {0, State::ScanningGlobals};
-			case State::ScanningGlobals:
-				return {1, State::ScanningHeap};
-			case State::ScanningHeap:
-				return {2, State::ScanningStacks};
-			case State::ScanningStacks:
+				return {0, State::Scanning};
+			case State::Scanning:
 				return {-1, State::NotRunning};
 		}
 	}

sdk/firmware.ldscript.in

@@ -13,12 +13,6 @@ SECTIONS
 		*(.loader_start);
 	}
 
-	@thread_trusted_stacks@
-
-	__stack_space_start = .;
-	@thread_stacks@
-	__stack_space_end = .;
-
 	.compartment_export_tables : ALIGN(8)
 	{
 		# The scheduler and allocator's export tables are at the start.
@@ -83,9 +77,19 @@ SECTIONS
 	@software_revoker_code@
 
 	@pcc_ld@
-
 	__compart_pccs_end = .;
 
+	# Revoker scan region must be cap aligned
+	. = ALIGN(8);
+	__revoker_scan_start = .;
+	__stack_space_start = .;
+
+	@thread_trusted_stacks@
+
+	@thread_stacks@
+
+	__stack_space_end = .;
+
 	__compart_cgps = ALIGN(64);
 
 	.scheduler_globals : CAPALIGN

sdk/include/platform/ibex/platform-hardware_revoker.hh

@@ -87,9 +87,9 @@ namespace Ibex
 			 * revoke capabilities everywhere from the start of compartment
 			 * globals to the end of the heap.
 			 */
-			extern char __compart_cgps, __export_mem_heap_end;
+			extern char __revoker_scan_start, __export_mem_heap_end;
 
-			auto  base   = LA_ABS(__compart_cgps);
+			auto  base   = LA_ABS(__revoker_scan_start);
 			auto  top    = LA_ABS(__export_mem_heap_end);
 			auto &device = revoker_device();
 			device.base  = base;