[Pal/Linux-SGX] Remove pal_vmas array to consolidate VMA logic in enclave_pages.c

Previously, Linux-SGX PAL contained a hack to bookkeep PAL-internal
VMAs in the pal_vmas array with 64 VMAs. This limit led to
out-of-PAL-memory errors for bigger applications, e.g., with huge
number of trusted files.

This commit removes the hack with pal_vmas and merges all its logic
into the existing VMA bookkeeping logic in `enclave_pages.c`. This
eliminates the PAL-internal limit and also improves performance,
since there is no need to traverse an additional array (pal_vmas).

Pal/src/host/Linux-SGX/db_files.c

@@ -199,7 +199,7 @@ static int file_map(PAL_HANDLE handle, void** addr, int prot, uint64_t offset, u
         return -PAL_ERROR_DENIED;
     }
 
-    mem = get_enclave_pages(mem, size);
+    mem = get_enclave_pages(mem, size, /*is_pal_internal=*/false);
     if (!mem)
         return -PAL_ERROR_NOMEM;
 

Pal/src/host/Linux-SGX/db_main.c

@@ -61,8 +61,8 @@ unsigned long _DkGetAllocationAlignment (void)
 void _DkGetAvailableUserAddressRange (PAL_PTR * start, PAL_PTR * end,
                                       PAL_PTR * hole_start, PAL_PTR * hole_end)
 {
-    *start = (PAL_PTR) pal_sec.heap_min;
-    *end = (PAL_PTR) get_enclave_pages(NULL, g_page_size);
+    *start = (PAL_PTR)pal_sec.heap_min;
+    *end   = (PAL_PTR)get_enclave_heap_top();
     *hole_start = SATURATED_P_SUB(pal_sec.exec_addr, MEMORY_GAP, *start);
     *hole_end = SATURATED_P_ADD(pal_sec.exec_addr + pal_sec.exec_size, MEMORY_GAP, *end);
 }

Pal/src/host/Linux-SGX/db_memory.c

@@ -20,145 +20,81 @@
  * This files contains APIs that allocate, free or protect virtual memory.
  */
 
-#include "pal_defs.h"
-#include "pal_linux_defs.h"
+#include "api.h"
+#include "enclave_pages.h"
 #include "pal.h"
+#include "pal_debug.h"
+#include "pal_defs.h"
+#include "pal_error.h"
 #include "pal_internal.h"
 #include "pal_linux.h"
+#include "pal_linux_defs.h"
 #include "pal_security.h"
-#include "pal_error.h"
-#include "pal_debug.h"
-#include "spinlock.h"
-#include "api.h"
 
-#include <asm/mman.h>
+extern struct atomic_int g_alloced_pages;
+extern size_t g_page_size;
 
