Merge pull request #36 from 0152la/lua-testing2

Memory layout improvements
capablevms · Aug 20, 2024 · db6e231 · db6e231
2 parents 59ef4e7 + 483b42c
commit db6e231
Show file tree

Hide file tree

Showing 9 changed files with 167 additions and 63 deletions.
diff --git a/include/compartment.h b/include/compartment.h
@@ -192,6 +192,7 @@ struct Compartment
     // Scratch memory
     void *scratch_mem_base;
     size_t scratch_mem_size;
+    size_t scratch_mem_extra;
 
     size_t scratch_mem_heap_size;
     void *scratch_mem_stack_top;
@@ -203,6 +204,7 @@ struct Compartment
     size_t entry_point_count;
     struct CompEntryPoint *entry_points;
     void *tls_lookup_func;
+    size_t total_tls_size;
     struct TLSDesc *libs_tls_sects;
 
     // Hardware info - maybe move

diff --git a/src/comp_utils.c b/src/comp_utils.c
@@ -96,10 +96,9 @@ malloc(size_t to_alloc)
     */
     size_t to_alloc_total = to_alloc + block_metadata_sz;
 
-    // TODO replace with return NULL and check `mem_left` works
     if (to_alloc_total > mem_left)
     {
-        errx(1, "comp_utils: Insufficient heap space left.");
+        return NULL;
     }
 
     // Find a sufficiently large block to allocate

diff --git a/src/compartment.c b/src/compartment.c
@@ -43,6 +43,8 @@ static void
 init_comp_scratch_mem(struct Compartment *);
 static void
 adjust_comp_scratch_mem(struct Compartment *, size_t);
+static inline void *
+get_extra_scratch_region_base(struct Compartment *);
 static void
 setup_environ(struct Compartment *);
 static void
@@ -52,8 +54,6 @@ static void
 print_lib_dep_seg(struct SegmentMap *);
 static void
 print_lib_dep(struct LibDependency *);
-static void
-print_comp(struct Compartment *);
 
 /*******************************************************************************
  * Main compartment functions
@@ -82,6 +82,7 @@ comp_init()
     new_comp->scratch_mem_heap_size = 0;
     new_comp->scratch_mem_stack_top = NULL;
     new_comp->scratch_mem_stack_size = 0;
+    new_comp->scratch_mem_extra = 0;
 
     new_comp->libs_count = 0;
     new_comp->libs = NULL;
@@ -126,7 +127,6 @@ comp_from_elf(char *filename, char **entry_points, size_t entry_point_count,
     {
         struct LibDependency *parsed_lib
             = parse_lib_file(libs_to_parse[libs_parsed_count], new_comp);
-        print_lib_dep(parsed_lib);
 
         // Get `tls_lookup_func` if we parsed `comp_utils.so`
         if (!strcmp(parsed_lib->lib_name, comp_utils_soname))
@@ -183,6 +183,19 @@ comp_from_elf(char *filename, char **entry_points, size_t entry_point_count,
             + // buffer between scratch memory and compartment libraries
             new_comp->scratch_mem_size // size of scratch memory
     );
+    assert(new_comp->scratch_mem_size % new_comp->page_size == 0);
+
+    /* Check correct scratch memory layout; we expect the stack and the heap to
+     * reside consecutively, with the heap at the edge of the compartment
+     * boundary, and any extra memory required residing before the stack.
+     *
+     * Potential extra memory regions: TLS, environ
+     */
+    assert((char *) new_comp->scratch_mem_base + new_comp->scratch_mem_extra
+            + new_comp->scratch_mem_stack_size
+        == new_comp->scratch_mem_stack_top);
+    assert(new_comp->environ_sz + new_comp->total_tls_size
+        == new_comp->scratch_mem_extra);
 
     return new_comp;
 }
@@ -228,6 +241,8 @@ comp_map(struct Compartment *to_map)
     // Map compartment scratch memory - heap, stack, sealed manager
     // capabilities for transition out, capabilities to call other compartments
     // (TODO fix this), TLS region (if applicable)
+    assert((intptr_t) to_map->scratch_mem_base % to_map->page_size == 0);
+    assert(to_map->scratch_mem_size % to_map->page_size == 0);
     map_result
         = mmap((void *) to_map->scratch_mem_base, to_map->scratch_mem_size,
             PROT_READ | PROT_WRITE, // | PROT_EXEC, // TODO Fix this
@@ -1132,7 +1147,14 @@ find_in_dir(const char *const lib_name, char *search_dir)
     return NULL;
 }
 
