From f8e8b3a286dc9554a5b865296d40a11fbf724dd5 Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Tue, 17 Feb 2009 19:00:01 +0000 Subject: [PATCH] Fix NaClVmmapUpdate and add test --- service_runtime/SConscript | 2 + service_runtime/mmap_test.c | 140 ++++++++++++++++++++++ service_runtime/nacl_memory_object.c | 2 + service_runtime/nacl_syscall_common.h | 10 ++ service_runtime/sel_mem.c | 216 +++++++--------------------------- service_runtime/sel_mem.h | 2 + 6 files changed, 198 insertions(+), 174 deletions(-) create mode 100644 service_runtime/mmap_test.c diff --git a/service_runtime/SConscript b/service_runtime/SConscript index 5f6c0fe1..1fcd8f30 100644 --- a/service_runtime/SConscript +++ b/service_runtime/SConscript @@ -322,6 +322,8 @@ unit_tests_exe = env.ComponentProgram('service_runtime_tests', unittest_inputs, OTHER_LIBS = ['gtest']) env.Requires(unit_tests_exe, sdl_dll) +env.ComponentProgram('mmap_test', 'mmap_test.c') + # TODO: the "-d" argument does nothing, however Hammer doesn't run # the test if it is launched without any arguments. node = env.Command( diff --git a/service_runtime/mmap_test.c b/service_runtime/mmap_test.c new file mode 100644 index 00000000..b0225a3e --- /dev/null +++ b/service_runtime/mmap_test.c @@ -0,0 +1,140 @@ + +#include +#include +#include + +#include "gio.h" +#include "include/sys/fcntl.h" +#include "sel_ldr.h" +#include "nacl_app_thread.h" +#include "nacl_desc_effector_ldr.h" +#include "nacl_syscall_common.h" + + +/* Based on NaClAppThreadCtor() */ +static void InitThread(struct NaClApp *nap, struct NaClAppThread *natp) +{ + struct NaClDescEffectorLdr *effp; + + memset(natp, 0xff, sizeof(*natp)); + + natp->nap = nap; + + if (!NaClMutexCtor(&natp->mu)) { + assert(0); + } + if (!NaClCondVarCtor(&natp->cv)) { + assert(0); + } + + natp->is_privileged = 0; + natp->refcount = 1; + + effp = (struct NaClDescEffectorLdr *) malloc(sizeof *effp); + assert(effp != NULL); + + if (!NaClDescEffectorLdrCtor(effp, natp)) { + assert(0); + } + natp->effp = (struct NaClDescEffector *) effp; +} + +int main() +{ + char *nacl_file = "scons-out/nacl/staging/hello_world.nexe"; + struct GioMemoryFileSnapshot gf; + struct NaClApp state; + struct NaClAppThread nat, *natp = &nat; + int errcode; + unsigned int initial_addr; + unsigned int addr; + struct NaClVmmap *mem_map; + + NaClLogModuleInit(); + + errcode = GioMemoryFileSnapshotCtor(&gf, nacl_file); + assert(errcode != 0); + errcode = NaClAppCtor(&state); + assert(errcode != 0); + errcode = NaClAppLoadFile((struct Gio *) &gf, &state); + assert(errcode == 0); + + InitThread(&state, natp); + + /* Allocate range */ + addr = NaClSysMmap(natp, 0, NACL_PAGESIZE * 3, + NACL_ABI_PROT_READ | NACL_ABI_PROT_WRITE, + NACL_ABI_MAP_ANONYMOUS | NACL_ABI_MAP_PRIVATE, -1, 0); + printf("addr=0x%x\n", addr); + initial_addr = addr; + + /* Map to overwrite the start of the previously allocated range */ + addr = NaClSysMmap(natp, (void *) initial_addr, NACL_PAGESIZE * 2, + NACL_ABI_PROT_READ | NACL_ABI_PROT_WRITE, + NACL_ABI_MAP_ANONYMOUS | NACL_ABI_MAP_PRIVATE, -1, 0); + printf("addr=0x%x\n", addr); + assert(addr == initial_addr); + + /* Allocate new page */ + addr = NaClSysMmap(natp, 0, NACL_PAGESIZE, + NACL_ABI_PROT_READ | NACL_ABI_PROT_WRITE, + NACL_ABI_MAP_ANONYMOUS | NACL_ABI_MAP_PRIVATE, -1, 0); + printf("addr=0x%x\n", addr); + assert(addr == initial_addr - NACL_PAGESIZE); + + mem_map = &state.mem_map; + NaClVmmapMakeSorted(mem_map); + assert(mem_map->nvalid == 7); + /* skip stack */ + assert(mem_map->vmentry[3]->page_num == initial_addr / NACL_PAGESIZE - 1); + assert(mem_map->vmentry[3]->npages == 1); + assert(mem_map->vmentry[4]->page_num == initial_addr / NACL_PAGESIZE); + assert(mem_map->vmentry[4]->npages == 2); + assert(mem_map->vmentry[5]->page_num == initial_addr / NACL_PAGESIZE + 2); + assert(mem_map->vmentry[5]->npages == 1); + /* skip zero page, trampolines and executable */ + + errcode = NaClSysMunmap(natp, (void *) (initial_addr - NACL_PAGESIZE), + NACL_PAGESIZE * 4); + assert(errcode == 0); + assert(mem_map->nvalid == 4); + + + /* Allocate range */ + addr = NaClSysMmap(natp, 0, NACL_PAGESIZE * 9, + NACL_ABI_PROT_READ | NACL_ABI_PROT_WRITE, + NACL_ABI_MAP_ANONYMOUS | NACL_ABI_MAP_PRIVATE, -1, 0); + printf("addr=0x%x\n", addr); + initial_addr = addr; + + /* Map into middle of previously allocated range */ + addr = NaClSysMmap(natp, (void *) initial_addr + NACL_PAGESIZE * 2, + NACL_PAGESIZE * 3, + NACL_ABI_PROT_READ | NACL_ABI_PROT_WRITE, + NACL_ABI_MAP_ANONYMOUS | NACL_ABI_MAP_PRIVATE, -1, 0); + printf("addr=0x%x\n", addr); + assert(addr == initial_addr + NACL_PAGESIZE * 2); + + NaClVmmapMakeSorted(mem_map); + assert(mem_map->nvalid == 7); + assert(mem_map->vmentry[3]->page_num == initial_addr / NACL_PAGESIZE); + assert(mem_map->vmentry[3]->npages == 2); + assert(mem_map->vmentry[4]->page_num == initial_addr / NACL_PAGESIZE + 2); + assert(mem_map->vmentry[4]->npages == 3); + assert(mem_map->vmentry[5]->page_num == initial_addr / NACL_PAGESIZE + 5); + assert(mem_map->vmentry[5]->npages == 4); + + errcode = NaClSysMunmap(natp, (void *) initial_addr, NACL_PAGESIZE * 9); + assert(errcode == 0); + assert(mem_map->nvalid == 4); + + /* Check handling of zero-sized mappings. */ + addr = NaClSysMmap(natp, 0, 0, + NACL_ABI_PROT_READ | NACL_ABI_PROT_WRITE, + NACL_ABI_MAP_ANONYMOUS | NACL_ABI_MAP_PRIVATE, -1, 0); + assert((int) addr == -NACL_ABI_EINVAL); + errcode = NaClSysMunmap(natp, (void *) initial_addr, 0); + assert(errcode == -NACL_ABI_EINVAL); + + return 0; +} diff --git a/service_runtime/nacl_memory_object.c b/service_runtime/nacl_memory_object.c index 57cf063b..f4c885b8 100644 --- a/service_runtime/nacl_memory_object.c +++ b/service_runtime/nacl_memory_object.c @@ -102,6 +102,8 @@ struct NaClMemObj *NaClMemObjSplit(struct NaClMemObj *orig, off_t additional) { struct NaClMemObj *nmop; + if (orig == NULL) + return NULL; if (NULL == (nmop = malloc(sizeof *nmop))) { NaClLog(LOG_FATAL, ("NaClMemObjSplit: out of memory creating object" diff --git a/service_runtime/nacl_syscall_common.h b/service_runtime/nacl_syscall_common.h index 11872e66..2130ab97 100644 --- a/service_runtime/nacl_syscall_common.h +++ b/service_runtime/nacl_syscall_common.h @@ -149,6 +149,16 @@ int32_t NaClCommonSysMmap(struct NaClAppThread *natp, int flags, int d, nacl_abi_off_t offset); +int32_t NaClSysMmap(struct NaClAppThread *natp, + void *start, + size_t length, + int prot, + int flags, + int d, + nacl_abi_off_t offset); +int32_t NaClSysMunmap(struct NaClAppThread *natp, + void *start, + size_t length); int32_t NaClCommonSysGetdents(struct NaClAppThread *natp, int d, diff --git a/service_runtime/sel_mem.c b/service_runtime/sel_mem.c index 82bfc1f3..2c34eec4 100644 --- a/service_runtime/sel_mem.c +++ b/service_runtime/sel_mem.c @@ -34,6 +34,7 @@ #include "native_client/include/portability.h" +#include #include #include @@ -216,7 +217,7 @@ static void NaClVmmapRemoveMarked(struct NaClVmmap *self) #endif } -static void NaClVmmapMakeSorted(struct NaClVmmap *self) +void NaClVmmapMakeSorted(struct NaClVmmap *self) { if (self->is_sorted) return; @@ -271,48 +272,6 @@ int NaClVmmapAdd(struct NaClVmmap *self, * Update the virtual memory map. Deletion is handled by a remove * flag, since a NULL nmop just means that the memory is backed by the * system paging file. - * - * must handle overlapping cases. let s and e be the start and end - * page numbers of an existing entry, and S and E be the start and end - * of the update. - * - * first, we find the *last* entry where entry->s <= S. we have the - * following cases to consider: - * - * (0) no such entry. S < s for all s. - * - * (1) s e - * S E - * - * split; done. - * s==S, Envalid; - /* self->nvalid will change if/when we add entries */ - new_region_end_page = page_num + npages; - /* - * search for *last* entry with page_num <= input page_num. we - * could use a modified binary search, but standard bsearch cannot - * handle this. - */ - for (i = 0; i < nvalid; ++i) { - if (page_num < self->vmentry[i]->page_num) { + assert(npages > 0); + + for (i = 0; i < self->nvalid; i++) { + struct NaClVmmapEntry *ent = self->vmentry[i]; + uintptr_t ent_end_page = ent->page_num + ent->npages; + off_t additional_offset = + (new_region_end_page - ent->page_num) << NACL_PAGESHIFT; + + if (ent->page_num < page_num && new_region_end_page < ent_end_page) { + /* Split existing mapping into two parts, with new mapping in + the middle. */ + if(!NaClVmmapAdd(self, + new_region_end_page, + ent_end_page - new_region_end_page, + ent->prot, + NaClMemObjSplit(ent->nmop, additional_offset))) + NaClLog(LOG_FATAL, "NaClVmmapUpdate: could not split entry\n"); + ent->npages = page_num - ent->page_num; break; } - } - if (i == 0) { - /* case 0: all entries have higher starting addresses */ - if (!remove) { - if (!NaClVmmapAdd(self, page_num, npages, prot, nmop)) { - NaClLog(LOG_FATAL, "NaClVmmapUpdate: could not add entry (case 0)\n"); - return; - } - /* NB: added at end, nvalid caches old end. */ - } - /* fall through to handle overlap with following entries */ - } else { - /* i \in [1...nvalid], so i-1 \in [0...nvalid) */ - ent = self->vmentry[i-1]; - /* ent points to last entry with page_num <= input page_num */ - end_page = ent->page_num + ent->npages; - - if (ent->page_num == page_num) { - if (new_region_end_page < end_page) { - /* case (1) */ - ent->page_num = new_region_end_page; - if (remove) { - return; - } - if (!NaClVmmapAdd(self, page_num, npages, prot, nmop)) { - NaClLog(LOG_FATAL, "NaClVmmapUpdate: could not add entry (case 1)\n"); - return; - } - } else { - /* case (2) */ - if (!remove) { - /* take over the entry */ - ent->npages = npages; - ent->prot = prot; - if (NULL != ent->nmop) { - NaClMemObjDtor(ent->nmop); - free(ent->nmop); - } - ent->nmop = nmop; - } else { - /* mark as removed for later cleanup */ - ent->removed = 1; - } - } - /* fall through to handle overlap with following entries */ - } else if (page_num < end_page) { - if (new_region_end_page < end_page) { - /* case (3) */ - ent->npages = page_num - ent->page_num; - additional_off = ent->npages << NACL_PAGESHIFT; - if (!NaClVmmapAdd(self, - new_region_end_page, - end_page - new_region_end_page, - prot, - NaClMemObjSplit(ent->nmop, additional_off))) { - NaClLog(LOG_FATAL, - "NaClVmmapUpdate: could not add entry (case 3.1)\n"); - return; - } - if (!remove) { - if (!NaClVmmapAdd(self, page_num, npages, prot, nmop)) { - NaClLog(LOG_FATAL, - "NaClVmmapUpdate: could not add entry (case 3.2)\n"); - return; - } - /* NB: NaClVmmapAdd may have grown vmentry and thus invalidate ent */ - } - - return; - } else { - /* case (4) */ - ent->npages = page_num - ent->page_num; - if (!NaClVmmapAdd(self, - page_num, - npages, - prot, - nmop)) { - NaClLog(LOG_FATAL, "NaClVmmapUpdate: could not add entry (case 4)\n"); - return; - } - /* fall through to handle overlap with following entries */ - } - } else { - /* page_num <= end_page -- case (5) */ - if (!remove) { - if (!NaClVmmapAdd(self, page_num, npages, prot, nmop)) { - NaClLog(LOG_FATAL, "NaClVmmapUpdate: could not add entry (case 5)\n"); - return; - } - } - /* fall through to handle overlap with following entries */ + else if (ent->page_num < page_num && page_num < ent_end_page) { + /* New mapping overlaps end of existing mapping. */ + ent->npages = page_num - ent->page_num; } - } - hole_start = i; - hole_end = nvalid; - for ( ; i < nvalid; ++i) { - /* trailing delete / left truncation */ - ent = self->vmentry[i]; - end_page = ent->page_num + ent->npages; - if (new_region_end_page < end_page) { - hole_end = i; - /* - * prefix of current region covered by a previously added (or - * removed) region. - */ - additional_off = (new_region_end_page - ent->page_num) << NACL_PAGESHIFT; + else if (ent->page_num < new_region_end_page && + new_region_end_page < ent_end_page) { + /* New mapping overlaps start of existing mapping. */ + if(ent->nmop != NULL) + NaClMemObjIncOffset(ent->nmop, additional_offset); ent->page_num = new_region_end_page; - ent->npages = end_page - new_region_end_page; - if (NULL != ent->nmop) { - NaClMemObjIncOffset(ent->nmop, additional_off); - } + ent->npages = ent_end_page - new_region_end_page; break; } + else if (page_num <= ent->page_num && ent_end_page <= new_region_end_page) { + /* New mapping covers all of the existing mapping. */ + ent->removed = 1; + } + else { + /* No overlap */ + assert(new_region_end_page <= ent->page_num || ent_end_page <= page_num); + } } - if (hole_start < hole_end) { - for (i = hole_start; i < hole_end; ++i) { - self->vmentry[i]->removed = 1; - } - NaClVmmapRemoveMarked(self); + if (!remove) { + if (!NaClVmmapAdd(self, page_num, npages, prot, nmop)) + NaClLog(LOG_FATAL, "NaClVmmapUpdate: could not add entry\n"); } - return; + NaClVmmapRemoveMarked(self); } static int NaClVmmapContainCmpEntries(void const *vkey, diff --git a/service_runtime/sel_mem.h b/service_runtime/sel_mem.h index e82b2932..8c9ff049 100644 --- a/service_runtime/sel_mem.h +++ b/service_runtime/sel_mem.h @@ -136,6 +136,8 @@ uintptr_t NaClVmmapFindSpace(struct NaClVmmap *self, uintptr_t NaClVmmapFindMapSpace(struct NaClVmmap *self, size_t num_pages); +void NaClVmmapMakeSorted(struct NaClVmmap *self); + EXTERN_C_END #endif -- 2.11.4.GIT