From c0d82db22b8ceda01b7aa069993d1b6d07c6fadc Mon Sep 17 00:00:00 2001 From: Sean McBride Date: Mon, 1 Jun 2020 22:28:19 -0400 Subject: [PATCH] fix: Correct memory leak and general cleanup --- runtime/include/module.h | 5 +- runtime/include/runtime.h | 1 - runtime/include/sandbox.h | 4 +- runtime/include/sandbox_completion_queue.h | 3 +- runtime/include/types.h | 1 + runtime/src/memory/64bit_nix.c | 4 + runtime/src/sandbox.c | 203 ++++++++++++++------- runtime/src/sandbox_completion_queue.c | 31 ++-- runtime/src/sandbox_run_queue_ps.c | 1 + runtime/src/worker_thread.c | 44 +++-- 10 files changed, 193 insertions(+), 104 deletions(-) diff --git a/runtime/include/module.h b/runtime/include/module.h index 60f8ca7..a8ef0d6 100644 --- a/runtime/include/module.h +++ b/runtime/include/module.h @@ -126,11 +126,10 @@ module_initialize_memory(struct module *module) * @param module * @return 1 if valid. 0 if invalid **/ -static inline int +static inline bool module_is_valid(struct module *module) { - if (module && module->dynamic_library_handle && module->main) return 1; - return 0; + return (module && module->dynamic_library_handle && module->main); } /** diff --git a/runtime/include/runtime.h b/runtime/include/runtime.h index 3b99c60..425cf70 100644 --- a/runtime/include/runtime.h +++ b/runtime/include/runtime.h @@ -8,7 +8,6 @@ extern int runtime_epoll_file_descriptor; void alloc_linear_memory(void); void expand_memory(void); -void free_linear_memory(void *base, u32 bound, u32 max); INLINE char *get_function_from_table(u32 idx, u32 type_id); INLINE char *get_memory_ptr_for_runtime(u32 offset, u32 bounds_check); void runtime_initialize(void); diff --git a/runtime/include/sandbox.h b/runtime/include/sandbox.h index 12a34bc..049f4ee 100644 --- a/runtime/include/sandbox.h +++ b/runtime/include/sandbox.h @@ -25,12 +25,12 @@ struct sandbox_io_handle { typedef enum { + INITIALIZING, RUNNABLE, BLOCKED, RETURNED } sandbox_state_t; -// TODO: linear_memory_max_size is not really used struct sandbox { sandbox_state_t state; @@ -39,7 +39,7 @@ struct sandbox { void *linear_memory_start; // after sandbox struct u32 linear_memory_size; // from after sandbox struct - u32 linear_memory_max_size; + u64 linear_memory_max_size; void *stack_start; // guess we need a mechanism for stack allocation. u32 stack_size; // and to set the size of it. diff --git a/runtime/include/sandbox_completion_queue.h b/runtime/include/sandbox_completion_queue.h index f9f8b71..f2d5b59 100644 --- a/runtime/include/sandbox_completion_queue.h +++ b/runtime/include/sandbox_completion_queue.h @@ -1,11 +1,10 @@ #ifndef SFRT_SANDBOX_COMPLETION_QUEUE_H #define SFRT_SANDBOX_COMPLETION_QUEUE_H -#include #include "sandbox.h" void sandbox_completion_queue_add(struct sandbox *sandbox); -void sandbox_completion_queue_free(unsigned int number_to_free); +void sandbox_completion_queue_free(); void sandbox_completion_queue_initialize(); #endif /* SFRT_SANDBOX_COMPLETION_QUEUE_H */ \ No newline at end of file diff --git a/runtime/include/types.h b/runtime/include/types.h index a0f8af8..0c47323 100644 --- a/runtime/include/types.h +++ b/runtime/include/types.h @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include diff --git a/runtime/src/memory/64bit_nix.c b/runtime/src/memory/64bit_nix.c index 2317794..fd6c040 100644 --- a/runtime/src/memory/64bit_nix.c +++ b/runtime/src/memory/64bit_nix.c @@ -40,6 +40,10 @@ expand_memory(void) } // TODO: check sandbox->linear_memory_max_size + if (sandbox_lmbound > sandbox->linear_memory_max_size) { + printf("expand_memory - Out of Memory!\n"); + exit(EXIT_FAILURE); + } sandbox_lmbound += WASM_PAGE_SIZE; sandbox->linear_memory_size = sandbox_lmbound; } diff --git a/runtime/src/sandbox.c b/runtime/src/sandbox.c index 1298495..edd913d 100644 --- a/runtime/src/sandbox.c +++ b/runtime/src/sandbox.c @@ -259,84 +259,138 @@ current_sandbox_main(void) } /** - * Allocates the memory for a sandbox to run a module + * Allocates a WebAssembly sandbox represented by the following layout + * struct sandbox | Buffer for HTTP Req/Resp | 4GB of Wasm Linear Memory | Guard Page * @param module the module that we want to run * @returns the resulting sandbox or NULL if mmap failed **/ static inline struct sandbox * sandbox_allocate_memory(struct module *module) { - unsigned long memory_size = SBOX_MAX_MEM; // 4GB + assert(module != NULL); - // Why do we add max_request_or_response_size? - unsigned long sandbox_size = sizeof(struct sandbox) + module->max_request_or_response_size; - unsigned long linear_memory_size = WASM_PAGE_SIZE * WASM_START_PAGES; + char * error_message = NULL; + unsigned long linear_memory_size = WASM_PAGE_SIZE * WASM_START_PAGES; // The initial pages + uint64_t linear_memory_max_size = (uint64_t)SBOX_MAX_MEM; + struct sandbox *sandbox = NULL; + unsigned long sandbox_size = sizeof(struct sandbox) + module->max_request_or_response_size; - if (linear_memory_size + sandbox_size > memory_size) return NULL; + // Control information should be page-aligned + // TODO: Should I use round_up_to_page when setting sandbox_page? assert(round_up_to_page(sandbox_size) == sandbox_size); - // What does mmap do exactly with file_descriptor -1? - void *addr = mmap(NULL, sandbox_size + memory_size + /* guard page */ PAGE_SIZE, PROT_NONE, + // At an address of the system's choosing, allocate the memory, marking it as inaccessible + errno = 0; + void *addr = mmap(NULL, sandbox_size + linear_memory_max_size + /* guard page */ PAGE_SIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); - if (addr == MAP_FAILED) return NULL; + if (addr == MAP_FAILED) { + error_message = "sandbox_allocate_memory - memory allocation failed"; + goto alloc_failed; + } + sandbox = (struct sandbox *)addr; + // Set the struct sandbox, HTTP Req/Resp buffer, and the initial Wasm Pages as read/write + errno = 0; void *addr_rw = mmap(addr, sandbox_size + linear_memory_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); if (addr_rw == MAP_FAILED) { - munmap(addr, memory_size + PAGE_SIZE); - return NULL; + error_message = "set to r/w"; + goto set_rw_failed; } - struct sandbox *sandbox = (struct sandbox *)addr; - // can it include sandbox as well? - sandbox->linear_memory_start = (char *)addr + sandbox_size; - sandbox->linear_memory_size = linear_memory_size; - sandbox->module = module; - sandbox->sandbox_size = sandbox_size; + // Populate Sandbox members + sandbox->linear_memory_start = (char *)addr + sandbox_size; + sandbox->linear_memory_size = linear_memory_size; + sandbox->linear_memory_max_size = linear_memory_max_size; + sandbox->module = module; + sandbox->sandbox_size = sandbox_size; module_acquire(module); +done: return sandbox; +set_rw_failed: + sandbox = NULL; + errno = 0; + int rc = munmap(addr, sandbox_size + linear_memory_size + PAGE_SIZE); + if (rc == -1) perror("Failed to munmap after fail to set r/w"); +alloc_failed: +alloc_too_big: +err: + perror(error_message); + goto done; } -struct sandbox * -sandbox_allocate(sandbox_request_t *sandbox_request) +int +sandbox_allocate_stack(sandbox_t *sandbox) { - struct module * module = sandbox_request->module; - char * arguments = sandbox_request->arguments; - int socket_descriptor = sandbox_request->socket_descriptor; - const struct sockaddr *socket_address = sandbox_request->socket_address; - u64 start_time = sandbox_request->start_time; - u64 absolute_deadline = sandbox_request->absolute_deadline; - - if (!module_is_valid(module)) return NULL; - - // FIXME: don't use malloc. huge security problem! - // perhaps, main should be in its own sandbox, when it is not running any sandbox. - struct sandbox *sandbox = (struct sandbox *)sandbox_allocate_memory(module); - if (!sandbox) return NULL; - - // Assign the start time from the request - sandbox->start_time = start_time; - sandbox->absolute_deadline = absolute_deadline; - - // actual module instantiation! - sandbox->arguments = (void *)arguments; - sandbox->stack_size = module->stack_size; + assert(sandbox); + assert(sandbox->module); + + errno = 0; + sandbox->stack_size = sandbox->module->stack_size; sandbox->stack_start = mmap(NULL, sandbox->stack_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_GROWSDOWN, -1, 0); - if (sandbox->stack_start == MAP_FAILED) { - perror("mmap"); - assert(0); - } - sandbox->client_socket_descriptor = socket_descriptor; + if (sandbox->stack_start == MAP_FAILED) goto err_stack_allocation_failed; + +done: + return 0; +err_stack_allocation_failed: + perror("sandbox_allocate_stack"); + return -1; +} + +struct sandbox * +sandbox_allocate(sandbox_request_t *sandbox_request) +{ + assert(sandbox_request != NULL); + assert(sandbox_request->module != NULL); + assert(module_is_valid(sandbox_request->module)); + + char * error_message = NULL; + int rc; + struct sandbox *sandbox = NULL; + + // Allocate Sandbox control structures, buffers, and linear memory in a 4GB address space + errno = 0; + sandbox = (struct sandbox *)sandbox_allocate_memory(sandbox_request->module); + if (!sandbox) goto err_memory_allocation_failed; + + // Set state to initializing + sandbox->state = INITIALIZING; + + // Allocate the Stack + rc = sandbox_allocate_stack(sandbox); + if (rc != 0) goto err_stack_allocation_failed; + + // Copy the socket descriptor, address, and arguments of the client invocation + sandbox->absolute_deadline = sandbox_request->absolute_deadline; + sandbox->arguments = (void *)sandbox_request->arguments; + sandbox->client_socket_descriptor = sandbox_request->socket_descriptor; + sandbox->start_time = sandbox_request->start_time; + + // Initialize the sandbox's context, stack, and instruction pointer + arch_context_init(&sandbox->ctxt, (reg_t)current_sandbox_main, + (reg_t)(sandbox->stack_start + sandbox->stack_size)); + + // What does it mean if there isn't a socket_address? Shouldn't this be a hard requirement? + // It seems that only the socket descriptor is used to send response + const struct sockaddr *socket_address = sandbox_request->socket_address; if (socket_address) memcpy(&sandbox->client_address, socket_address, sizeof(struct sockaddr)); + + // Initialize file descriptors to -1 for (int i = 0; i < SANDBOX_MAX_IO_HANDLE_COUNT; i++) sandbox->io_handles[i].file_descriptor = -1; + + // Initialize Parsec control structures (used by Completion Queue) ps_list_init_d(sandbox); - // Setup the sandbox's context, stack, and instruction pointer - arch_context_init(&sandbox->ctxt, (reg_t)current_sandbox_main, - (reg_t)(sandbox->stack_start + sandbox->stack_size)); +done: return sandbox; +err_stack_allocation_failed: + sandbox_free(sandbox); +err_memory_allocation_failed: +err: + perror(error_message); + goto done; } /** @@ -346,31 +400,48 @@ sandbox_allocate(sandbox_request_t *sandbox_request) void sandbox_free(struct sandbox *sandbox) { - // you have to context switch away to free a sandbox. - if (!sandbox || sandbox == current_sandbox_get()) return; - - // again sandbox should be done and waiting for the parent. - if (sandbox->state != RETURNED) return; + assert(sandbox != NULL); + assert(sandbox != current_sandbox_get()); + assert(sandbox->state == INITIALIZING || sandbox->state == RETURNED); - int sz = sizeof(struct sandbox); + char *error_message = NULL; + int rc; - sz += sandbox->module->max_request_or_response_size; module_release(sandbox->module); void * stkaddr = sandbox->stack_start; size_t stksz = sandbox->stack_size; - // depending on the memory type - // free_linear_memory(sandbox->linear_memory_start, sandbox->linear_memory_size, - // sandbox->linear_memory_max_size); - int ret; - // mmaped memory includes sandbox structure in there. - ret = munmap(sandbox, sz); - if (ret) perror("munmap sandbox"); + // Free Sandbox Stack + errno = 0; + rc = munmap(stkaddr, stksz); + if (rc == -1) { + error_message = "Failed to unmap stack"; + goto err_free_stack_failed; + }; + - // remove stack! - // for some reason, removing stack seem to cause crash in some cases. - ret = munmap(stkaddr, stksz); - if (ret) perror("munmap stack"); + // Free Sandbox Linear Address Space + // struct sandbox | HTTP Buffer | 4GB of Wasm Linear Memory | Guard Page + // sandbox_size includes the struct and HTTP buffer + size_t sandbox_address_space_size = sandbox->sandbox_size + sandbox->linear_memory_max_size + + /* guard page */ PAGE_SIZE; + + errno = 0; + rc = munmap(sandbox, sandbox_address_space_size); + if (rc == -1) { + error_message = "Failed to unmap sandbox"; + goto err_free_sandbox_failed; + }; + +done: + return; +err_free_sandbox_failed: +err_free_stack_failed: + // Inability to free memory is a fatal error + perror(error_message); + exit(EXIT_FAILURE); +err: + goto done; } diff --git a/runtime/src/sandbox_completion_queue.c b/runtime/src/sandbox_completion_queue.c index 5a69858..c7afa15 100644 --- a/runtime/src/sandbox_completion_queue.c +++ b/runtime/src/sandbox_completion_queue.c @@ -9,6 +9,12 @@ sandbox_completion_queue_initialize() ps_list_head_init(&sandbox_completion_queue); } +static inline bool +sandbox_completion_queue_is_empty() +{ + return ps_list_head_empty(&sandbox_completion_queue); +} + /** * Adds sandbox to the completion queue * @param sandbox @@ -16,29 +22,26 @@ sandbox_completion_queue_initialize() void sandbox_completion_queue_add(struct sandbox *sandbox) { + assert(sandbox); assert(ps_list_singleton_d(sandbox)); ps_list_head_append_d(&sandbox_completion_queue, sandbox); + assert(!sandbox_completion_queue_is_empty()); } -static inline bool -sandbox_completion_queue_is_empty() -{ - return ps_list_head_empty(&sandbox_completion_queue); -} /** - * @brief Pops n sandboxes from the thread local completion queue and then frees them - * @param number_to_free The number of sandboxes to pop and free + * @brief Frees all sandboxes in the thread local completion queue * @return void */ void -sandbox_completion_queue_free(unsigned int number_to_free) +sandbox_completion_queue_free() { - for (int i = 0; i < number_to_free; i++) { - if (sandbox_completion_queue_is_empty()) break; - struct sandbox *sandbox = ps_list_head_first_d(&sandbox_completion_queue, struct sandbox); - if (!sandbox) break; - ps_list_rem_d(sandbox); - sandbox_free(sandbox); + struct sandbox *sandbox_iterator; + struct sandbox *buffer; + + ps_list_foreach_del_d(&sandbox_completion_queue, sandbox_iterator, buffer) + { + ps_list_rem_d(sandbox_iterator); + sandbox_free(sandbox_iterator); } } \ No newline at end of file diff --git a/runtime/src/sandbox_run_queue_ps.c b/runtime/src/sandbox_run_queue_ps.c index 82e7f56..523b98e 100644 --- a/runtime/src/sandbox_run_queue_ps.c +++ b/runtime/src/sandbox_run_queue_ps.c @@ -84,6 +84,7 @@ sandbox_run_queue_ps_get_next() // Otherwise, allocate the sandbox request as a runnable sandbox and place on the runqueue struct sandbox *sandbox = sandbox_allocate(sandbox_request); + if (sandbox == NULL) return NULL; assert(sandbox); free(sandbox_request); sandbox->state = RUNNABLE; diff --git a/runtime/src/worker_thread.c b/runtime/src/worker_thread.c index f5fdbbd..6d75a44 100644 --- a/runtime/src/worker_thread.c +++ b/runtime/src/worker_thread.c @@ -45,7 +45,7 @@ static __thread bool worker_thread_is_in_callback; /** * @brief Switches to the next sandbox, placing the current sandbox on the completion queue if in RETURNED state - * @param next_sandbox The Sandbox Context to switch to or NULL + * @param next_sandbox The Sandbox Context to switch to or NULL, which forces return to base context * @return void */ static inline void @@ -67,9 +67,15 @@ worker_thread_switch_to_sandbox(struct sandbox *next_sandbox) worker_thread_next_context = next_register_context; arch_context_switch(previous_register_context, next_register_context); + assert(previous_sandbox == NULL || previous_sandbox->state == RUNNABLE || previous_sandbox->state == BLOCKED + || previous_sandbox->state == RETURNED); + // If the current sandbox we're switching from is in a RETURNED state, add to completion queue - if (previous_sandbox != NULL && previous_sandbox->state == RETURNED) + if (previous_sandbox != NULL && previous_sandbox->state == RETURNED) { sandbox_completion_queue_add(previous_sandbox); + } else if (previous_sandbox != NULL) { + printf("Switched away from sandbox is state %d\n", previous_sandbox->state); + } software_interrupt_enable(); } @@ -201,10 +207,9 @@ worker_thread_main(void *return_code) next_sandbox = sandbox_run_queue_get_next(); software_interrupt_enable(); - if (next_sandbox != NULL) { - worker_thread_switch_to_sandbox(next_sandbox); - sandbox_completion_queue_free(1); - } + if (next_sandbox != NULL) worker_thread_switch_to_sandbox(next_sandbox); + + sandbox_completion_queue_free(); } *(int *)return_code = -1; @@ -220,18 +225,25 @@ worker_thread_main(void *return_code) void worker_thread_exit_current_sandbox(void) { - // Remove the sandbox that exited from the runqueue and set state to RETURNED - struct sandbox *previous_sandbox = current_sandbox_get(); - assert(previous_sandbox); - software_interrupt_disable(); - sandbox_run_queue_delete(previous_sandbox); - previous_sandbox->state = RETURNED; + struct sandbox *exiting_sandbox = current_sandbox_get(); + assert(exiting_sandbox); - struct sandbox *next_sandbox = sandbox_run_queue_get_next(); - assert(next_sandbox != previous_sandbox); + // TODO: I do not understand when software interrupts must be disabled? + software_interrupt_disable(); + sandbox_run_queue_delete(exiting_sandbox); + exiting_sandbox->state = RETURNED; software_interrupt_enable(); + // Because the stack is still in use, only unmap linear memory and defer free resources until "main // function execution" - munmap(previous_sandbox->linear_memory_start, SBOX_MAX_MEM + PAGE_SIZE); - worker_thread_switch_to_sandbox(next_sandbox); + int rc = munmap(exiting_sandbox->linear_memory_start, SBOX_MAX_MEM + PAGE_SIZE); + if (rc == -1) { + perror("worker_thread_exit_current_sandbox - munmap failed"); + assert(0); + } + + sandbox_completion_queue_add(exiting_sandbox); + + // This should force return to main event loop + worker_thread_switch_to_sandbox(NULL); }