Browse Source

[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.
Dmitrii Kuvaiskii 5 years ago
parent
commit
b871a962dd

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

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

+ 22 - 6
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;

+ 14 - 0
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)

+ 49 - 0
LibOS/shim/test/regression/getcwd.c

@@ -0,0 +1,49 @@
+#include <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+#include <linux/limits.h>
+#include <sys/mman.h>
+
+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;
+}