CHERIoT SAFE 33MHz fails to catch some spatial memory safety violations

[hlef]

It seems that CHERIoT SAFE on the Arty A7 fails to catch some spatial safety violations.

Here is a proof of concept:

diff --git a/examples/01.hello_world/hello.cc b/examples/01.hello_world/hello.cc
index 17ec80c..d3a72ae 100644
--- a/examples/01.hello_world/hello.cc
+++ b/examples/01.hello_world/hello.cc
@@ -11,6 +11,18 @@ using Debug = ConditionalDebug<true, "Hello world compartment">;
 /// Thread entry point.
 void __cheri_compartment("hello") say_hello()
 {
-       // Print hello world, along with the compartment's name to the default UART.
-       Debug::log("Hello world");
+   Debug::log("Corrupting the allocator.");
+   struct Timeout    timeout = {0, UnlimitedTimeout};
+   volatile uint8_t *x1 =
+     (uint8_t *)heap_allocate(&timeout, MALLOC_CAPABILITY, 0x20);
+   Debug::log("Got capability {}", x1);
+   Debug::log("Capability after addition {}", (volatile void **)(x1 + 0x20));
+   *(volatile void **)(x1 + 0x20) = (void *)-1;
+   Debug::log("Done. Now running the allocator to get an assert.");
+   for (int i = 0; i < 10; i++)
+   {
+       auto p = heap_allocate(&timeout, MALLOC_CAPABILITY, 10);
+       heap_free(MALLOC_CAPABILITY, p);
+   }
+   Debug::log("Exiting.");
 }
diff --git a/examples/01.hello_world/xmake.lua b/examples/01.hello_world/xmake.lua
index 3483e64..69e8898 100644
--- a/examples/01.hello_world/xmake.lua
+++ b/examples/01.hello_world/xmake.lua
@@ -27,8 +27,8 @@ firmware("hello_world")
                 compartment = "hello",
                 priority = 1,
                 entry_point = "say_hello",
-                stack_size = 0x200,
-                trusted_stack_frames = 1
+                stack_size = 0x1000,
+                trusted_stack_frames = 2
             }
         }, {expand = false})
     end)

