From b871a962dd1393dbbb319984c6ca348c920c92cc Mon Sep 17 00:00:00 2001 From: Dmitrii Kuvaiskii Date: Tue, 11 Jun 2019 20:12:59 -0700 Subject: [PATCH] [LibOS] Take into account adjacent VMAs on correct-memory check Previously the check on correct memory region under Linux-SGX (called is_in_one_vma()) only checked that the whole memory region fits into one VMA. In some cases (e.g. DATA and BSS sections), memory regions can span several adjacent VMAs. This commit refines the check and renames it to is_in_adjacent_vmas(). It also adds a LibOS test. --- LibOS/shim/include/shim_vma.h | 2 +- LibOS/shim/src/bookkeep/shim_signal.c | 4 +- LibOS/shim/src/bookkeep/shim_vma.c | 28 +++++++++++--- LibOS/shim/test/regression/30_getcwd.py | 14 +++++++ LibOS/shim/test/regression/getcwd.c | 49 +++++++++++++++++++++++++ 5 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 LibOS/shim/test/regression/30_getcwd.py create mode 100644 LibOS/shim/test/regression/getcwd.c diff --git a/LibOS/shim/include/shim_vma.h b/LibOS/shim/include/shim_vma.h index c0f0e081..7c91ab5e 100644 --- a/LibOS/shim/include/shim_vma.h +++ b/LibOS/shim/include/shim_vma.h @@ -128,7 +128,7 @@ int lookup_vma (void * addr, struct shim_vma_val * vma); int lookup_overlap_vma (void * addr, size_t length, struct shim_vma_val * vma); /* True if [addr, addr+length) is found in one VMA (valid memory region) */ -bool is_in_one_vma (void * addr, size_t length); +bool is_in_adjacent_vmas (void * addr, size_t length); /* * Looking for an unmapped space and then adding the corresponding bookkeeping diff --git a/LibOS/shim/src/bookkeep/shim_signal.c b/LibOS/shim/src/bookkeep/shim_signal.c index a6e8d61a..e57ccfd4 100644 --- a/LibOS/shim/src/bookkeep/shim_signal.c +++ b/LibOS/shim/src/bookkeep/shim_signal.c @@ -348,7 +348,7 @@ bool test_user_memory (void * addr, size_t size, bool write) /* SGX path: check if [addr, addr+size) is addressable (in some VMA) */ if (is_sgx_pal()) - return !is_in_one_vma(addr, size); + return !is_in_adjacent_vmas(addr, size); /* Non-SGX path: check if [addr, addr+size) is addressable by touching * a byte of each page; invalid access will be caught in memfault_upcall */ @@ -406,7 +406,7 @@ bool test_user_string (const char * addr) do { maxlen = next - addr; - if (!access_ok(addr, maxlen) || !is_in_one_vma((void*) addr, maxlen)) + if (!access_ok(addr, maxlen) || !is_in_adjacent_vmas((void*) addr, maxlen)) return true; size = strnlen(addr, maxlen); diff --git a/LibOS/shim/src/bookkeep/shim_vma.c b/LibOS/shim/src/bookkeep/shim_vma.c index 76a75cf7..12ac7018 100644 --- a/LibOS/shim/src/bookkeep/shim_vma.c +++ b/LibOS/shim/src/bookkeep/shim_vma.c @@ -992,16 +992,32 @@ int lookup_overlap_vma (void * addr, size_t length, struct shim_vma_val * res) return 0; } -bool is_in_one_vma (void * addr, size_t length) +bool is_in_adjacent_vmas (void * addr, size_t length) { struct shim_vma* vma; - + struct shim_vma* prev = NULL; lock(&vma_list_lock); - LISTP_FOR_EACH_ENTRY(vma, &vma_list, list) - if (test_vma_contain(vma, addr, addr + length)) { - unlock(&vma_list_lock); - return true; + + /* we rely on the fact that VMAs are sorted (for adjacent VMAs) */ + assert_vma_list(); + + LISTP_FOR_EACH_ENTRY(vma, &vma_list, list) { + if (addr >= vma->start && addr < vma->end) { + assert(prev == NULL); + prev = vma; } + if (prev) { + if (prev != vma && prev->end != vma->start) { + /* prev and current VMAs are not adjacent */ + break; + } + if ((addr + length) > vma->start && (addr + length) <= vma->end) { + unlock(&vma_list_lock); + return true; + } + prev = vma; + } + } unlock(&vma_list_lock); return false; diff --git a/LibOS/shim/test/regression/30_getcwd.py b/LibOS/shim/test/regression/30_getcwd.py new file mode 100644 index 00000000..c763b23c --- /dev/null +++ b/LibOS/shim/test/regression/30_getcwd.py @@ -0,0 +1,14 @@ +import os, sys, mmap +from regression import Regression + +loader = sys.argv[1] + +# Running stat +regression = Regression(loader, "getcwd") + +regression.add_check(name="Getcwd syscall", + check=lambda res: "[bss_cwd_buf] getcwd succeeded: /" in res[0].out and \ + "[mmapped_cwd_buf] getcwd succeeded: /" in res[0].out) + +rv = regression.run_checks() +if rv: sys.exit(rv) diff --git a/LibOS/shim/test/regression/getcwd.c b/LibOS/shim/test/regression/getcwd.c new file mode 100644 index 00000000..4953e2d4 --- /dev/null +++ b/LibOS/shim/test/regression/getcwd.c @@ -0,0 +1,49 @@ +#include +#include +#include +#include +#include + +static char bss_cwd_buf[PATH_MAX]; + +int main(int argc, char** argv) { + char* cwd = NULL; + + /* Option 1: use global variable. + * bss_cwd_buf resides in BSS section which starts right after DATA section; + * under Linux-SGX, BSS section is in a separate VMA from DATA section but + * cwd_buf spans both sections. This checks the correctness of internal + * test_user_memory() spanning several adjacent VMAs. */ + cwd = getcwd(bss_cwd_buf, sizeof(bss_cwd_buf)); + if (!cwd) { + perror("[bss_cwd_buf] getcwd failed\n"); + } else { + printf("[bss_cwd_buf] getcwd succeeded: %s\n", cwd); + } + + /* Option 2: use 2-page mmapped variable. + * mmapped_cwd_buf resides on the heap and occupies two consecutive pages; + * we divide the original single VMA into two adjacent VMAs via mprotect(). + * This checks the correctness of internal test_user_memory() spanning + * several adjacent VMAs. */ + void* mmapped_cwd_buf = mmap(NULL, 4096 * 2, PROT_READ | PROT_WRITE | PROT_EXEC, + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + if (mmapped_cwd_buf == MAP_FAILED) { + perror("mmap failed\n"); + return 1; + } + int ret = mprotect(mmapped_cwd_buf, 4096, PROT_READ | PROT_WRITE); + if (ret < 0) { + perror("mprotect failed\n"); + return 1; + } + cwd = getcwd(mmapped_cwd_buf, 4096 * 2); + if (!cwd) { + perror("[mmapped_cwd_buf] getcwd failed\n"); + } else { + printf("[mmapped_cwd_buf] getcwd succeeded: %s\n", cwd); + } + + munmap(mmapped_cwd_buf, 4096 * 2); + return 0; +}