From 181f9ea988698f9ffbb956728747b2ce2f554279 Mon Sep 17 00:00:00 2001 From: Don Porter Date: Tue, 6 Feb 2018 11:48:53 -0500 Subject: [PATCH] Fix race condition of memory mappings during start-up. (#126) * Fix a system_malloc() race during start-up. This should address issues #100 and #13 --- LibOS/shim/src/bookkeep/shim_vma.c | 4 ++-- LibOS/shim/src/shim_malloc.c | 20 ++++++++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/LibOS/shim/src/bookkeep/shim_vma.c b/LibOS/shim/src/bookkeep/shim_vma.c index b2761a1d..5aebd204 100644 --- a/LibOS/shim/src/bookkeep/shim_vma.c +++ b/LibOS/shim/src/bookkeep/shim_vma.c @@ -891,7 +891,7 @@ static struct shim_vma * __lookup_supervma (const void * addr, uint64_t length, struct shim_vma ** pprev) { struct shim_vma * tmp, * prev = NULL; - + listp_for_each_entry(tmp, &vma_list, list) { if (test_vma_contain(tmp, addr, length)) { if (pprev) @@ -903,7 +903,7 @@ static struct shim_vma * __lookup_supervma (const void * addr, uint64_t length, if (!(!prev || prev->addr + prev->length <= tmp->addr)) { struct shim_vma * tmp2; warn("Failure\n"); - uint64_t last_addr = 0; + void * last_addr = NULL; listp_for_each_entry(tmp2, &vma_list, list) { warn ("Entry: %llx..%llx (%llx)\n", tmp2->addr, tmp2->addr + tmp2->length, tmp2->length); // Don't do an infinite dump if the list gets corrupted diff --git a/LibOS/shim/src/shim_malloc.c b/LibOS/shim/src/shim_malloc.c index ee560fee..306f4612 100644 --- a/LibOS/shim/src/shim_malloc.c +++ b/LibOS/shim/src/shim_malloc.c @@ -145,18 +145,30 @@ static struct shim_heap * __alloc_enough_heap (size_t size) return heap; } +/* Returns NULL on failure */ void * __system_malloc (size_t size) { size_t alloc_size = ALIGN_UP(size); - void *addr; + void *addr, *addr_new; lock(shim_heap_lock); if (vmas_initialized) { - addr = (void *) DkVirtualMemoryAlloc(NULL, alloc_size, 0, - PAL_PROT_WRITE|PAL_PROT_READ); + /* If vmas are initialized, we need to request a free address range + * using get_unmapped_vma(). The current mmap code uses this function + * to synchronize all address allocation, via a "publication" + * pattern. It is not safe to just call DkVirtualMemoryAlloc directly + * without reserving the vma region first. + */ + int flags = MAP_PRIVATE|MAP_ANONYMOUS|VMA_INTERNAL; + addr = get_unmapped_vma(alloc_size, flags); + if (!addr) return NULL; + addr_new = (void *) DkVirtualMemoryAlloc(addr, alloc_size, 0, + PAL_PROT_WRITE|PAL_PROT_READ); + if (!addr_new) return NULL; + assert (addr == addr_new); bkeep_mmap(addr, alloc_size, PROT_READ|PROT_WRITE, - MAP_PRIVATE|MAP_ANONYMOUS|VMA_INTERNAL, NULL, 0, NULL); + flags, NULL, 0, NULL); } else { struct shim_heap * heap = __alloc_enough_heap(alloc_size);