Just patch this into the CHERIoT RTOS repo (https://github.com/microsoft/cheriot-rtos), and build 01.hello_world for 33MHz with debugging enabled for the memory allocator. Running it on the Arty A7, you should see the following:

Ready to load firmware, hold BTN0 to ignore UART input.                              

Ready to load firmware, hold BTN0 to ignore UART input.                              
Starting loading.  First word was: 40812A15                                          
.................................................                                    
Finished loading.  Last word was: 020001F4                                           
Number of words loaded to IRAM: 000030AC                                             
Loaded firmware, jumping to IRAM.                                                    

Hello world compartment: Corrupting the allocator.                                   
Hello world compartment: Got capability 0x20049510 (v:1 0x20049510-0x20049530 l:0x20 o:0x0 p:
 G RWcgm- -- ---)                                                                    
Hello world compartment: Capability after addition 0x20049530 (v:1 0x20049510-0x20049530 l:0x
20 o:0x0 p: G RWcgm- -- ---)                                                         
Hello world compartment: Done. Now running the allocator to get an assert.           
../../sdk/core/allocator/alloc.h:2253 Assertion failure in tmalloc_smallest          
Free chunk 0x20049538 (v:1 0x20049400-0x20080000 l:0x36c00 o:0x0 p: G RWcgm- -- ---) follows 
another free chunk                                                                   
../../sdk/core/allocator/alloc.h:2253 Assertion failure in tmalloc_smallest
Free chunk 0x20049538 (v:1 0x20049400-0x20080000 l:0x36c00 o:0x0 p: G RWcgm- -- ---) follows 
another free chunk
../../sdk/core/allocator/alloc.h:2253 Assertion failure in tmalloc_smallest
Free chunk 0x20049538 (v:1 0x20049400-0x20080000 l:0x36c00 o:0x0 p: G RWcgm- -- ---) follows 
another free chunk
../../sdk/core/allocator/alloc.h:2253 Assertion failure in tmalloc_smallest
Free chunk 0x20049538 (v:1 0x20049400-0x20080000 l:0x36c00 o:0x0 p: G RWcgm- -- ---) follows 
another free chunk
../../sdk/core/allocator/alloc.h:2253 Assertion failure in tmalloc_smallest
Free chunk 0x20049538 (v:1 0x20049400-0x20080000 l:0x36c00 o:0x0 p: G RWcgm- -- ---) follows 
another free chunk
../../sdk/core/allocator/alloc.h:2253 Assertion failure in tmalloc_smallest
Free chunk 0x20049538 (v:1 0x20049400-0x20080000 l:0x36c00 o:0x0 p: G RWcgm- -- ---) follows 
another free chunk
../../sdk/core/allocator/alloc.h:2253 Assertion failure in tmalloc_smallest
Free chunk 0x20049538 (v:1 0x20049400-0x20080000 l:0x36c00 o:0x0 p: G RWcgm- -- ---) follows 
another free chunk
../../sdk/core/allocator/alloc.h:2253 Assertion failure in tmalloc_smallest
Free chunk 0x20049538 (v:1 0x20049400-0x20080000 l:0x36c00 o:0x0 p: G RWcgm- -- ---) follows 
another free chunk
../../sdk/core/allocator/alloc.h:2253 Assertion failure in tmalloc_smallest
Free chunk 0x20049538 (v:1 0x20049400-0x20080000 l:0x36c00 o:0x0 p: G RWcgm- -- ---) follows 
another free chunk
../../sdk/core/allocator/alloc.h:2253 Assertion failure in tmalloc_smallest
Free chunk 0x20049538 (v:1 0x20049400-0x20080000 l:0x36c00 o:0x0 p: G RWcgm- -- ---) follows 
another free chunk
Hello world compartment: Exiting.
Error handler: BoundsViolation(0x1) error at 0x2004862e (v:0 0x20048360-0x20048f80 l:0xc20 o:
0x0 p: G R-cgm- X- ---) (return address: 0x0 (v:0 0x0-0x0 l:0x0 o:0x0 p: - ------ -- ---)), w
ith capability register CRA(0x1): 0x0 (v:0 0x0-0x0 l:0x0 o:0x0 p: - ------ -- ---)

The overflow is not detected and it corrupts the allocator which gets some assertion violations (!).

I seem to be able to get similar behavior on the stack:

diff --git a/examples/01.hello_world/hello.cc b/examples/01.hello_world/hello.cc
index 17ec80c..e7e40b5 100644
--- a/examples/01.hello_world/hello.cc
+++ b/examples/01.hello_world/hello.cc
@@ -11,6 +11,28 @@ using Debug = ConditionalDebug<true, "Hello world compartment">;
 /// Thread entry point.
 void __cheri_compartment("hello") say_hello()
 {
-   // Print hello world, along with the compartment's name to the default UART.
-   Debug::log("Hello world");
+   Debug::log("Corrupting a stack value.");
+   uint8_t a[12] = {0};
+   CHERI::Capability<uint8_t> cap{a};
+   cap.bounds() = 2;
+   volatile uint8_t *b = cap;
+   Debug::log("a {}; b {}", a, b);
+   Debug::log("a[2] before corruption: {}", a[2]);
+   * (volatile void **)(b + 0x1) = (void*) -1;
+   Debug::log("a[2] after corruption: {}", a[2]);
+
+   Debug::log("Corrupting the allocator.");
+   struct Timeout    timeout = {0, UnlimitedTimeout};
+   volatile uint8_t *x1 =
+     (uint8_t *)heap_allocate(&timeout, MALLOC_CAPABILITY, 0x20);
+   Debug::log("Got capability {}", x1);
+   Debug::log("Capability after addition {}", (volatile void **)(x1 + 0x20));
+   *(volatile void **)(x1 + 0x20) = (void *)-1;
+   Debug::log("Done. Now running the allocator to get an assert.");
+   for (int i = 0; i < 10; i++)
+   {
+       auto p = heap_allocate(&timeout, MALLOC_CAPABILITY, 10);
+       heap_free(MALLOC_CAPABILITY, p);
+   }
+   Debug::log("Exiting.");
 }
diff --git a/examples/01.hello_world/xmake.lua b/examples/01.hello_world/xmake.lua
index 3483e64..69e8898 100644
--- a/examples/01.hello_world/xmake.lua
+++ b/examples/01.hello_world/xmake.lua
@@ -27,8 +27,8 @@ firmware("hello_world")
                 compartment = "hello",
                 priority = 1,
                 entry_point = "say_hello",
-                stack_size = 0x200,
-                trusted_stack_frames = 1
+                stack_size = 0x1000,
+                trusted_stack_frames = 2
             }
         }, {expand = false})
     end)

Here it simply hangs, without triggering a fault, which seems to be an indicator that the stack is corrupted.

Additional information

• This does not reproduce on the emulator (on any emulator in fact, neither the IBEX SAFE emulator nor SAIL. There you will simply see the faults, as expected.
• @davidchisnall reproduced this bug on another A7 board, so this is likely not due to a faulty board.
• @davidchisnall also found out that this only happens on the 33MHz and not on the 20MHz, so something went wrong when changing the clock speed.
• This was found while investigating this bug which went unnoticed for a long time because the memory safety violation wasn't caught.

[kliuMsft]

@hlef, could you let me know whether we are still seeing the issue using the new FPGA build scripts (which increases timing margin)? Thanks,