Bug 1439057 - Use the sandbox broker directly for shared memory file creation on Linux. r?gcp r?froydnj draft
authorJed Davis <jld@mozilla.com>
Fri, 16 Mar 2018 16:29:32 -0600
changeset 772862 94108afcc1f7af06e1c7bf300d2098faa536d7c0
parent 772861 bf91d8bcdbf062c3b7c01015e07fbd618a5cf1af
child 772863 73e89e235d3df83493379c6aa05968373590d892
push id104068
push userbmo:jld@mozilla.com
push dateTue, 27 Mar 2018 01:50:10 +0000
reviewersgcp, froydnj
bugs1439057, 1440203
milestone61.0a1
Bug 1439057 - Use the sandbox broker directly for shared memory file creation on Linux. r?gcp r?froydnj Previously the open() and unlink() calls would raise SIGSYS and each do a broker RPC, and we had to allow access to all of /dev/shm (allowing the possibility of interfering with other processes) in the broker policy. This patch allowing creating an anonymous shared memory file with a single broker RPC, bypassing the signal handling overhead and avoiding the need to give access to any other files -- the broker returns the file already unlinked and the client doesn't specify a filename or path. The big picture for how this works: base::SharedMemory::Create in a content process calls into libmozsandbox, which calls a method on its broker client, which messages the parent process, which calls its own instance of base::SharedMemory::Create, which doesn't have a broker client and so does the open/unlink itself. The broker uses a dedicated thread per child process, so the synchronous IPC here will interfere only with other brokered file I/O for the same process, and that was already the case before this patch. (See also bug 1440203, which will avoid the IPC entirely on newer kernels.) Bonus fix: the global SandboxReporterClient pointer is made atomic, to match the new global SandboxBrokerClient pointer. MozReview-Commit-ID: FuupQjGqy8j
ipc/chromium/src/base/shared_memory_posix.cc
security/sandbox/linux/Sandbox.cpp
security/sandbox/linux/Sandbox.h
security/sandbox/linux/SandboxBrokerClient.cpp
security/sandbox/linux/SandboxBrokerClient.h
security/sandbox/linux/broker/SandboxBroker.cpp
security/sandbox/linux/broker/SandboxBrokerCommon.cpp
security/sandbox/linux/broker/SandboxBrokerCommon.h
--- a/ipc/chromium/src/base/shared_memory_posix.cc
+++ b/ipc/chromium/src/base/shared_memory_posix.cc
@@ -11,16 +11,19 @@
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/platform_thread.h"
 #include "base/string_util.h"
+#if defined(XP_LINUX) && defined(MOZ_SANDBOX)
+#include "mozilla/Sandbox.h"
+#endif
 #include "mozilla/UniquePtr.h"
 
 namespace base {
 
 SharedMemory::SharedMemory()
     : mapped_file_(-1),
       memory_(NULL),
       read_only_(false),
@@ -58,37 +61,67 @@ class ScopedFILEClose {
     if (x) {
       fclose(x);
     }
   }
 };
 
 typedef mozilla::UniquePtr<FILE, ScopedFILEClose> ScopedFILE;
 
+static int CreateViaSandbox(size_t size) {
+#if defined(XP_LINUX) && defined(MOZ_SANDBOX)
+  int rv = mozilla::SandboxSharedMemoryCreate(size);
+  if (rv < 0) {
+    DCHECK_EQ(-rv, ENOTCONN);
+    return -1;
+  }
+  return rv;
+#else
+  return -1;
+#endif
 }
 
+} // anonymous namespace
+
 bool SharedMemory::Create(size_t size) {
   read_only_ = false;
 
   DCHECK(size > 0);
   DCHECK(mapped_file_ == -1);
 
+  // Check for sandboxing and attempt brokered file creation.
+  mapped_file_ = CreateViaSandbox(size);
+  if (mapped_file_ >= 0) {
+    max_size_ = size;
+    return true;
+  }
+
   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)
+  if (fp == NULL) {
+    // Handle a possible race condition: CreateViaSandbox fails
+    // because the sandbox isn't started, then the sandbox is started,
+    // then filesystem access is attemped and fails.  In that case a
+    // second CreateViaSandbox should succeed.
+    mapped_file_ = CreateViaSandbox(size);
+    if (mapped_file_ >= 0) {
+      max_size_ = size;
+      return true;
+    }
     return false;
+  }
   file_closer.reset(fp);  // close when we go out of scope
 
   // 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
