Stop allocating RW memory in AllocateGuarded

AllocateGuarded previously fell back on Allocate and then called Guard
to set the protection to PROT_NONE. Linux commits RW memory, but the
important thing here is to reserve the address space without committing
it. This change adds a new variant of Allocate that takes explicit
permission bits so that AllocateGuarded allocates non-RW memory from the
beginning.

Bug: v8:6320
Change-Id: I7962acbed09938951bf3bb4af2d1f302adba2547
Reviewed-on: https://chromium-review.googlesource.com/491928
Commit-Queue: Eric Holk <eholk@chromium.org>
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45075}

src/base/platform/platform-aix.cc

@@ -71,9 +71,10 @@ double AIXTimezoneCache::LocalTimeOffset() {
 
 TimezoneCache* OS::CreateTimezoneCache() { return new AIXTimezoneCache(); }
 
-void* OS::Allocate(const size_t requested, size_t* allocated, bool executable) {
+void* OS::Allocate(const size_t requested, size_t* allocated,
+                   OS::MemoryPermission access) {
   const size_t msize = RoundUp(requested, getpagesize());
-  int prot = PROT_READ | PROT_WRITE | (executable ? PROT_EXEC : 0);
+  int prot = GetProtectionFromMemoryPermission(access);
   void* mbase = mmapHelper(msize, prot, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
   if (mbase == MAP_FAILED) return NULL;

src/base/platform/platform-cygwin.cc

@@ -55,12 +55,10 @@ double CygwinTimezoneCache::LocalTimeOffset() {
                              (loc->tm_isdst > 0 ? 3600 * msPerSecond : 0));
 }
 
-
-void* OS::Allocate(const size_t requested,
-                   size_t* allocated,
-                   bool is_executable) {
+void* OS::Allocate(const size_t requested, size_t* allocated,
+                   OS::MemoryPermission access) {
   const size_t msize = RoundUp(requested, sysconf(_SC_PAGESIZE));
-  int prot = PROT_READ | PROT_WRITE | (is_executable ? PROT_EXEC : 0);
+  int prot = GetProtectionFromMemoryPermission(access);
   void* mbase = mmap(NULL, msize, prot, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
   if (mbase == MAP_FAILED) return NULL;
   *allocated = msize;

src/base/platform/platform-freebsd.cc

@@ -37,11 +37,10 @@ namespace base {
 
 TimezoneCache* OS::CreateTimezoneCache() { return new PosixTimezoneCache(); }
 
-void* OS::Allocate(const size_t requested,
-                   size_t* allocated,
-                   bool executable) {
+void* OS::Allocate(const size_t requested, size_t* allocated,
+                   OS::MemoryPermission access) {
   const size_t msize = RoundUp(requested, getpagesize());
-  int prot = PROT_READ | PROT_WRITE | (executable ? PROT_EXEC : 0);
+  int prot = GetProtectionFromMemoryPermission(access);
   void* mbase = mmap(NULL, msize, prot, MAP_PRIVATE | MAP_ANON, -1, 0);
 
   if (mbase == MAP_FAILED) return NULL;

src/base/platform/platform-linux.cc

@@ -95,9 +95,9 @@ bool OS::ArmUsingHardFloat() {
 TimezoneCache* OS::CreateTimezoneCache() { return new PosixTimezoneCache(); }
 
 void* OS::Allocate(const size_t requested, size_t* allocated,
-                   bool is_executable) {
+                   OS::MemoryPermission access) {
   const size_t msize = RoundUp(requested, AllocateAlignment());
-  int prot = PROT_READ | PROT_WRITE | (is_executable ? PROT_EXEC : 0);
+  int prot = GetProtectionFromMemoryPermission(access);
   void* addr = OS::GetRandomMmapAddr();
   void* mbase = mmap(addr, msize, prot, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
   if (mbase == MAP_FAILED) return NULL;

src/base/platform/platform-macos.cc

@@ -51,12 +51,10 @@ namespace base {
 static const int kMmapFd = VM_MAKE_TAG(255);
 static const off_t kMmapFdOffset = 0;
 
-
-void* OS::Allocate(const size_t requested,
-                   size_t* allocated,
-                   bool is_executable) {
+void* OS::Allocate(const size_t requested, size_t* allocated,
+                   OS::MemoryPermission access) {
   const size_t msize = RoundUp(requested, getpagesize());
-  int prot = PROT_READ | PROT_WRITE | (is_executable ? PROT_EXEC : 0);
+  int prot = GetProtectionFromMemoryPermission(access);
   void* mbase = mmap(OS::GetRandomMmapAddr(),
                      msize,
                      prot,

src/base/platform/platform-openbsd.cc

@@ -35,11 +35,10 @@ namespace base {
 
 TimezoneCache* OS::CreateTimezoneCache() { return new PosixTimezoneCache(); }
 
-void* OS::Allocate(const size_t requested,
-                   size_t* allocated,
-                   bool is_executable) {
+void* OS::Allocate(const size_t requested, size_t* allocated,
+                   OS::MemoryPermission access) {
   const size_t msize = RoundUp(requested, AllocateAlignment());
-  int prot = PROT_READ | PROT_WRITE | (is_executable ? PROT_EXEC : 0);
+  int prot = GetProtectionFromMemoryPermission(access);
   void* addr = OS::GetRandomMmapAddr();
   void* mbase = mmap(addr, msize, prot, MAP_PRIVATE | MAP_ANON, -1, 0);
   if (mbase == MAP_FAILED) return NULL;

src/base/platform/platform-posix.cc

@@ -101,10 +101,17 @@ intptr_t OS::CommitPageSize() {
   return page_size;
 }
 
+void* OS::Allocate(const size_t requested, size_t* allocated,
+                   bool is_executable) {
+  return OS::Allocate(requested, allocated,
+                      is_executable ? OS::MemoryPermission::kReadWriteExecute
+                                    : OS::MemoryPermission::kReadWrite);
+}
+
 void* OS::AllocateGuarded(const size_t requested) {
   size_t allocated = 0;
-  const bool is_executable = false;
-  void* mbase = OS::Allocate(requested, &allocated, is_executable);
+  void* mbase =
+      OS::Allocate(requested, &allocated, OS::MemoryPermission::kNoAccess);
   if (allocated != requested) {
     OS::Free(mbase, allocated);
     return nullptr;
@@ -112,7 +119,6 @@ void* OS::AllocateGuarded(const size_t requested) {
   if (mbase == nullptr) {
     return nullptr;
   }
-  OS::Guard(mbase, requested);
   return mbase;
 }
 
@@ -776,5 +782,17 @@ void Thread::SetThreadLocal(LocalStorageKey key, void* value) {
   USE(result);
 }
 
+int GetProtectionFromMemoryPermission(OS::MemoryPermission access) {
+  switch (access) {
+    case OS::MemoryPermission::kNoAccess:
+      return PROT_NONE;
+    case OS::MemoryPermission::kReadWrite:
+      return PROT_READ | PROT_WRITE;
+    case OS::MemoryPermission::kReadWriteExecute:
+      return PROT_READ | PROT_WRITE | PROT_EXEC;
+  }
+  UNREACHABLE();
+}
+
 }  // namespace base
 }  // namespace v8

src/base/platform/platform-posix.h

@@ -5,6 +5,7 @@
 #ifndef V8_BASE_PLATFORM_PLATFORM_POSIX_H_
 #define V8_BASE_PLATFORM_PLATFORM_POSIX_H_
 
+#include "src/base/platform/platform.h"
 #include "src/base/timezone-cache.h"
 
 namespace v8 {
@@ -22,6 +23,8 @@ class PosixTimezoneCache : public TimezoneCache {
   static const int msPerSecond = 1000;
 };
 
+int GetProtectionFromMemoryPermission(OS::MemoryPermission access);
+
 }  // namespace base
 }  // namespace v8
 

src/base/platform/platform-qnx.cc

@@ -86,11 +86,10 @@ bool OS::ArmUsingHardFloat() {
 
 TimezoneCache* OS::CreateTimezoneCache() { return new PosixTimezoneCache(); }
 
-void* OS::Allocate(const size_t requested,
-                   size_t* allocated,
-                   bool is_executable) {
+void* OS::Allocate(const size_t requested, size_t* allocated,
+                   OS::MemoryPermission access) {
   const size_t msize = RoundUp(requested, AllocateAlignment());
-  int prot = PROT_READ | PROT_WRITE | (is_executable ? PROT_EXEC : 0);
+  int prot = GetProtectionFromMemoryPermission(access);
   void* addr = OS::GetRandomMmapAddr();
   void* mbase = mmap(addr, msize, prot, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
   if (mbase == MAP_FAILED) return NULL;

src/base/platform/platform-solaris.cc

@@ -58,11 +58,10 @@ double SolarisTimezoneCache::LocalTimeOffset() {
 
 TimezoneCache* OS::CreateTimezoneCache() { return new SolarisTimezoneCache(); }
 
-void* OS::Allocate(const size_t requested,
-                   size_t* allocated,
-                   bool is_executable) {
+void* OS::Allocate(const size_t requested, size_t* allocated,
+                   OS::MemoryPermission access) {
   const size_t msize = RoundUp(requested, getpagesize());
-  int prot = PROT_READ | PROT_WRITE | (is_executable ? PROT_EXEC : 0);
+  int prot = GetProtectionFromMemoryPermission(access);
   void* mbase = mmap(NULL, msize, prot, MAP_PRIVATE | MAP_ANON, -1, 0);
 
   if (mbase == MAP_FAILED) return NULL;

src/base/platform/platform-win32.cc

@@ -764,15 +764,34 @@ static void* RandomizedVirtualAlloc(size_t size, int action, int protection) {
   return base;
 }
 
-
-void* OS::Allocate(const size_t requested,
-                   size_t* allocated,
+void* OS::Allocate(const size_t requested, size_t* allocated,
                    bool is_executable) {
+  return OS::Allocate(requested, allocated,
+                      is_executable ? OS::MemoryPermission::kReadWriteExecute
+                                    : OS::MemoryPermission::kReadWrite);
+}
+
+void* OS::Allocate(const size_t requested, size_t* allocated,
+                   OS::MemoryPermission access) {
   // VirtualAlloc rounds allocated size to page size automatically.
   size_t msize = RoundUp(requested, static_cast<int>(GetPageSize()));
 
   // Windows XP SP2 allows Data Excution Prevention (DEP).
-  int prot = is_executable ? PAGE_EXECUTE_READWRITE : PAGE_READWRITE;
+  int prot = PAGE_NOACCESS;
+  switch (access) {
+    case OS::MemoryPermission::kNoAccess: {
+      prot = PAGE_NOACCESS;
+      break;
+    }
+    case OS::MemoryPermission::kReadWrite: {
+      prot = PAGE_READWRITE;
+      break;
+    }
+    case OS::MemoryPermission::kReadWriteExecute: {
+      prot = PAGE_EXECUTE_READWRITE;
+      break;
+    }
+  }
 
   LPVOID mbase = RandomizedVirtualAlloc(msize,
                                         MEM_COMMIT | MEM_RESERVE,

src/base/platform/platform.h

@@ -158,6 +158,14 @@ class V8_BASE_EXPORT OS {
   static PRINTF_FORMAT(1, 2) void PrintError(const char* format, ...);
   static PRINTF_FORMAT(1, 0) void VPrintError(const char* format, va_list args);
 
+  // Memory access permissions. Only the modes currently used by V8 are listed
+  // here even though most systems support additional modes.
+  enum class MemoryPermission { kNoAccess, kReadWrite, kReadWriteExecute };
+
+  // Allocate/Free memory used by JS heap. Permissions are set according to the
+  // is_* flags. Returns the address of allocated memory, or NULL if failed.
+  static void* Allocate(const size_t requested, size_t* allocated,
+                        MemoryPermission access);
   // Allocate/Free memory used by JS heap. Pages are readable/writable, but
   // they are not guaranteed to be executable unless 'executable' is true.
   // Returns the address of allocated memory, or NULL if failed.
@@ -167,8 +175,8 @@ class V8_BASE_EXPORT OS {
   static void Free(void* address, const size_t size);
 
   // Allocates a region of memory that is inaccessible. On Windows this reserves
-  // but does not commit the memory. On Linux, it is equivalent to a call to
-  // Allocate() followed by Guard().
+  // but does not commit the memory. On POSIX systems it allocates memory as
+  // PROT_NONE, which also prevents it from being committed.
   static void* AllocateGuarded(const size_t requested);
 
   // This is the granularity at which the ProtectCode(...) call can set page

test/unittests/base/platform/platform-unittest.cc

@@ -5,6 +5,8 @@
 #include "src/base/platform/platform.h"
 
 #if V8_OS_POSIX
+#include <setjmp.h>
+#include <signal.h>
 #include <unistd.h>  // NOLINT
 #endif
 
@@ -96,5 +98,106 @@ TEST_F(ThreadLocalStorageTest, DoTest) {
   Join();
 }
 
+#if V8_OS_POSIX
+// TODO(eholk): Add a windows version of these tests
+
+namespace {
+
+// These tests make sure the routines to allocate memory do so with the correct
+// permissions.
+//
+// Unfortunately, there is no API to find the protection of a memory address,
+// so instead we test permissions by installing a signal handler, probing a
+// memory location and recovering from the fault.
+//
+// We don't test the execution permission because to do so we'd have to
+// dynamically generate code and test if we can execute it.
+
+class MemoryAllocationPermissionsTest : public ::testing::Test {
+  static void SignalHandler(int signal, siginfo_t* info, void*) {
+    siglongjmp(continuation_, 1);
+  }
+  struct sigaction old_action_;
+// On Mac, sometimes we get SIGBUS instead of SIGSEGV.
+#if V8_OS_MACOSX
+  struct sigaction old_bus_action_;
+#endif
+
+ protected:
+  virtual void SetUp() {
+    struct sigaction action;
+    action.sa_sigaction = SignalHandler;
+    sigemptyset(&action.sa_mask);
+    action.sa_flags = SA_SIGINFO;
+    sigaction(SIGSEGV, &action, &old_action_);
+#if V8_OS_MACOSX
+    sigaction(SIGBUS, &action, &old_bus_action_);
+#endif
+  }
+
+  virtual void TearDown() {
+    // be a good citizen and restore the old signal handler.
+    sigaction(SIGSEGV, &old_action_, nullptr);
+#if V8_OS_MACOSX
+    sigaction(SIGBUS, &old_bus_action_, nullptr);
+#endif
+  }
+
+ public:
+  static sigjmp_buf continuation_;
+
+  enum class MemoryAction { kRead, kWrite };
+
+  void ProbeMemory(volatile int* buffer, MemoryAction action,
+                   bool should_succeed) {
+    const int save_sigs = 1;
+    if (!sigsetjmp(continuation_, save_sigs)) {
+      switch (action) {
+        case MemoryAction::kRead: {
+          USE(*buffer);
+          break;
+        }
+        case MemoryAction::kWrite: {
+          *buffer = 0;
+          break;
+        }
+      }
+      if (should_succeed) {
+        SUCCEED();
+      } else {
+        FAIL();
+      }
+      return;
+    }
+    if (should_succeed) {
+      FAIL();
+    } else {
+      SUCCEED();
+    }
+  }
+
+  void TestPermissions(OS::MemoryPermission permission, bool can_read,
+                       bool can_write) {
+    const size_t allocation_size = OS::CommitPageSize();
+    size_t actual = 0;
+    int* buffer =
+        static_cast<int*>(OS::Allocate(allocation_size, &actual, permission));
+    ProbeMemory(buffer, MemoryAction::kRead, can_read);
+    ProbeMemory(buffer, MemoryAction::kWrite, can_write);
+    OS::Free(buffer, actual);
+  }
+};
+
+sigjmp_buf MemoryAllocationPermissionsTest::continuation_;
+
+TEST_F(MemoryAllocationPermissionsTest, DoTest) {
+  TestPermissions(OS::MemoryPermission::kNoAccess, false, false);
+  TestPermissions(OS::MemoryPermission::kReadWrite, true, true);
+  TestPermissions(OS::MemoryPermission::kReadWriteExecute, true, true);
+}
+
+}  // namespace
+#endif  // V8_OS_POSIX
+
 }  // namespace base
 }  // namespace v8