-// TODO carefully recheck all the numbers are right
+/* Lay out compartment's stack and heap. They each grow from
+ * `scratch_mem_stack_top`, with the stack growing downward, and the heap
+ * growing upwards. The heap shall reside at the edge of the DDC, such that any
+ * heap overflows will trigger a SIGPROT. Any further scratch memory required
+ * (such as for TLS) will be added at `scratch_mem_base`, and
+ * `scratch_mem_stack_top` will be adjusted appropriately, via
+ * `adjust_comp_scratch_mem()`.
+ */
 static void
 init_comp_scratch_mem(struct Compartment *new_comp)
 {
@@ -1142,7 +1164,7 @@ init_comp_scratch_mem(struct Compartment *new_comp)
     new_comp->scratch_mem_heap_size = 0x800000UL; // TODO
     new_comp->scratch_mem_stack_size = 0x80000UL; // TODO
     new_comp->scratch_mem_stack_top = align_down(
-        (char *) new_comp->scratch_mem_base + new_comp->scratch_mem_heap_size,
+        (char *) new_comp->scratch_mem_base + new_comp->scratch_mem_stack_size,
         16);
 
     new_comp->scratch_mem_size
@@ -1162,14 +1184,29 @@ init_comp_scratch_mem(struct Compartment *new_comp)
                - new_comp->scratch_mem_stack_size)
             % 16
         == 0);
-    assert(new_comp->scratch_mem_size % 16 == 0);
+    assert(new_comp->scratch_mem_size % new_comp->page_size == 0);
 }
 
 static void
 adjust_comp_scratch_mem(struct Compartment *new_comp, size_t to_adjust)
 {
+    assert(to_adjust % new_comp->page_size == 0);
     new_comp->scratch_mem_size += to_adjust;
+    new_comp->scratch_mem_stack_top
+        = (char *) new_comp->scratch_mem_stack_top + to_adjust;
     new_comp->mem_top = (char *) new_comp->mem_top + to_adjust;
+    new_comp->scratch_mem_extra += to_adjust;
+}
+
+/* New scratch regions will be added after previous extra regions
+ */
+static inline void *
+get_extra_scratch_region_base(struct Compartment *new_comp)
+{
+    char *new_scratch_region_base
+        = (char *) new_comp->scratch_mem_base + new_comp->scratch_mem_extra;
+    assert((intptr_t) new_scratch_region_base % new_comp->page_size == 0);
+    return new_scratch_region_base;
 }
 
 static void
@@ -1178,7 +1215,7 @@ setup_environ(struct Compartment *new_comp)
     assert(proc_env_ptr != NULL); // TODO consider optional check
     new_comp->environ_sz
         = align_up(max_env_sz, new_comp->page_size) + new_comp->page_size;
-    new_comp->environ_ptr = new_comp->mem_top;
+    new_comp->environ_ptr = get_extra_scratch_region_base(new_comp);
     adjust_comp_scratch_mem(new_comp, new_comp->environ_sz);
 }
 
@@ -1193,7 +1230,8 @@ resolve_comp_tls_regions(struct Compartment *new_comp)
 
     // TODO currently we only support one thread
     new_comp->libs_tls_sects->region_count = 1;
-    new_comp->libs_tls_sects->region_start = new_comp->mem_top;
+    new_comp->libs_tls_sects->region_start
+        = get_extra_scratch_region_base(new_comp);
     new_comp->libs_tls_sects->libs_count = 0;
 
     size_t comp_tls_size = 0;
@@ -1212,11 +1250,10 @@ resolve_comp_tls_regions(struct Compartment *new_comp)
 
     intptr_t total_tls_size
         = comp_tls_size * new_comp->libs_tls_sects->region_count;
+    total_tls_size = align_up(total_tls_size, new_comp->page_size);
     adjust_comp_scratch_mem(new_comp, total_tls_size);
+    new_comp->total_tls_size = total_tls_size;
     new_comp->libs_tls_sects->region_size = comp_tls_size;
