Fuchsia: Implement memory region protections

runtime/vm/virtual_memory_fuchsia.cc

Index: runtime/vm/virtual_memory_fuchsia.cc
diff --git a/runtime/vm/virtual_memory_fuchsia.cc b/runtime/vm/virtual_memory_fuchsia.cc
index 0e627ecc81026f061c2450b3a2f54f57dac0ea5c..8bfff06b0cf49cd20159c546c179cab35bd84440 100644
--- a/runtime/vm/virtual_memory_fuchsia.cc
+++ b/runtime/vm/virtual_memory_fuchsia.cc
@@ -13,9 +13,13 @@
 #include <unistd.h>
 
 #include "platform/assert.h"
+#include "vm/allocation.h"
+#include "vm/growable_array.h"
 #include "vm/isolate.h"
+#include "vm/lockers.h"
 #include "vm/memory_region.h"
 #include "vm/os.h"
+#include "vm/os_thread.h"
 
 // #define VIRTUAL_MEMORY_LOGGING 1
 #if defined(VIRTUAL_MEMORY_LOGGING)
@@ -30,6 +34,115 @@
 
 namespace dart {
 
+// The Magenta system call to protect memory regions (mx_vmar_protect) takes a
+// VM area (vmar) handle as first argument. We call VirtualMemory::Protect()
+// from the memory freelist code in vm/freelist.cc where the vmar handle is not
+// available. Additionally, there is no mx_vmar system call to retrieve a handle
+// for the leaf vmar given an address. Thus, when memory protections are
+// enabled, we maintain a sorted list of our leaf vmar handles that we can
+// query by address in calls to VirtualMemory::Protect().
+class VmarList : public AllStatic {
+ public:
+  static void AddVmar(mx_handle_t vmar, uword addr, intptr_t size);
+  static void RemoveVmar(uword addr);
+  static mx_handle_t LookupVmar(uword addr);
+
+ private:
+  static intptr_t LookupVmarIndexLocked(uword addr);
+
+  struct VmarListElement {
+    mx_handle_t vmar;
+    uword addr;
+    intptr_t size;
+  };
+
+  static Mutex* vmar_array_lock_;
+  static MallocGrowableArray<VmarListElement> vmar_array_;
+};
+
+Mutex* VmarList::vmar_array_lock_ = new Mutex();
+MallocGrowableArray<VmarList::VmarListElement> VmarList::vmar_array_;
+
+void VmarList::AddVmar(mx_handle_t vmar, uword addr, intptr_t size) {
+  MutexLocker ml(vmar_array_lock_);
+  LOG_INFO("AddVmar(%d, %lx, %ld)\n", vmar, addr, size);
+  // Sorted insert in increasing order.
+  const intptr_t length = vmar_array_.length();
+  intptr_t idx;
+  for (idx = 0; idx < length; idx++) {
+    const VmarListElement& m = vmar_array_.At(idx);
+    if (m.addr >= addr) {
+      break;
+    }
+  }
+#if defined(DEBUG)
+  if ((length > 0) && (idx < (length - 1))) {
+    const VmarListElement& m = vmar_array_.At(idx);
+    ASSERT(m.addr != addr);
+  }
+#endif
+  LOG_INFO("AddVmar(%d, %lx, %ld) at index = %ld\n", vmar, addr, size, idx);
+  VmarListElement new_mapping;
+  new_mapping.vmar = vmar;
+  new_mapping.addr = addr;
+  new_mapping.size = size;
+  vmar_array_.InsertAt(idx, new_mapping);
+}
+
+
+intptr_t VmarList::LookupVmarIndexLocked(uword addr) {
+  // Binary search for the vmar containing addr.
+  intptr_t imin = 0;
+  intptr_t imax = vmar_array_.length();
+  while (imax >= imin) {
+    const intptr_t imid = ((imax - imin) / 2) + imin;
+    const VmarListElement& mapping = vmar_array_.At(imid);
+    if ((mapping.addr + mapping.size) <= addr) {
+      imin = imid + 1;
+    } else if (mapping.addr > addr) {
+      imax = imid - 1;
+    } else {
+      return imid;
+    }
+  }
+  return -1;
+}
+
+
+mx_handle_t VmarList::LookupVmar(uword addr) {
+  MutexLocker ml(vmar_array_lock_);
+  LOG_INFO("LookupVmar(%lx)\n", addr);
+  const intptr_t idx = LookupVmarIndexLocked(addr);
+  if (idx == -1) {
+    LOG_ERR("LookupVmar(%lx) NOT FOUND\n", addr);
+    return MX_HANDLE_INVALID;
+  }
+  LOG_INFO("LookupVmar(%lx) found at %ld\n", addr, idx);
+  return vmar_array_[idx].vmar;
+}
+
+
+void VmarList::RemoveVmar(uword addr) {
+  MutexLocker ml(vmar_array_lock_);
+  LOG_INFO("RemoveVmar(%lx)\n", addr);
+  const intptr_t idx = LookupVmarIndexLocked(addr);
+  ASSERT(idx != -1);
+#if defined(DEBUG)
+  mx_handle_t vmar = vmar_array_[idx].vmar;
+#endif
+  // Swap idx to the end, and then RemoveLast()
+  const intptr_t length = vmar_array_.length();
+  for (intptr_t i = idx; i < length - 1; i++) {
+    vmar_array_.Swap(i, i + 1);
+  }
+#if defined(DEBUG)
+  const VmarListElement& mapping = vmar_array_.Last();
+  ASSERT(mapping.vmar == vmar);
+#endif
+  vmar_array_.RemoveLast();
+}
+
+
 uword VirtualMemory::page_size_ = 0;
 
 
@@ -51,7 +164,7 @@ VirtualMemory* VirtualMemory::ReserveInternal(intptr_t size) {
             mx_status_get_string(status));
     return NULL;
   }
