Bug 1447867 - Replace base::SharedMemory POSIX backend with shm_open and ashmem. r=froydnj draft
authorJed Davis <jld@mozilla.com>
Thu, 12 Apr 2018 21:37:38 -0600
changeset 785424 ddbcdc7556960a7c031c531d239393706186e14d
parent 783966 789e30ff2e3d6e1fcfce1a373c1e5635488d24da
child 785765 d85d9a8464f597e4e21a785191e1f7c92ad5bc37
push id107227
push userbmo:jld@mozilla.com
push dateFri, 20 Apr 2018 04:23:45 +0000
reviewersfroydnj
bugs1447867
milestone61.0a1
Bug 1447867 - Replace base::SharedMemory POSIX backend with shm_open and ashmem. r=froydnj This replaces using file_util to open and unlink temporary files (/dev/shm on Linux, $TMPDIR or /tmp otherwise) with the POSIX shm_open API, or ashmem on Android (which doesn't implement shm_open). glibc maps shm_open/shm_unlink to open and unlink in /dev/shm (as does musl libc), so the Linux situation is mostly unchanged except we aren't duplicating code from system libraries. Other OSes may (and some do) use more efficient implementations than temporary files. FreeBSD's SHM_ANON extension is used if available. Sadly, it's not standard; it would make this patch much simpler if it were. This patch changes the shm file names; they now start with "org.mozilla" instead of "org.chromium" because the original Chromium code is mostly gone at this point. When running as a Snap package, the required filename prefix is added; other container/sandbox environments using AppArmor to restrict the allowed filenames may need to be adjusted. The shm names now include the creating process's pid, to allow using sandboxing to prevent interfering with shm belonging to other applications or other processes within the same browser instance. MozReview-Commit-ID: 7PirIlcblh4
ipc/chromium/src/base/shared_memory.h
ipc/chromium/src/base/shared_memory_posix.cc
--- a/ipc/chromium/src/base/shared_memory.h
+++ b/ipc/chromium/src/base/shared_memory.h
@@ -108,16 +108,24 @@ class SharedMemory {
   //   return ok;
   // Note that the memory is unmapped by calling this method, regardless of the
   // return value.
   bool GiveToProcess(ProcessId target_pid,
                      SharedMemoryHandle* new_handle) {
     return ShareToProcessCommon(target_pid, new_handle, true);
   }
 
+#ifdef OS_POSIX
+  // If named POSIX shm is being used, append the prefix (including
+  // the leading '/') that would be used by a process with the given
+  // pid to the given string and return true.  If not, return false.
+  // (This is public so that the Linux sandboxing code can use it.)
+  static bool AppendPosixShmPrefix(std::string* str, pid_t pid);
+#endif
+
  private:
   bool ShareToProcessCommon(ProcessId target_pid,
                             SharedMemoryHandle* new_handle,
                             bool close_self);
 
 #if defined(OS_WIN)
   HANDLE             mapped_file_;
 #elif defined(OS_POSIX)
--- a/ipc/chromium/src/base/shared_memory_posix.cc
+++ b/ipc/chromium/src/base/shared_memory_posix.cc
@@ -7,21 +7,25 @@
 #include "base/shared_memory.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
-#include "base/file_util.h"
+#ifdef ANDROID
+#include <linux/ashmem.h>
+#endif
+
+#include "base/eintr_wrapper.h"
 #include "base/logging.h"
-#include "base/platform_thread.h"
 #include "base/string_util.h"
-#include "mozilla/UniquePtr.h"
+#include "mozilla/Atomics.h"
+#include "prenv.h"
 
 namespace base {
 
 SharedMemory::SharedMemory()
     : mapped_file_(-1),
       memory_(NULL),
       read_only_(false),
       max_size_(0) {
@@ -44,69 +48,102 @@ bool SharedMemory::IsHandleValid(const S
   return handle.fd >= 0;
 }
 
 // static
 SharedMemoryHandle SharedMemory::NULLHandle() {
   return SharedMemoryHandle();
 }
 
-namespace {
-
-// A class to handle auto-closing of FILE*'s.
-class ScopedFILEClose {
- public:
-  inline void operator()(FILE* x) const {
-    if (x) {
-      fclose(x);
-    }
+// static
+bool SharedMemory::AppendPosixShmPrefix(std::string* str, pid_t pid)
+{
+#if defined(ANDROID) || defined(SHM_ANON)
+  return false;
+#else
+  *str += '/';
+#ifdef OS_LINUX
+  // The Snap package environment doesn't provide a private /dev/shm
+  // (it's used for communication with services like PulseAudio);
+  // instead AppArmor is used to restrict access to it.  Anything with
+  // this prefix is allowed:
+  static const char* const kSnap = PR_GetEnv("SNAP_NAME");
+  if (kSnap) {
+    StringAppendF(str, "snap.%s.", kSnap);
   }
-};
-
-typedef mozilla::UniquePtr<FILE, ScopedFILEClose> ScopedFILE;
-
+#endif // OS_LINUX
+  // Hopefully the "implementation defined" name length limit is long
+  // enough for this.
+  StringAppendF(str, "org.mozilla.ipc.%d.", static_cast<int>(pid));
+  return true;
+#endif // !ANDROID && !SHM_ANON
 }
 
 bool SharedMemory::Create(size_t size) {
   read_only_ = false;
 
   DCHECK(size > 0);
   DCHECK(mapped_file_ == -1);
 
-  ScopedFILE file_closer;
-  FILE *fp;
-
-  FilePath path;
-  fp = file_util::CreateAndOpenTemporaryShmemFile(&path);
-
-  // Deleting the file prevents anyone else from mapping it in
-  // (making it private), and prevents the need for cleanup (once
-  // the last fd is closed, it is truly freed).
-  file_util::Delete(path);
-
-  if (fp == NULL)
-    return false;
-  file_closer.reset(fp);  // close when we go out of scope
+  int fd;
+  bool needs_truncate = true;
 
-  // Set the file size.
-  //
-  // According to POSIX, (f)truncate can be used to extend files;
-  // previously this required the XSI option but as of the 2008
-  // edition it's required for everything.  (Linux documents that this
-  // may fail on non-"native" filesystems like FAT, but /dev/shm
-  // should always be tmpfs.)
-  if (ftruncate(fileno(fp), size) != 0)
+#ifdef ANDROID
+  // Android has its own shared memory facility:
+  fd = open("/" ASHMEM_NAME_DEF, O_RDWR, 0600);
+  if (fd < 0) {
+    CHROMIUM_LOG(WARNING) << "failed to open shm: " << strerror(errno);
+    return false;
+  }
+  if (ioctl(fd, ASHMEM_SET_SIZE, size) != 0) {
+    CHROMIUM_LOG(WARNING) << "failed to set shm size: " << strerror(errno);
+    close(fd);
     return false;
-  // This probably isn't needed.
-  if (fseeko(fp, size, SEEK_SET) != 0)
-    return false;
+  }
+  needs_truncate = false;
+#elif defined(SHM_ANON)
+  // FreeBSD (or any other Unix that might decide to implement this
+  // nice, simple API):
+  fd = shm_open(SHM_ANON, O_RDWR, 0600);
+#else
+  // Generic Unix: shm_open + shm_unlink
+  do {
+    // The names don't need to be unique, but it saves time if they
+    // usually are.
+    static mozilla::Atomic<size_t> sNameCounter;
+    std::string name;
+    CHECK(AppendPosixShmPrefix(&name, getpid()));
+    StringAppendF(&name, "%zu", sNameCounter++);
+    // O_EXCL means the names being predictable shouldn't be a problem.
+    fd = HANDLE_EINTR(shm_open(name.c_str(), O_RDWR | O_CREAT | O_EXCL, 0600));
+    if (fd >= 0) {
+      if (shm_unlink(name.c_str()) != 0) {
+        // This shouldn't happen, but if it does: assume the file is
+        // in fact leaked, and bail out now while it's still 0-length.
+        DLOG(FATAL) << "failed to unlink shm: " << strerror(errno);
+        return false;
+      }
+    }
+  } while (fd < 0 && errno == EEXIST);
+#endif
 
-  mapped_file_ = dup(fileno(fp));
-  DCHECK(mapped_file_ >= 0);
+  if (fd < 0) {
+    CHROMIUM_LOG(WARNING) << "failed to open shm: " << strerror(errno);
+    return false;
+  }
 
+  if (needs_truncate) {
+    if (HANDLE_EINTR(ftruncate(fd, static_cast<off_t>(size))) != 0) {
+      CHROMIUM_LOG(WARNING) << "failed to set shm size: " << strerror(errno);
+      close(fd);
+      return false;
+    }
+  }
+
+  mapped_file_ = fd;
   max_size_ = size;
   return true;
 }
 
 bool SharedMemory::Map(size_t bytes) {
   if (mapped_file_ == -1)
     return false;