-
-    assert((uintptr_t) new_comp->libs_tls_sects->region_start % 16 == 0);
-    // TODO reconsider scratch memory layout
 }
 
 /*******************************************************************************
@@ -1259,50 +1296,3 @@ print_lib_dep(struct LibDependency *lib_dep)
     printf("- rela_maps_count : %zu\n", lib_dep->rela_maps_count);
     printf("== DONE\n");
 }
-
-static void
-print_comp(struct Compartment *to_print)
-{
-    printf("== COMPARTMENT\n");
-    printf("- id : %lu\n", to_print->id);
-    {
-        printf("- DDC : ");
-        printf(" base - 0x%lx ", cheri_base_get(to_print->ddc));
-        printf(" length - 0x%lx ", cheri_length_get(to_print->ddc));
-        printf(" address - 0x%lx ", cheri_address_get(to_print->ddc));
-        printf(" offset - 0x%lx ", cheri_offset_get(to_print->ddc));
-        printf("\n");
-    }
-    printf("- size : 0x%zx\n", to_print->size);
-    printf("- base : %p\n", to_print->base);
-    printf("- mem_top : %p\n", to_print->mem_top);
-    printf("- mapped : %s\n", to_print->mapped ? "true" : "false");
-
-    printf("- scratch_mem_base : %p\n", to_print->scratch_mem_base);
-    printf("- scratch_mem_size : 0x%zx", to_print->scratch_mem_size);
-    printf(" [0x%zx heap + 0x%zx stack + 0x%zx tls]\n",
-        to_print->scratch_mem_heap_size, to_print->scratch_mem_stack_size,
-        to_print->libs_tls_sects->region_size
-            * to_print->libs_tls_sects->region_count);
-    printf(
-        "- scratch_mem_heap_size : 0x%zx\n", to_print->scratch_mem_heap_size);
-    printf("- scratch_mem_stack_top : %p\n", to_print->scratch_mem_stack_top);
-    printf(
-        "- scratch_mem_stack_size : 0x%zx\n", to_print->scratch_mem_stack_size);
-
-    printf("- libs_count : %lu\n", to_print->libs_count);
-    printf("- entry_point_count : %lu\n", to_print->entry_point_count);
-    // TODO entry_points
-    printf("- tls_lookup_func : %p\n", to_print->tls_lookup_func);
-    printf("- libs_tls_sects :\n");
-    printf("\t> region_count : %hu\n", to_print->libs_tls_sects->region_count);
-    printf("\t> region_size : 0x%zx\n", to_print->libs_tls_sects->region_size);
-    // TODO region_start
-    printf("\t> region_start : %p\n", to_print->libs_tls_sects->region_start);
-    printf("\t> libs_count : %hu\n", to_print->libs_tls_sects->libs_count);
-    printf("\n");
-
-    printf("- page_size : %lu\n", to_print->page_size);
-
-    printf("== DONE\n");
-}
diff --git a/src/manager.c b/src/manager.c
@@ -22,6 +22,8 @@ const size_t max_env_sz
     = max_env_count * sizeof(char *) + avg_sz_per_env_entry * max_env_count;
 extern char **environ;
 
+// Functions
+
 static struct CompEntryPointDef *
 parse_compartment_config(char *, size_t *, bool);
 static struct CompEntryPointDef *
@@ -36,6 +38,13 @@ prepare_compartment_args(char **args, struct CompEntryPointDef);
 static struct CompWithEntries *
 get_comp_with_entries(struct Compartment *);
 
+// Printing
+static void print_full_cap(uintcap_t);
+static void
+pp_cap(void *__capability);
+static void
+print_comp(struct Compartment *);
+
 /*******************************************************************************
  * Utility functions
  ******************************************************************************/
@@ -111,7 +120,7 @@ register_new_comp(char *filename, bool allow_default_entry)
     new_comp_ddc = cheri_bounds_set(
         new_comp_ddc, (char *) new_comp->mem_top - (char *) new_comp->base);
     new_comp_ddc = cheri_offset_set(new_comp_ddc,
-        (char *) new_comp->scratch_mem_base - (char *) new_comp->base);
+        (char *) new_comp->scratch_mem_stack_top - (char *) new_comp->base);
     new_comp->ddc = new_comp_ddc;
 
     struct CompWithEntries *new_cwe = malloc(sizeof(struct CompWithEntries));
@@ -417,3 +426,55 @@ make_default_entry_point()
     cep->args_type = NULL;
     return cep;
 }
