From 7141afc8a9d4cd306f02b4a3618903e5b5e361a5 Mon Sep 17 00:00:00 2001 From: Dmitrii Kuvaiskii Date: Wed, 4 Mar 2020 16:27:42 -0800 Subject: [PATCH] [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 | 2 +- Pal/src/host/Linux-SGX/db_main.c | 4 +- Pal/src/host/Linux-SGX/db_memory.c | 124 ++++++---------------- Pal/src/host/Linux-SGX/enclave_pages.c | 136 +++++++++++++++++++------ Pal/src/host/Linux-SGX/enclave_pages.h | 4 +- 5 files changed, 142 insertions(+), 128 deletions(-) diff --git a/Pal/src/host/Linux-SGX/db_files.c b/Pal/src/host/Linux-SGX/db_files.c index 50b6aae0..e9a87c31 100644 --- a/Pal/src/host/Linux-SGX/db_files.c +++ b/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; diff --git a/Pal/src/host/Linux-SGX/db_main.c b/Pal/src/host/Linux-SGX/db_main.c index 323f782f..71882901 100644 --- a/Pal/src/host/Linux-SGX/db_main.c +++ b/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); } diff --git a/Pal/src/host/Linux-SGX/db_memory.c b/Pal/src/host/Linux-SGX/db_memory.c index d0367dad..9ad7c3a6 100644 --- a/Pal/src/host/Linux-SGX/db_memory.c +++ b/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 +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; } diff --git a/Pal/src/host/Linux-SGX/enclave_pages.c b/Pal/src/host/Linux-SGX/enclave_pages.c index 30a17e98..5ac3a848 100644 --- a/Pal/src/host/Linux-SGX/enclave_pages.c +++ b/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; +} diff --git a/Pal/src/host/Linux-SGX/enclave_pages.h b/Pal/src/host/Linux-SGX/enclave_pages.h index 9491e1e0..7fa112ee 100644 --- a/Pal/src/host/Linux-SGX/enclave_pages.h +++ b/Pal/src/host/Linux-SGX/enclave_pages.h @@ -1,5 +1,7 @@ +#include #include 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);