Browse Source

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
Don Porter 6 years ago
parent
commit
181f9ea988
2 changed files with 18 additions and 6 deletions
  1. 2 2
      LibOS/shim/src/bookkeep/shim_vma.c
  2. 16 4
      LibOS/shim/src/shim_malloc.c

+ 2 - 2
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

+ 16 - 4
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);