+
+static void
+print_comp(struct Compartment *to_print)
+{
+    printf("== COMPARTMENT\n");
+    printf("- id : %lu\n", to_print->id);
+    {
+        printf("- DDC : ");
+        printf(" base - 0x%lx ", cheri_base_get(to_print->ddc));
+        printf(" length - 0x%lx ", cheri_length_get(to_print->ddc));
+        printf(" address - 0x%lx ", cheri_address_get(to_print->ddc));
+        printf(" offset - 0x%lx ", cheri_offset_get(to_print->ddc));
+        printf("\n");
+    }
+    printf("- size : 0x%zx\n", to_print->size);
+    printf("- base : %p\n", to_print->base);
+    printf("- mem_top : %p\n", to_print->mem_top);
+    printf("- mapped : %s\n", to_print->mapped ? "true" : "false");
+
+    printf("- environ_ptr : %p\n", (void *) to_print->environ_ptr);
+    printf("- environ_sz : 0x%zx\n", to_print->environ_sz);
+
+    printf("- scratch_mem_base : %p\n", to_print->scratch_mem_base);
+    printf("- scratch_mem_size : %#zx", to_print->scratch_mem_size);
+    printf(" [0x%zx heap + %#zx stack]\n", to_print->scratch_mem_heap_size,
+        to_print->scratch_mem_stack_size);
+    printf("- scratch_mem_extra : %#zx", to_print->scratch_mem_extra);
+    printf(" [%#zx tls + %#zx environ]\n", to_print->total_tls_size,
+        to_print->environ_sz);
+    printf(
+        "- scratch_mem_heap_size : 0x%zx\n", to_print->scratch_mem_heap_size);
+    printf("- scratch_mem_stack_top : %p\n", to_print->scratch_mem_stack_top);
+    printf(
+        "- scratch_mem_stack_size : 0x%zx\n", to_print->scratch_mem_stack_size);
+
+    printf("- libs_count : %lu\n", to_print->libs_count);
+    printf("- entry_point_count : %lu\n", to_print->entry_point_count);
+    // TODO entry_points
+    printf("- tls_lookup_func : %p\n", to_print->tls_lookup_func);
+    printf("- total_tls_size : %#zx\n", to_print->total_tls_size);
+    printf("- libs_tls_sects :\n");
+    printf("\t> region_count : %hu\n", to_print->libs_tls_sects->region_count);
+    printf("\t> region_size : 0x%zx\n", to_print->libs_tls_sects->region_size);
+    // TODO region_start
+    printf("\t> region_start : %p\n", to_print->libs_tls_sects->region_start);
+    printf("\t> libs_count : %hu\n", to_print->libs_tls_sects->libs_count);
+    printf("\n");
+
+    printf("- page_size : %lu\n", to_print->page_size);
+
+    printf("== DONE\n");
+}
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -133,6 +133,7 @@ set(comp_binaries
     "simple_global_var-external"
     "simple_libc"
     "simple_malloc"
+    "simple_malloc_saturate"
     "simple_open_write"
     "simple_printf"
     "simple_static_var"
@@ -170,6 +171,7 @@ set(tests
     "simple_global_var"
     "simple_libc"
     "simple_malloc"
+    "simple_malloc_saturate"
     "simple_open_write"
     "simple_printf"
     "simple_static_var"
@@ -199,6 +201,10 @@ set(tests
     "args-ulong-max args_simple check_ullong_max 18446744073709551615"
     )
 
+set(tests_fail
+    "simple_malloc_saturate@Memory saturated."
+)
+
 # Build targets
 foreach(comp_t IN LISTS comp_binaries)
     string(FIND ${comp_t} " " space_pos)
@@ -260,3 +266,11 @@ foreach(test_t IN LISTS tests)
         new_test(${test_name} ${test_bin} "${test_args}")
     endif()
 endforeach()
+
+foreach(test_t IN LISTS tests_fail)
+    string(FIND ${test_t} "@" delim_pos)
+    string(SUBSTRING ${test_t} 0 ${delim_pos} test_name)
+    string(SUBSTRING ${test_t} ${delim_pos} -1 pass_regex)
+    string(SUBSTRING ${pass_regex} 1 -1 pass_regex)
+    set_property(TEST ${test_name} PROPERTY PASS_REGULAR_EXPRESSION ${pass_regex})
+endforeach()
diff --git a/tests/run_test.py b/tests/run_test.py
@@ -2,6 +2,7 @@
 import argparse
 import pathlib
 import os
+import sys
 
 from fabric import Connection
 
@@ -37,7 +38,7 @@ def put_file(conn, src_file):
     conn.put(src_file, remote = f'{CHERIBSD_TEST_DIR}/')
 
 def exec_cmd(conn, cmd, remote_env):
-    return conn.run(cmd, env = remote_env, echo = True)
+    return conn.run(cmd, env = remote_env, echo = True, warn = True)
 
 ################################################################################
 # Main
@@ -56,5 +57,6 @@ def exec_cmd(conn, cmd, remote_env):
 file_deps = [args.test, *args.dependencies]
 for dep in file_deps:
     put_file(vm_conn, dep)
-exec_cmd(vm_conn, f'cd {CHERIBSD_TEST_DIR} ; ./{args.test.name} {" ".join(args.test_args)}', remote_env)
+res = exec_cmd(vm_conn, f'cd {CHERIBSD_TEST_DIR} ; ./{args.test.name} {" ".join(args.test_args)}', remote_env)
 vm_conn.close()
+sys.exit(res.exited)
diff --git a/tests/simple_call_internal_weak.c b/tests/simple_call_internal_weak.c
@@ -1,8 +1,11 @@
 #include <assert.h>
 #include <math.h>
 
+// clang-format off: local clang-format seems to have diverged from CHERI one
 int __attribute__((weak)) call_internal(int x) { return pow(x, 2); }
 
+// clang-format on
+
 int
 main(void)
 {