Give base::SharedMemory::CreateAnonymous an executable flag

NaCl on Mac and Linux needs to create a shared memory object that it can
later make executable with mprotect.  Express this need in the interface it
uses.  Add a test that pages mapped from such an object can later be passed
to mprotect with PROT_EXEC.

This lays the groundwork for a later change that will sometimes use a
different method to allocate an object on Linux when it needs to be
executable.  On some Linux distributions, shm_open yields objects whose
mappings cannot be made executable.

BUG= http://code.google.com/p/chromium/issues/detail?id=103377
TEST= SharedMemory.AnonymousExecutable

R=mark@chromium.org,jam@chromium.org,amit@chromium.org,ben@chromium.org

Review URL: http://codereview.chromium.org/8585002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@112570 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 83 insertions, 32 deletions

diff --git a/base/shared_memory.h b/base/shared_memory.h
index c9dfd6e..8cb27ec 100644
--- a/base/shared_memory.h
+++ b/base/shared_memory.h
@@ -38,6 +38,29 @@ typedef ino_t SharedMemoryId;
 // needed.
 #endif
 
+// Options for creating a shared memory object.
+struct SharedMemoryCreateOptions {
+  SharedMemoryCreateOptions() : name(NULL), size(0), open_existing(false),
+                                executable(false) {}
+
+  // If NULL, the object is anonymous.  This pointer is owned by the caller
+  // and must live through the call to Create().
+  const std::string* name;
+
+  // Size of the shared memory object to be created.
+  // When opening an existing object, this has no effect.
+  uint32 size;
+
+  // If true, and the shared memory already exists, Create() will open the
+  // existing shared memory and ignore the size parameter.  If false,
+  // shared memory must not exist.  This flag is meaningless unless name is
+  // non-NULL.
+  bool open_existing;
+
+  // If true, mappings might need to be made executable later.
+  bool executable;
+};
+
 // Platform abstraction for shared memory.  Provides a C++ wrapper
 // around the OS primitive for a memory mapped file.
 class BASE_EXPORT SharedMemory {
@@ -74,13 +97,21 @@ class BASE_EXPORT SharedMemory {
   // Closes a shared memory handle.
   static void CloseHandle(const SharedMemoryHandle& handle);
 
+  // Creates a shared memory object as described by the options struct.
+  // Returns true on success and false on failure.
+  bool Create(const SharedMemoryCreateOptions& options);
+
   // Creates and maps an anonymous shared memory segment of size size.
   // Returns true on success and false on failure.
   bool CreateAndMapAnonymous(uint32 size);
 
   // Creates an anonymous shared memory segment of size size.
   // Returns true on success and false on failure.
-  bool CreateAnonymous(uint32 size);
+  bool CreateAnonymous(uint32 size) {
+    SharedMemoryCreateOptions options;
+    options.size = size;
+    return Create(options);
+  }
 
   // Creates or opens a shared memory segment based on a name.
   // If open_existing is true, and the shared memory already exists,
@@ -88,7 +119,13 @@ class BASE_EXPORT SharedMemory {
   // If open_existing is false, shared memory must not exist.
   // size is the size of the block to be created.
   // Returns true on success, false on failure.
-  bool CreateNamed(const std::string& name, bool open_existing, uint32 size);
+  bool CreateNamed(const std::string& name, bool open_existing, uint32 size) {
+    SharedMemoryCreateOptions options;
+    options.name = &name;
+    options.open_existing = open_existing;
+    options.size = size;
+    return Create(options);
+  }
 
   // Deletes resources associated with a shared memory segment based on name.
   // Not all platforms require this call.
diff --git a/base/shared_memory_android.cc b/base/shared_memory_android.cc
index 72c3a56..8e55e3a 100644
--- a/base/shared_memory_android.cc
+++ b/base/shared_memory_android.cc
@@ -16,12 +16,13 @@ namespace base {
 // all the file descriptors from different processes associated with the region
 // are closed, the memory buffer will go away.
 
-bool SharedMemory::CreateNamed(const std::string& name,
-                               bool open_existing, uint32 size) {
+bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
   DCHECK_EQ(-1, mapped_file_ );
 
   // "name" is just a label in ashmem. It is visible in /proc/pid/maps.
-  mapped_file_ = ashmem_create_region(name.c_str(), size);
+  mapped_file_ = ashmem_create_region(
+      options.name == NULL ? "" : options.name.c_str(),
+      options.size);
   if (-1 == mapped_file_) {
     DLOG(ERROR) << "Shared memory creation failed";
     return false;
diff --git a/base/shared_memory_posix.cc b/base/shared_memory_posix.cc
index 3e5699a..030061a0 100644
--- a/base/shared_memory_posix.cc
+++ b/base/shared_memory_posix.cc
@@ -98,10 +98,6 @@ bool SharedMemory::CreateAndMapAnonymous(uint32 size) {
   return CreateAnonymous(size) && Map(size);
 }
 
-bool SharedMemory::CreateAnonymous(uint32 size) {
-  return CreateNamed("", false, size);
-}
-
 #if !defined(OS_ANDROID)
 // Chromium mostly only uses the unique/private shmem as specified by
 // "name == L"". The exception is in the StatsTable.
@@ -109,10 +105,9 @@ bool SharedMemory::CreateAnonymous(uint32 size) {
 // we restart from a crash.  (That isn't a new problem, but it is a problem.)
 // In case we want to delete it later, it may be useful to save the value
 // of mem_filename after FilePathForMemoryName().
-bool SharedMemory::CreateNamed(const std::string& name,
-                               bool open_existing, uint32 size) {
+bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
   DCHECK_EQ(-1, mapped_file_);
-  if (size == 0) return false;
+  if (options.size == 0) return false;
 
   // This function theoretically can block on the disk, but realistically
   // the temporary files we create will just go into the buffer cache
@@ -123,9 +118,9 @@ bool SharedMemory::CreateNamed(const std::string& name,
   bool fix_size = true;
 
   FilePath path;
-  if (name.empty()) {
+  if (options.name == NULL || options.name->empty()) {
     // It doesn't make sense to have a open-existing private piece of shmem
-    DCHECK(!open_existing);
+    DCHECK(!options.open_existing);
     // Q: Why not use the shm_open() etc. APIs?
     // A: Because they're limited to 4mb on OS X.  FFFFFFFUUUUUUUUUUU
     fp = file_util::CreateAndOpenTemporaryShmemFile(&path);
@@ -137,11 +132,11 @@ bool SharedMemory::CreateNamed(const std::string& name,
       file_util::Delete(path, false);
 
   } else {
-    if (!FilePathForMemoryName(name, &path))
+    if (!FilePathForMemoryName(*options.name, &path))
       return false;
 
     fp = file_util::OpenFile(path, "w+x");
-    if (fp == NULL && open_existing) {
+    if (fp == NULL && options.open_existing) {
       // "w+" will truncate if it already exists.
       fp = file_util::OpenFile(path, "a+");
       fix_size = false;
@@ -155,17 +150,17 @@ bool SharedMemory::CreateNamed(const std::string& name,
       return false;
     }
     const uint32 current_size = stat.st_size;
-    if (current_size != size) {
-      if (HANDLE_EINTR(ftruncate(fileno(fp), size)) != 0) {
+    if (current_size != options.size) {
+      if (HANDLE_EINTR(ftruncate(fileno(fp), options.size)) != 0) {
         file_util::CloseFile(fp);
         return false;
       }
-      if (fseeko(fp, size, SEEK_SET) != 0) {
+      if (fseeko(fp, options.size, SEEK_SET) != 0) {
         file_util::CloseFile(fp);
         return false;
       }
     }
-    created_size_ = size;
+    created_size_ = options.size;
   }
   if (fp == NULL) {
 #if !defined(OS_MACOSX)
diff --git a/base/shared_memory_unittest.cc b/base/shared_memory_unittest.cc
index 9f69a5d..41b8ba5 100644
--- a/base/shared_memory_unittest.cc
+++ b/base/shared_memory_unittest.cc
@@ -15,6 +15,10 @@
 #include "base/mac/scoped_nsautorelease_pool.h"
 #endif
 
+#if defined(OS_POSIX)
+#include <sys/mman.h>
+#endif
+
 static const int kNumThreads = 5;
 static const int kNumTasks = 5;
 
@@ -332,6 +336,24 @@ TEST(SharedMemoryTest, AnonymousPrivate) {
   }
 }
 
+#if defined(OS_POSIX)
+// Create a shared memory object, mmap it, and mprotect it to PROT_EXEC.
+TEST(SharedMemoryTest, AnonymousExecutable) {
+  const uint32 kTestSize = 1 << 16;
+
+  SharedMemory shared_memory;
+  SharedMemoryCreateOptions options;
+  options.size = kTestSize;
+  options.executable = true;
+
+  EXPECT_TRUE(shared_memory.Create(options));
+  EXPECT_TRUE(shared_memory.Map(shared_memory.created_size()));
+
+  EXPECT_EQ(0, mprotect(shared_memory.memory(), shared_memory.created_size(),
+                        PROT_READ | PROT_EXEC));
+}
+#endif
+
 // On POSIX it is especially important we test shmem across processes,
 // not just across threads.  But the test is enabled on all platforms.
 class SharedMemoryProcessTest : public MultiProcessTest {
diff --git a/base/shared_memory_win.cc b/base/shared_memory_win.cc
index 3e5ad36..042eb8b 100644
--- a/base/shared_memory_win.cc
+++ b/base/shared_memory_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -74,14 +74,10 @@ bool SharedMemory::CreateAndMapAnonymous(uint32 size) {
   return CreateAnonymous(size) && Map(size);
 }
 
-bool SharedMemory::CreateAnonymous(uint32 size) {
-  return CreateNamed("", false, size);
-}
-
-bool SharedMemory::CreateNamed(const std::string& name,
-                               bool open_existing, uint32 size) {
+bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
+  DCHECK(!options.executable);
   DCHECK(!mapped_file_);
-  if (size == 0)
+  if (options.size == 0)
     return false;
 
   // NaCl's memory allocator requires 0mod64K alignment and size for
@@ -89,22 +85,22 @@ bool SharedMemory::CreateNamed(const std::string& name,
   // therefore we round the size actually created to the nearest 64K unit.
   // To avoid client impact, we continue to retain the size as the
   // actual requested size.
-  uint32 rounded_size = (size + 0xffff) & ~0xffff;
-  name_ = ASCIIToWide(name);
+  uint32 rounded_size = (options.size + 0xffff) & ~0xffff;
+  name_ = ASCIIToWide(options.name == NULL ? "" : *options.name);
   mapped_file_ = CreateFileMapping(INVALID_HANDLE_VALUE, NULL,
       PAGE_READWRITE, 0, static_cast<DWORD>(rounded_size),
       name_.empty() ? NULL : name_.c_str());
   if (!mapped_file_)
     return false;
 
-  created_size_ = size;
+  created_size_ = options.size;
 
   // Check if the shared memory pre-exists.
   if (GetLastError() == ERROR_ALREADY_EXISTS) {
     // If the file already existed, set created_size_ to 0 to show that
     // we don't know the size.
     created_size_ = 0;
-    if (!open_existing) {
+    if (!options.open_existing) {
       Close();
       return false;
     }