Bug 1511560 - Allow dup and ftruncate (when needed) in SandboxPolicyCommon. r=gcp
authorJed Davis <jld@mozilla.com>
Wed, 27 Feb 2019 20:14:52 +0000
changeset 519403 94cb1fe9db5eb0f0aa0634541afb08af17cf5c05
parent 519402 db2dee78ddb0dd23e29948258abd6c7404555b59
child 519404 bf58d8320f5a1de358b930d996615c73ff22cce9
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1511560, 1440203
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1511560 - Allow dup and ftruncate (when needed) in SandboxPolicyCommon. r=gcp File descriptors are sometimes dup()ed in the process of communicating them over IPC; some of this may be unnecessary (due to insufficient use of move-only types), but dup() is relatively harmless. It was previously allowed for both content and GMP, so this doesn't change anything. The handling of ftruncate is a little complicated -- it's used for IPC shared memory, but only when creating segments; so GMP doesn't allow it and should continue not allowing it, but content needs it and RDD will as well. As a result, the subclass indicates if it will be needed. Note that even when we have memfd_create support (bug 1440203), ftruncate is still necessary even though brokering may not. Depends on D14523 Differential Revision: https://phabricator.services.mozilla.com/D14524
security/sandbox/linux/SandboxFilter.cpp
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -91,20 +91,29 @@ using namespace sandbox::bpf_dsl;
 namespace mozilla {
 
 // This class allows everything used by the sandbox itself, by the
 // core IPC code, by the crash reporter, or other core code.  It also
 // contains support for brokering file operations, but file access is
 // denied if no broker client is provided by the concrete class.
 class SandboxPolicyCommon : public SandboxPolicyBase {
  protected:
-  SandboxBrokerClient* mBroker;
+  enum class ShmemUsage {
+    MAY_CREATE,
+    ONLY_USE,
+  };
 
-  explicit SandboxPolicyCommon(SandboxBrokerClient* aBroker = nullptr)
-      : mBroker(aBroker) {}
+  SandboxBrokerClient* mBroker;
+  ShmemUsage mShmemUsage;
+
+  explicit SandboxPolicyCommon(SandboxBrokerClient* aBroker,
+                               ShmemUsage aShmemUsage = ShmemUsage::MAY_CREATE)
+      : mBroker(aBroker), mShmemUsage(aShmemUsage) {}
+
+  SandboxPolicyCommon() : SandboxPolicyCommon(nullptr, ShmemUsage::ONLY_USE) {}
 
   typedef const sandbox::arch_seccomp_data& ArgsRef;
 
   static intptr_t BlockedSyscallTrap(ArgsRef aArgs, void* aux) {
     MOZ_ASSERT(!aux);
     return -ENOSYS;
   }
 
@@ -488,16 +497,30 @@ class SandboxPolicyCommon : public Sandb
         // Simple I/O
       case __NR_write:
       case __NR_read:
       case __NR_readv:
       case __NR_writev:  // see SandboxLogging.cpp
       CASES_FOR_lseek:
         return Allow();
 
+      CASES_FOR_ftruncate:
+        switch (mShmemUsage) {
+          case ShmemUsage::MAY_CREATE:
+            return Allow();
+          case ShmemUsage::ONLY_USE:
+            return InvalidSyscall();
+          default:
+            MOZ_CRASH("unreachable");
+        }
+
+        // Used by our fd/shm classes
+      case __NR_dup:
+        return Allow();
+
         // Memory mapping
       CASES_FOR_mmap:
       case __NR_munmap:
         return Allow();
 
         // ipc::Shmem; also, glibc when creating threads:
       case __NR_mprotect:
         return Allow();
@@ -977,17 +1000,16 @@ class ContentSandboxPolicy : public Sand
         return Error(EPERM);
 #  endif
 
       CASES_FOR_select:
       case __NR_pselect6:
         return Allow();
 
       CASES_FOR_getdents:
-      CASES_FOR_ftruncate:
       case __NR_writev:
       case __NR_pread64:
 #  ifdef DESKTOP
       case __NR_pwrite64:
       case __NR_readahead:
 #  endif
         return Allow();
 
@@ -1083,17 +1105,16 @@ class ContentSandboxPolicy : public Sand
       case __NR_set_thread_area:
         return Allow();
 #  endif
 
       case __NR_getrusage:
       case __NR_times:
         return Allow();
 
-      case __NR_dup:
       case __NR_dup2:  // See ConnectTrapCommon
         return Allow();
 
       CASES_FOR_getuid:
       CASES_FOR_getgid:
       CASES_FOR_geteuid:
       CASES_FOR_getegid:
         return Allow();
@@ -1361,19 +1382,16 @@ class GMPSandboxPolicy : public SandboxP
         return Allow();
 
       // Bug 1372428
       case __NR_uname:
         return Trap(UnameTrap, nullptr);
       CASES_FOR_fcntl:
         return Trap(FcntlTrap, nullptr);
 
-      case __NR_dup:
-        return Allow();
-
       default:
         return SandboxPolicyCommon::EvaluateSyscall(sysno);
     }
   }
 };
 
 UniquePtr<sandbox::bpf_dsl::Policy> GetMediaSandboxPolicy(
     const SandboxOpenedFiles* aFiles) {