Adaptively use temp dir instead of /dev/shm for executable shmem file on Linux

base/file_util_posix.cc

Index: base/file_util_posix.cc
diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc
index 582f70ed7c6450c7e3e3b228368abaebcc2f4cb3..d40b6ab2d93cadd44b795b321719d6569189e721 100644
--- a/base/file_util_posix.cc
+++ b/base/file_util_posix.cc
@@ -481,9 +481,9 @@ bool CreateTemporaryFile(FilePath* path) {
   return true;
 }
 
-FILE* CreateAndOpenTemporaryShmemFile(FilePath* path) {
+FILE* CreateAndOpenTemporaryShmemFile(FilePath* path, bool executable) {
   FilePath directory;
-  if (!GetShmemTempDir(&directory))
+  if (!GetShmemTempDir(&directory, executable))
     return NULL;
 
   return CreateAndOpenTemporaryFileInDir(directory, path);
@@ -910,15 +910,66 @@ bool GetTempDir(FilePath* path) {
 }
 
 #if !defined(OS_ANDROID)
-bool GetShmemTempDir(FilePath* path) {
+
 #if defined(OS_LINUX)
-  *path = FilePath("/dev/shm");
-  return true;
-#else
-  return GetTempDir(path);
-#endif
+// Determine if /dev/shm files can be mapped and then mprotect'd PROT_EXEC.
+// This depends on the mount options used for /dev/shm, which vary among
+// different Linux distributions and possibly local configuration.  It also
+// depends on details of kernel--ChromeOS uses the noexec option for /dev/shm
+// but its kernel allows mprotect with PROT_EXEC anyway.
+//
+// Do this once the first time we need to know.  The global variable
+// storing the state is not locked, so there is the possibility of a race
+// here.  But it should be a harmless one, since the answer should come back
+// the same on multiple attempts.
+
+namespace {
+
+enum DevShmExecutable {
+  DevShmExecutableUnknown,
+  DevShmExecutableYes,
+  DevShmExecutableNo
+} g_dev_shm_executable;
+
+DevShmExecutable DetermineDevShmExecutable() {
+  if (g_dev_shm_executable == DevShmExecutableUnknown) {
+    FilePath path;
+    int fd = CreateAndOpenFdForTemporaryFile(FilePath("/dev/shm"), &path);

[Mark Mentovai, 2011/12/05 23:44:47]

A ScopedFDClose would be a good idea here.

+    if (fd < 0) {
+      g_dev_shm_executable = DevShmExecutableNo;
+    } else {
+      Delete(path, false);
+      size_t pagesize = sysconf(_SC_PAGESIZE);
+      void *mapping = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, fd, 0);
+      if (mapping == MAP_FAILED) {
+        g_dev_shm_executable = DevShmExecutableNo;
+      } else {
+        if (mprotect(mapping, pagesize, PROT_READ | PROT_EXEC) == 0)
+          g_dev_shm_executable = DevShmExecutableYes;
+        else
+          g_dev_shm_executable = DevShmExecutableNo;
+        munmap(mapping, pagesize);
+      }
+      close(fd);
+    }
+  }
+  return g_dev_shm_executable;
 }
+
+};  // namespace
+#endif  // defined(OS_LINUX)
+
+bool GetShmemTempDir(FilePath* path, bool executable) {
+#if defined(OS_LINUX)
+  if (!executable ||

[Mark Mentovai, 2011/12/05 23:44:47]

It might be better to do:

  DevShmExecutable dev_shm_executable;
  if (executable) {
    static DevShmExecutable s_dev_shm_executable = DetermineDevShmExecutable();
    dev_shm_executable = s_dev_shm_executable;
  }
  if (!executable || dev_shm_executable == DevShmExecutableYes) {
    // ...
  }

This gets rid of g_dev_shm_executable (no big deal) but also gets rid of the
DevShmExecutableUnknown state and eliminates your own tracking of that state.
You’re using the static to your advantage.

I’m assuming that you’re not worried about threading concerns here, but the
patch you proposed also seemed not to be worried about threading.

+      DetermineDevShmExecutable() == DevShmExecutableYes) {
+    *path = FilePath("/dev/shm");
+    return true;
+  }
 #endif
+  return GetTempDir(path);
+}
+#endif  // !defined(OS_ANDROID)
 
 FilePath GetHomeDir() {
   const char* home_dir = getenv("HOME");