--- a/security/sandbox/linux/Sandbox.cpp
+++ b/security/sandbox/linux/Sandbox.cpp
@@ -82,17 +82,18 @@ mozilla::Atomic<int> gSeccompTsyncBroadc
 
 namespace mozilla {
 
 static bool gSandboxCrashOnError = false;
 
 // This is initialized by SandboxSetCrashFunc().
 SandboxCrashFunc gSandboxCrashFunc;
 
-static SandboxReporterClient* gSandboxReporterClient;
+static Atomic<SandboxReporterClient*> gSandboxReporterClient;
+static Atomic<SandboxBrokerClient*> gSandboxBrokerClient;
 static void (*gChromiumSigSysHandler)(int, siginfo_t*, void*);
 
 // Test whether a ucontext, interpreted as the state after a syscall,
 // indicates the given error.  See also sandbox::Syscall::PutValueInUcontext.
 static bool
 ContextIsError(const ucontext_t *aContext, int aError)
 {
   // Avoid integer promotion warnings.  (The unary addition makes
@@ -134,17 +135,18 @@ SigSysHandler(int nr, siginfo_t *info, v
   // which will overwrite one or more registers with the return value.
   ucontext_t savedCtx = *ctx;
 
   gChromiumSigSysHandler(nr, info, ctx);
   if (!ContextIsError(ctx, ENOSYS)) {
     return;
   }
 
-  SandboxReport report = gSandboxReporterClient->MakeReportAndSend(&savedCtx);
+  SandboxReporterClient* client = gSandboxReporterClient;
+  SandboxReport report = client->MakeReportAndSend(&savedCtx);
 
   // TODO, someday when this is enabled on MIPS: include the two extra
   // args in the error message.
   SANDBOX_LOG_ERROR("seccomp sandbox violation: pid %d, tid %d, syscall %d,"
                     " args %d %d %d %d %d %d.%s",
                     report.mPid, report.mTid, report.mSyscall,
                     report.mArgs[0], report.mArgs[1], report.mArgs[2],
                     report.mArgs[3], report.mArgs[4], report.mArgs[5],
@@ -619,23 +621,24 @@ SetContentProcessSandbox(ContentProcessS
     return false;
   }
 
   auto procType = aParams.mFileProcess
     ? SandboxReport::ProcType::FILE
     : SandboxReport::ProcType::CONTENT;
   gSandboxReporterClient = new SandboxReporterClient(procType);
 
-  // This needs to live until the process exits.
-  static SandboxBrokerClient* sBroker;
+  SandboxBrokerClient* broker = nullptr;
   if (brokerFd >= 0) {
-    sBroker = new SandboxBrokerClient(brokerFd);
+    // This needs to live until the process exits.
+    broker = new SandboxBrokerClient(brokerFd);
   }
+  gSandboxBrokerClient = broker;
 
-  SetCurrentProcessSandbox(GetContentSandboxPolicy(sBroker, Move(aParams)));
+  SetCurrentProcessSandbox(GetContentSandboxPolicy(broker, Move(aParams)));
   return true;
 }
 #endif // MOZ_CONTENT_SANDBOX
 
 #ifdef MOZ_GMP_SANDBOX
 /**
  * Starts the seccomp sandbox for a media plugin process.  Should be
  * called only once, and before any potentially harmful content is
@@ -675,9 +678,19 @@ SetMediaPluginSandbox(const char *aFileP
   files->Add("/proc/self/auxv"); // Info also in process's address space.
 #endif
 
   // Finally, start the sandbox.
   SetCurrentProcessSandbox(GetMediaSandboxPolicy(files));
 }
 #endif // MOZ_GMP_SANDBOX
 
+int
+SandboxSharedMemoryCreate(size_t aSize)
+{
+  SandboxBrokerClient* client = gSandboxBrokerClient;
+  if (client == nullptr) {
+    return -ENOTCONN;
+  }
+  return client->ShmCreate(aSize);
+}
+
 } // namespace mozilla
--- a/security/sandbox/linux/Sandbox.h
+++ b/security/sandbox/linux/Sandbox.h
@@ -56,11 +56,17 @@ MOZ_EXPORT bool SetContentProcessSandbox
 
 #ifdef MOZ_GMP_SANDBOX
 // Call only if SandboxInfo::CanSandboxMedia() returns true.
 // (No-op if MOZ_DISABLE_GMP_SANDBOX is set.)
 // aFilePath is the path to the plugin file.
 MOZ_EXPORT void SetMediaPluginSandbox(const char *aFilePath);
 #endif
 
+// Create a new anonymous shared memory file of the given size if this
+// process has a SandboxBrokerClient.  Returns the fd if successful,
+// or -1 and sets errno on error; errno may be ENOTCONN if there is no
+// broker client, or an error returned by the broker client.
+MOZ_EXPORT int SandboxSharedMemoryCreate(size_t aSize);
+
 } // namespace mozilla
 
 #endif // mozilla_Sandbox_h
--- a/security/sandbox/linux/SandboxBrokerClient.cpp
+++ b/security/sandbox/linux/SandboxBrokerClient.cpp
@@ -240,16 +240,23 @@ SandboxBrokerClient::Rmdir(const char* a
 int
 SandboxBrokerClient::Readlink(const char* aPath, void* aBuff, size_t aSize)
 {
   Request req = {SANDBOX_FILE_READLINK, 0, aSize};
   return DoCall(&req, aPath, nullptr, aBuff, false);
 }
 
 int
+SandboxBrokerClient::ShmCreate(size_t aSize)
+{
+  const Request req = { SANDBOX_FILE_SHM_CREATE, 0, aSize };
+  return DoCall(&req, "", nullptr, nullptr, true);
+}
+
+int
 SandboxBrokerClient::Connect(const sockaddr_un* aAddr, size_t aLen, int aType)
 {
   static const size_t maxLen = sizeof(aAddr->sun_path);
   const char* path = aAddr->sun_path;
   const auto addrEnd = reinterpret_cast<const char*>(aAddr) + aLen;
   // Ensure that the length isn't impossibly small.
   if (addrEnd <= path) {
     return -EINVAL;
--- a/security/sandbox/linux/SandboxBrokerClient.h
+++ b/security/sandbox/linux/SandboxBrokerClient.h
@@ -38,16 +38,17 @@ class SandboxBrokerClient final : privat
   int Chmod(const char* aPath, int aMode);
   int Link(const char* aPath, const char* aPath2);
   int Mkdir(const char* aPath, int aMode);
   int Symlink(const char* aOldPath, const char* aNewPath);
   int Rename(const char* aOldPath, const char* aNewPath);
   int Unlink(const char* aPath);
   int Rmdir(const char* aPath);
   int Readlink(const char* aPath, void* aBuf, size_t aBufSize);
+  int ShmCreate(size_t aSize);
   int Connect(const struct sockaddr_un* aAddr, size_t aLen, int aType);
 
  private:
   int mFileDesc;
 
   int DoCall(const Request* aReq,
              const char* aPath,
              const char* aPath2,
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -18,16 +18,17 @@
 #include <sys/un.h>
 #include <unistd.h>
 
 #ifdef XP_LINUX
 #include <sys/prctl.h>
 #endif
 
 #include "base/string_util.h"
+#include "base/shared_memory.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Move.h"
 #include "mozilla/NullPtr.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/ipc/FileDescriptor.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsAppDirectoryServiceDefs.h"
@@ -502,16 +503,29 @@ DoLink(const char* aPath, const char* aP
   }
   if (aOper == SandboxBrokerCommon::Operation::SANDBOX_FILE_SYMLINK) {
     return symlink(aPath, aPath2);
   }
   MOZ_CRASH("SandboxBroker: Unknown link operation");
 }
 
 static int
+GetSharedMemory(size_t aSize)
+{
+  base::SharedMemory shm;
+  if (shm.Create(aSize)) {
+    base::FileDescriptor fd;
+    if (shm.GiveToProcess(/* pid; not used */ 0, &fd)) {
+      return fd.fd;
+    }
+  }
+  return -1;
+}
+
+static int
 DoConnect(const char* aPath, size_t aLen, int aType)
 {
   // Deny SOCK_DGRAM for the same reason it's denied for socketpair.
   if (aType != SOCK_STREAM && aType != SOCK_SEQPACKET) {
     errno = EACCES;
     return -1;
   }
   // Ensure that the address is a pathname.  (An empty string
@@ -831,16 +845,21 @@ SandboxBroker::ThreadMain(void)
         // Take the intersection of the permissions for both paths.
         perms &= perms2;
       }
     } else {
       // Failed to receive intelligible paths.
       perms = 0;
     }
 
