From 483b42cf6f1860c05ff9d1553712c12b1e77f4d8 Mon Sep 17 00:00:00 2001 From: Andrei Lascu Date: Thu, 15 Aug 2024 15:24:41 +0100 Subject: [PATCH] Memory layout improvements Move around compartment scratch memory, to ensure the heap is next to the DDC boundary in the compartment. Thus, any overflows should trigger a `SIGPROT`. Ensure compartment boundaries are as they are defined in the struct. Particularly, in `mmap`, `length` would be padded such that it would be divisible by `page_size`. This padding is now done explicitly, and additional checks are added throughout to ensure this remains consistent. This prevents the compartment scratch heap to be larger than configured in the compartment. Further changes: * `run_test.py` now always returns, without giving an exception * added `simple_malloc_saturate`, which allocates all available heap memory, and then some, and expects to fail * `malloc` in `comp_utils` now returns `NULL` when running out of memory, instead of calling `err` * mark some code in `simple_call_internal_weak` to not be `clang-format`ted, as it seems to diverge from the CHERI version of `clang-format` --- include/compartment.h | 2 + src/comp_utils.c | 3 +- src/compartment.c | 106 ++++++++++++++---------------- src/manager.c | 63 +++++++++++++++++- tests/CMakeLists.txt | 14 ++++ tests/run_test.py | 6 +- tests/simple_call_internal_weak.c | 3 + tests/simple_malloc.c | 13 ++++ tests/simple_malloc_saturate.c | 20 ++++++ 9 files changed, 167 insertions(+), 63 deletions(-) create mode 100644 tests/simple_malloc_saturate.c diff --git a/include/compartment.h b/include/compartment.h index bf4976d..a453b19 100644 --- 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 index ab56f55..d14548f 100644 --- 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 index ef6e27d..e460eee 100644 --- 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 index c602f43..e557b78 100644 --- 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 index 6489588..dcbfa47 100644 --- 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 index 08f0c4e..03d2ebd 100755 --- 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 index 25b039f..2e63f3e 100644 --- a/tests/simple_call_internal_weak.c +++ b/tests/simple_call_internal_weak.c @@ -1,8 +1,11 @@ #include #include +// 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) { diff --git a/tests/simple_malloc.c b/tests/simple_malloc.c index 8fc9395..d9ddfbb 100644 --- a/tests/simple_malloc.c +++ b/tests/simple_malloc.c @@ -8,6 +8,12 @@ check_next(void *addr, void *to_check) assert(*((void **) ((char *) addr - sizeof(void *))) == to_check); } +static void +double_val(int *v) +{ + *v = *v * 2; +} + int main(void) { @@ -38,6 +44,13 @@ main(void) free(tmp02); free(tmp01); + // Check stack and heap disjointment + int *int01 = malloc(1 * sizeof(int)); + *int01 = 42; + double_val(int01); + assert(*int01 == 84); + free(int01); + // Check realloc void *tmp11 = realloc(NULL, 2 * malloc_block_sz); void *tmp13 = realloc(NULL, 1 * malloc_block_sz); diff --git a/tests/simple_malloc_saturate.c b/tests/simple_malloc_saturate.c new file mode 100644 index 0000000..75225fd --- /dev/null +++ b/tests/simple_malloc_saturate.c @@ -0,0 +1,20 @@ +#include +#include +#include + +const size_t max_heap_size = 0x800000; +const size_t malloc_block_sz = 0x10; + +int +main(void) +{ + void *x = malloc(max_heap_size - malloc_block_sz); + void *y = malloc(malloc_block_sz); + if (y == NULL) + { + printf("Memory saturated.\n"); + } + + free(x); + free(y); +}