-#include "enclave_pages.h"
-
-/* TODO: Having VMAs in an array is extremely inefficient */
-#define PAL_VMA_MAX 64
-static struct pal_vma {
-    void * top, * bottom;
-} pal_vmas[PAL_VMA_MAX];
-
-static uint32_t pal_nvmas = 0;
-static spinlock_t pal_vma_lock = INIT_SPINLOCK_UNLOCKED;
-
-bool _DkCheckMemoryMappable (const void * addr, size_t size)
-{
+bool _DkCheckMemoryMappable(const void* addr, size_t size) {
     if (addr < DATA_END && addr + size > TEXT_START) {
-        printf("address %p-%p is not mappable\n", addr, addr + size);
+        SGX_DBG(DBG_E, "Address %p-%p is not mappable\n", addr, addr + size);
         return true;
     }
 
-    spinlock_lock(&pal_vma_lock);
+    /* FIXME: this function is almost useless now; note that _DkVirtualMemoryAlloc() checks whether
+     * [addr, addr + size) overlaps with VMAs and errors out */
 
-    for (uint32_t i = 0 ; i < pal_nvmas ; i++)
-        if (addr < pal_vmas[i].top && addr + size > pal_vmas[i].bottom) {
-            spinlock_unlock(&pal_vma_lock);
-            printf("address %p-%p is not mappable\n", addr, addr + size);
-            return true;
-        }
-
-    spinlock_unlock(&pal_vma_lock);
     return false;
 }
 
-int _DkVirtualMemoryAlloc (void ** paddr, uint64_t size, int alloc_type, int prot)
-{
-    if (!WITHIN_MASK(prot, PAL_PROT_MASK))
+int _DkVirtualMemoryAlloc(void** paddr, uint64_t size, int alloc_type, int prot) {
+    if (!size || !WITHIN_MASK(prot, PAL_PROT_MASK))
         return -PAL_ERROR_INVAL;
 
-    void * addr = *paddr, * mem;
+    void* addr = *paddr;
 
-    if ((alloc_type & PAL_ALLOC_INTERNAL) && addr)
+    if ((alloc_type & PAL_ALLOC_INTERNAL) && addr) {
+        /* internal-PAL memory allocation never uses fixed addresses */
         return -PAL_ERROR_INVAL;
+    }
 
-    if (size == 0)
-        __asm__ volatile ("int $3");
-
-    mem = get_enclave_pages(addr, size);
+    void* mem = get_enclave_pages(addr, size, alloc_type & PAL_ALLOC_INTERNAL);
     if (!mem)
         return addr ? -PAL_ERROR_DENIED : -PAL_ERROR_NOMEM;
 
-    if (alloc_type & PAL_ALLOC_INTERNAL) {
-        spinlock_lock(&pal_vma_lock);
-        if (pal_nvmas >= PAL_VMA_MAX) {
-            spinlock_unlock(&pal_vma_lock);
-            SGX_DBG(DBG_E, "Pal is out of VMAs (current limit on VMAs PAL_VMA_MAX = %d)!\n",
-                    PAL_VMA_MAX);
-            free_enclave_pages(mem, size);
-            return -PAL_ERROR_NOMEM;
-        }
-
-        pal_vmas[pal_nvmas].bottom = mem;
-        pal_vmas[pal_nvmas].top = mem + size;
-        pal_nvmas++;
-        spinlock_unlock(&pal_vma_lock);
-
-        SGX_DBG(DBG_M, "pal allocated %p-%p for internal use\n", mem, mem + size);
-    }
-
     memset(mem, 0, size);
 
     *paddr = mem;
     return 0;
 }
 
-int _DkVirtualMemoryFree (void * addr, uint64_t size)
-{
+int _DkVirtualMemoryFree(void* addr, uint64_t size) {
     if (sgx_is_completely_within_enclave(addr, size)) {
         int ret = free_enclave_pages(addr, size);
         if (ret < 0) {
             return ret;
         }
-
-        /* check if it is internal PAL memory and remove this VMA from pal_vmas if yes */
-        spinlock_lock(&pal_vma_lock);
-        for (uint32_t i = 0; i < pal_nvmas; i++) {
-            if (addr == pal_vmas[i].bottom) {
-                /* TODO: currently assume that internal PAL memory is freed at same granularity as
-                 *       was allocated in _DkVirtualMemoryAlloc(); may be false in general case */
-                assert(addr + size == pal_vmas[i].top);
-
-                for (uint32_t j = i; j < pal_nvmas - 1; j++) {
-                    pal_vmas[j].bottom = pal_vmas[j + 1].bottom;
-                    pal_vmas[j].top    = pal_vmas[j + 1].top;
-                }
-
-                pal_nvmas--;
-                break;
-            }
-        }
-        spinlock_unlock(&pal_vma_lock);
     } else {
-        /* Possible to have untrusted mapping. Simply unmap
-           the memory outside the enclave */
+        /* possible to have untrusted mapping, simply unmap memory outside the enclave */
         ocall_munmap_untrusted(addr, size);
     }
     return 0;
 }
 
-int _DkVirtualMemoryProtect (void * addr, uint64_t size, int prot)
-{
-    static struct atomic_int at_cnt = {.counter = 0};
+int _DkVirtualMemoryProtect(void* addr, uint64_t size, int prot) {
+    __UNUSED(addr);
+    __UNUSED(size);
+    __UNUSED(prot);
 
+    static struct atomic_int at_cnt = {.counter = 0};
     if (atomic_cmpxchg(&at_cnt, 0, 1) == 0)
-        SGX_DBG(DBG_M, "[Warning] DkVirtualMemoryProtect (0x%p, %lu, %d) is unimplemented",
-                addr, size, prot);
+        SGX_DBG(DBG_M, "[Warning] DkVirtualMemoryProtect is unimplemented in Linux-SGX PAL");
     return 0;
 }
 
-unsigned long _DkMemoryQuota (void)
-{
+uint64_t _DkMemoryQuota(void) {
     return pal_sec.heap_max - pal_sec.heap_min;
 }
 
-extern struct atomic_int g_alloced_pages;
-extern unsigned int g_page_size;
-
-unsigned long _DkMemoryAvailableQuota (void)
-{
-    return (pal_sec.heap_max - pal_sec.heap_min) -
-        atomic_read(&g_alloced_pages) * g_page_size;
+uint64_t _DkMemoryAvailableQuota(void) {
+    return (pal_sec.heap_max - pal_sec.heap_min) - atomic_read(&g_alloced_pages) * g_page_size;
 }

Pal/src/host/Linux-SGX/enclave_pages.c

@@ -19,6 +19,7 @@ struct heap_vma {
     LIST_TYPE(heap_vma) list;
     void* bottom;
     void* top;
+    bool is_pal_internal;
 };
 DEFINE_LISTP(heap_vma);
 
@@ -42,6 +43,7 @@ int init_enclave_pages(void) {
         }
         exec_vma->bottom = SATURATED_P_SUB(pal_sec.exec_addr, MEMORY_GAP, g_heap_bottom);
         exec_vma->top = SATURATED_P_ADD(pal_sec.exec_addr + pal_sec.exec_size, MEMORY_GAP, g_heap_top);
+        exec_vma->is_pal_internal = false;
         INIT_LIST_HEAD(exec_vma, list);
         LISTP_ADD(exec_vma, &g_heap_vma_list, list);
 
@@ -54,25 +56,15 @@ int init_enclave_pages(void) {
     return 0;
 }
 
-static void* __create_vma_and_merge(void* addr, size_t size, struct heap_vma* vma_above) {
+static void* __create_vma_and_merge(void* addr, size_t size, bool is_pal_internal,
+                                    struct heap_vma* vma_above) {
     assert(_DkInternalIsLocked(&g_heap_vma_lock));
     assert(addr && size);
 
     if (addr < g_heap_bottom)
         return NULL;
 
-    /* create VMA with [addr, addr+size); in case of existing overlapping VMAs, the created VMA is
-     * merged with them and the old VMAs are discarded, similar to mmap(MAX_FIXED) */
-    struct heap_vma* vma = malloc(sizeof(*vma));
-    if (!vma)
-        return NULL;
-
-    vma->bottom = addr;
-    vma->top    = addr + size;
-
-    /* find VMAs to merge:
-     *   (1) start from `vma_above` and iterate through VMAs with higher-addresses for merges
-     *   (2) start from `vma_below` and iterate through VMAs with lower-addresses for merges */
+    /* find enclosing VMAs and check that pal-internal VMAs do not overlap with normal VMAs */
     struct heap_vma* vma_below;
     if (vma_above) {
         vma_below = LISTP_NEXT_ENTRY(vma_above, &g_heap_vma_list, list);
@@ -81,11 +73,53 @@ static void* __create_vma_and_merge(void* addr, size_t size, struct heap_vma* vm
         vma_below = LISTP_FIRST_ENTRY(&g_heap_vma_list, struct heap_vma, list);
     }
 
-    while (vma_above && vma_above->bottom <= vma->top) {
+    /* check whether [addr, addr + size) overlaps with above VMAs of different type */
+    struct heap_vma* check_vma_above = vma_above;
+    while (check_vma_above && addr + size > check_vma_above->bottom) {
+        if (check_vma_above->is_pal_internal != is_pal_internal) {
+            SGX_DBG(DBG_M, "VMA %p-%p (internal=%d) overlaps with %p-%p (internal=%d)\n",
+                    addr, addr + size, is_pal_internal, check_vma_above->bottom,
+                    check_vma_above->top, check_vma_above->is_pal_internal);
+            return NULL;
+        }
+        check_vma_above = LISTP_PREV_ENTRY(check_vma_above, &g_heap_vma_list, list);
+    }
+
+    /* check whether [addr, addr + size) overlaps with below VMAs of different type */
+    struct heap_vma* check_vma_below = vma_below;
+    while (check_vma_below && addr < check_vma_below->top) {
+        if (check_vma_below->is_pal_internal != is_pal_internal) {
+            SGX_DBG(DBG_M, "VMA %p-%p (internal=%d) overlaps with %p-%p (internal=%d)\n",
+                    addr, addr + size, is_pal_internal, check_vma_below->bottom,
+                    check_vma_below->top, check_vma_below->is_pal_internal);
+            return NULL;
+        }
+        check_vma_below = LISTP_NEXT_ENTRY(check_vma_below, &g_heap_vma_list, list);
+    }
+
+    /* create VMA with [addr, addr+size); in case of existing overlapping VMAs, the created VMA is
+     * merged with them and the old VMAs are discarded, similar to mmap(MAX_FIXED) */
+    struct heap_vma* vma = malloc(sizeof(*vma));
+    if (!vma)
+        return NULL;
+    vma->bottom          = addr;
+    vma->top             = addr + size;
+    vma->is_pal_internal = is_pal_internal;
+
+    /* how much memory was freed because [addr, addr + size) overlapped with VMAs */
+    size_t freed = 0;
+
+    /* Try to merge VMAs as an optimization:
+     *   (1) start from `vma_above` and iterate through VMAs with higher-addresses for merges
+     *   (2) start from `vma_below` and iterate through VMAs with lower-addresses for merges.
+     * Note that we never merge normal VMAs with pal-internal VMAs. */
+    while (vma_above && vma_above->bottom <= vma->top &&
+           vma_above->is_pal_internal == vma->is_pal_internal) {
         /* newly created VMA grows into above VMA; expand newly created VMA and free above-VMA */
         SGX_DBG(DBG_M, "Merge %p-%p and %p-%p\n", vma->bottom, vma->top,
                 vma_above->bottom, vma_above->top);
 
+        freed += vma_above->top - vma_above->bottom;
         struct heap_vma* vma_above_above = LISTP_PREV_ENTRY(vma_above, &g_heap_vma_list, list);
 
         vma->bottom = MIN(vma_above->bottom, vma->bottom);
@@ -96,11 +130,13 @@ static void* __create_vma_and_merge(void* addr, size_t size, struct heap_vma* vm
         vma_above = vma_above_above;
     }
 
-    while (vma_below && vma_below->top >= vma->bottom) {
+    while (vma_below && vma_below->top >= vma->bottom &&
+           vma_below->is_pal_internal == vma->is_pal_internal) {
         /* newly created VMA grows into below VMA; expand newly create VMA and free below-VMA */
         SGX_DBG(DBG_M, "Merge %p-%p and %p-%p\n", vma->bottom, vma->top,
                 vma_below->bottom, vma_below->top);
 
+        freed += vma_below->top - vma_below->bottom;
         struct heap_vma* vma_below_below = LISTP_NEXT_ENTRY(vma_below, &g_heap_vma_list, list);
 
         vma->bottom = MIN(vma_below->bottom, vma->bottom);
@@ -120,11 +156,13 @@ static void* __create_vma_and_merge(void* addr, size_t size, struct heap_vma* vm
         ocall_exit(/*exitcode=*/1, /*is_exitgroup=*/true);
     }
 
-    atomic_add(size / g_page_size, &g_alloced_pages);
+    assert(vma->top - vma->bottom >= (ptrdiff_t)freed);
+    size_t allocated = vma->top - vma->bottom - freed;
+    atomic_add(allocated / g_page_size, &g_alloced_pages);
     return addr;
 }
 
-void* get_enclave_pages(void* addr, size_t size) {
+void* get_enclave_pages(void* addr, size_t size, bool is_pal_internal) {
     void* ret = NULL;
 
     if (!size)
@@ -135,7 +173,8 @@ void* get_enclave_pages(void* addr, size_t size) {
 
     assert(access_ok(addr, size));
 
-    SGX_DBG(DBG_M, "Allocating %ld bytes at %p\n", size, addr);
+    SGX_DBG(DBG_M, "Allocating %lu bytes in enclave memory at %p (%s)\n", size, addr,
+            is_pal_internal ? "PAL internal" : "normal");
 
     struct heap_vma* vma_above = NULL;
     struct heap_vma* vma;
@@ -154,14 +193,14 @@ void* get_enclave_pages(void* addr, size_t size) {
             }
             vma_above = vma;
         }
-        ret = __create_vma_and_merge(addr, size, vma_above);
+        ret = __create_vma_and_merge(addr, size, is_pal_internal, vma_above);
     } else {
         /* caller did not specify address; find first (highest-address) empty slot that fits */
         void* vma_above_bottom = g_heap_top;
 
         LISTP_FOR_EACH_ENTRY(vma, &g_heap_vma_list, list) {
             if (vma->top < vma_above_bottom - size) {
-                ret = __create_vma_and_merge(vma_above_bottom - size, size, vma_above);
+                ret = __create_vma_and_merge(vma_above_bottom - size, size, is_pal_internal, vma_above);
                 goto out;
             }
             vma_above = vma;
@@ -170,15 +209,11 @@ void* get_enclave_pages(void* addr, size_t size) {
 
         /* corner case: there may be enough space between heap bottom and the lowest-address VMA */
         if (g_heap_bottom < vma_above_bottom - size)
-            ret = __create_vma_and_merge(vma_above_bottom - size, size, vma_above);
+            ret = __create_vma_and_merge(vma_above_bottom - size, size, is_pal_internal, vma_above);
     }
 
 out:
     _DkInternalUnlock(&g_heap_vma_lock);
-
-    if (!ret) {
-        SGX_DBG(DBG_E, "*** Cannot allocate %lu bytes on the heap (at address %p) ***\n", size, addr);
-    }
     return ret;
 }
 
@@ -195,10 +230,18 @@ int free_enclave_pages(void* addr, size_t size) {
         return -PAL_ERROR_INVAL;
     }
 
-    SGX_DBG(DBG_M, "Freeing %ld bytes at %p\n", size, addr);
+    SGX_DBG(DBG_M, "Freeing %lu bytes in enclave memory at %p\n", size, addr);
 
     _DkInternalLock(&g_heap_vma_lock);
 
+    /* VMA list contains both normal and pal-internal VMAs; it is impossible to free an area
+     * that overlaps with VMAs of two types at the same time, so we fail in such cases */
+    bool is_pal_internal_set = false;
+    bool is_pal_internal;
+
+    /* how much memory was actually freed, since [addr, addr + size) can overlap with VMAs */
+    size_t freed = 0;
+
     struct heap_vma* vma;
     struct heap_vma* p;
     LISTP_FOR_EACH_ENTRY_SAFE(vma, p, &g_heap_vma_list, list) {
@@ -207,7 +250,21 @@ int free_enclave_pages(void* addr, size_t size) {
         if (vma->top <= addr)
             break;
 
-        /* found VMA overlapping with memory area to free */
+        /* found VMA overlapping with area to free; check it is either normal or pal-internal */
+        if (!is_pal_internal_set) {
+            is_pal_internal = vma->is_pal_internal;
+            is_pal_internal_set = true;
+        }
+
+        if (is_pal_internal != vma->is_pal_internal) {
+            SGX_DBG(DBG_E, "*** Area to free (address %p, size %lu) overlaps with both normal and "
+                    "pal-internal VMAs ***\n", addr, size);
+            ret = -PAL_ERROR_INVAL;
+            goto out;
+        }
+
+        freed += MIN(vma->top, addr + size) - MAX(vma->bottom, addr);
+
         if (vma->bottom < addr) {
             /* create VMA [vma->bottom, addr); this may leave VMA [addr + size, vma->top), see below */
             struct heap_vma* new = malloc(sizeof(*new));
@@ -216,8 +273,9 @@ int free_enclave_pages(void* addr, size_t size) {
                 ret = -PAL_ERROR_NOMEM;
                 goto out;
             }
-            new->top    = addr;
-            new->bottom = vma->bottom;
+            new->top             = addr;
+            new->bottom          = vma->bottom;
+            new->is_pal_internal = vma->is_pal_internal;
             INIT_LIST_HEAD(new, list);
             LIST_ADD(new, vma, list);
         }
@@ -231,9 +289,27 @@ int free_enclave_pages(void* addr, size_t size) {
         }
     }
 
-    atomic_sub(size / g_page_size, &g_alloced_pages);
+    atomic_sub(freed / g_page_size, &g_alloced_pages);
 
 out:
     _DkInternalUnlock(&g_heap_vma_lock);
     return ret;
 }
+
+/* returns current highest available address on the enclave heap */
+void* get_enclave_heap_top(void) {
+    _DkInternalLock(&g_heap_vma_lock);
+
+    void* addr = g_heap_top;
+    struct heap_vma* vma;
+    LISTP_FOR_EACH_ENTRY(vma, &g_heap_vma_list, list) {
+        if (vma->top < addr) {
+            goto out;
+        }
+        addr = vma->bottom;
+    }
+
+out:
+    _DkInternalUnlock(&g_heap_vma_lock);
+    return addr;
+}

Pal/src/host/Linux-SGX/enclave_pages.h

@@ -1,5 +1,7 @@
+#include <stdbool.h>
 #include <stddef.h>
 
 int init_enclave_pages(void);
-void* get_enclave_pages(void* addr, size_t size);
+void* get_enclave_heap_top(void);
+void* get_enclave_pages(void* addr, size_t size, bool is_pal_internal);
 int free_enclave_pages(void* addr, size_t size);