+    // SHM_CREATE doesn't take a path, so bypass the policy check for it.
+    if (req.mOp == SANDBOX_FILE_SHM_CREATE) {
+      perms |= MAY_ACCESS;
+    }
+
     // And now perform the operation if allowed.
     if (perms & CRASH_INSTEAD) {
       // This is somewhat nonmodular, but it works.
       resp.mError = -ENOSYS;
     } else if (permissive || perms & MAY_ACCESS) {
       // If the operation was only allowed because of permissive mode, log it.
       if (permissive && !(perms & MAY_ACCESS)) {
         AuditPermissive(req.mOp, req.mFlags, perms, pathBuf);
@@ -1004,16 +1023,25 @@ SandboxBroker::ThreadMain(void)
           } else {
             resp.mError = -errno;
           }
         } else {
           AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
         }
         break;
 
+      case SANDBOX_FILE_SHM_CREATE:
+        openedFd = GetSharedMemory(req.mBufSize);
+        if (openedFd >= 0) {
+          resp.mError = 0;
+        } else {
+          resp.mError = -errno;
+        }
+        break;
+
       case SANDBOX_SOCKET_CONNECT:
         if (permissive || (perms & MAY_CONNECT) != 0) {
           openedFd = DoConnect(pathBuf, pathLen, req.mFlags);
           if (openedFd >= 0) {
             resp.mError = 0;
           } else {
             resp.mError = -errno;
           }
--- a/security/sandbox/linux/broker/SandboxBrokerCommon.cpp
+++ b/security/sandbox/linux/broker/SandboxBrokerCommon.cpp
@@ -37,16 +37,17 @@ const char* SandboxBrokerCommon::Operati
   "chmod",
   "link",
   "symlink",
   "mkdir",
   "rename",
   "rmdir",
   "unlink",
   "readlink",
+  "shm_create",
   "connect"
 };
 
 /* static */ ssize_t
 SandboxBrokerCommon::RecvWithFd(int aFd, const iovec* aIO, size_t aNumIO,
                                     int* aPassedFdPtr)
 {
   struct msghdr msg = {};
--- a/security/sandbox/linux/broker/SandboxBrokerCommon.h
+++ b/security/sandbox/linux/broker/SandboxBrokerCommon.h
@@ -32,37 +32,43 @@ public:
     SANDBOX_FILE_CHMOD,
     SANDBOX_FILE_LINK,
     SANDBOX_FILE_SYMLINK,
     SANDBOX_FILE_MKDIR,
     SANDBOX_FILE_RENAME,
     SANDBOX_FILE_RMDIR,
     SANDBOX_FILE_UNLINK,
     SANDBOX_FILE_READLINK,
+    SANDBOX_FILE_SHM_CREATE,
     SANDBOX_SOCKET_CONNECT,
   };
   // String versions of the above
   static const char* OperationDescription[];
 
   struct Request {
     Operation mOp;
     // For open, flags; for access, "mode"; for stat, O_NOFOLLOW for lstat.
     // For connect, the socket type.
     int mFlags;
-    // Size of return value buffer, if any
+    // For SANDBOX_FILE_READLINK, the size of the return buffer.
+    // For SANDBOX_FILE_SHM_CREATE, the size of the file to create.
     size_t mBufSize;
-    // The rest of the packet is the pathname.
-    // SCM_RIGHTS for response socket attached.
+    // The rest of the packet is the pathname(s), null-terminated; see
+    // the comments in SandboxBroker::ThreadMain.
+    //
+    // For SANDBOX_FILE_SHM_CREATE the pathname is ignored and should be empty.
+    //
+    // A SCM_RIGHTS message containing the reponse socket is attached.
   };
 
   struct Response {
     // Syscall result, -errno if failure, or 0 for no error
     int mError;
     // Followed by struct stat for stat/lstat.
-    // SCM_RIGHTS attached for successful open.
+    // SHM_RIGHTS attached for successful open / shm_create.
   };
 
   // This doesn't need to be the system's maximum path length, just
   // the largest path that would be allowed by any policy.  (It's used
   // to size a stack-allocated buffer.)
   static const size_t kMaxPathLen = 4096;
 
   static ssize_t RecvWithFd(int aFd, const iovec* aIO, size_t aNumIO,