-
+  VmarList::AddVmar(vmar, addr, size);
   MemoryRegion region(reinterpret_cast<void*>(addr), size);
   return new VirtualMemory(region, vmar);
 }
@@ -68,6 +181,7 @@ VirtualMemory::~VirtualMemory() {
     if (status != NO_ERROR) {
       LOG_ERR("mx_handle_close failed: %s\n", mx_status_get_string(status));
     }
+    VmarList::RemoveVmar(start());
   }
 }
 
@@ -99,8 +213,10 @@ bool VirtualMemory::Commit(uword addr, intptr_t size, bool executable) {
 
   mx_handle_t vmar = static_cast<mx_handle_t>(handle());
   const size_t offset = addr - start();
-  const uint32_t flags = MX_VM_FLAG_SPECIFIC | MX_VM_FLAG_PERM_READ |
-                         MX_VM_FLAG_PERM_WRITE | MX_VM_FLAG_PERM_EXECUTE;
+  const uint32_t flags = MX_VM_FLAG_SPECIFIC |
+                         MX_VM_FLAG_PERM_READ |
+                         MX_VM_FLAG_PERM_WRITE |
+                         (executable ? MX_VM_FLAG_PERM_EXECUTE : 0);
   uintptr_t mapped_addr;
   status = mx_vmar_map(vmar, offset, vmo, 0, size, flags, &mapped_addr);
   if (status != NO_ERROR) {
@@ -110,15 +226,56 @@ bool VirtualMemory::Commit(uword addr, intptr_t size, bool executable) {
     return false;
   }
   if (addr != mapped_addr) {
+    mx_handle_close(vmo);
     LOG_ERR("mx_vmar_map: addr != mapped_addr: %lx != %lx\n", addr,
             mapped_addr);
     return false;
   }
+  mx_handle_close(vmo);
+  LOG_INFO("Commit(%lx, %ld, %s): success\n",
+      addr, size, executable ? "executable" : "");
   return true;
 }
 
 
 bool VirtualMemory::Protect(void* address, intptr_t size, Protection mode) {
+  ASSERT(Thread::Current()->IsMutatorThread() ||
+         Isolate::Current()->mutator_thread()->IsAtSafepoint());
+  const uword start_address = reinterpret_cast<uword>(address);
+  const uword end_address = start_address + size;
+  const uword page_address = Utils::RoundDown(start_address, PageSize());
+  mx_handle_t vmar = VmarList::LookupVmar(page_address);
+  ASSERT(vmar != MX_HANDLE_INVALID);
+  uint32_t prot = 0;
+  switch (mode) {
+    case kNoAccess:
+      prot = MX_VM_FLAG_PERM_READ;

[rmacnak, 2017/01/25 01:46:19]

I would expect "0", but the wording of the syscall doc suggests at least one
permission is required. Magenta bug?

[zra, 2017/01/25 05:44:03]

Not clear if it is a bug, or simply not yet implemented, but I've added a
comment with the issue number.

+      break;
+    case kReadOnly:
+      prot = MX_VM_FLAG_PERM_READ;
+      break;
+    case kReadWrite:
+      prot = MX_VM_FLAG_PERM_READ | MX_VM_FLAG_PERM_WRITE;
+      break;
+    case kReadExecute:
+      prot = MX_VM_FLAG_PERM_READ | MX_VM_FLAG_PERM_EXECUTE;
+      break;
+    case kReadWriteExecute:
+      prot = MX_VM_FLAG_PERM_READ |
+             MX_VM_FLAG_PERM_WRITE |
+             MX_VM_FLAG_PERM_EXECUTE;
+      break;
+  }
+  mx_status_t status =
+      mx_vmar_protect(vmar, page_address, end_address - page_address, prot);
+  if (status != NO_ERROR) {
+    LOG_ERR("mx_vmar_protect(%lx, %lx, %x) success: %s\n",
+        page_address, end_address - page_address,
+        prot, mx_status_get_string(status));
+    return false;
+  }
+  LOG_INFO("mx_vmar_protect(%lx, %lx, %x) success\n",
+        page_address, end_address - page_address, prot);
   return true;
 }