Bug 1410191 - Correctly handle errors when using syscalls in sandbox trap handlers. r=gcp
authorJed Davis <jld@mozilla.com>
Wed, 25 Oct 2017 16:38:20 -0600
changeset 439288 671e6d994ecb598ad15bb3329e78d1d0135345cc
parent 439287 7e53467676cac3f79f79c3f183d6b5cc58835aa2
child 439289 072007f834314978acf0dc15efd3c3b935b2957e
push id8114
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 16:33:21 +0000
treeherdermozilla-beta@73e0d89a540f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1410191
milestone58.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 1410191 - Correctly handle errors when using syscalls in sandbox trap handlers. r=gcp MozReview-Commit-ID: JX81xpNBMIm
security/sandbox/linux/SandboxFilter.cpp
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -92,24 +92,35 @@ class SandboxPolicyCommon : public Sandb
 protected:
   typedef const sandbox::arch_seccomp_data& ArgsRef;
 
   static intptr_t BlockedSyscallTrap(ArgsRef aArgs, void *aux) {
     MOZ_ASSERT(!aux);
     return -ENOSYS;
   }
 
+  // Convert Unix-style "return -1 and set errno" APIs back into the
+  // Linux ABI "return -err" style.
+  static intptr_t ConvertError(long rv) {
+    return rv < 0 ? -errno : rv;
+  }
+
+  template<typename... Args>
+  static intptr_t DoSyscall(long nr, Args... args) {
+    return ConvertError(syscall(nr, args...));
+  }
+
 private:
   // Bug 1093893: Translate tkill to tgkill for pthread_kill; fixed in
   // bionic commit 10c8ce59a (in JB and up; API level 16 = Android 4.1).
   // Bug 1376653: musl also needs this, and security-wise it's harmless.
   static intptr_t TKillCompatTrap(const sandbox::arch_seccomp_data& aArgs,
                                   void *aux)
   {
-    return syscall(__NR_tgkill, getpid(), aArgs.args[0], aArgs.args[1]);
+    return DoSyscall(__NR_tgkill, getpid(), aArgs.args[0], aArgs.args[1]);
   }
 
   static intptr_t SetNoNewPrivsTrap(ArgsRef& aArgs, void* aux) {
     if (gSetSandboxFilter == nullptr) {
       // Called after BroadcastSetThreadSandbox finished, therefore
       // not our doing and not expected.
       return BlockedSyscallTrap(aArgs, nullptr);
     }
@@ -520,20 +531,17 @@ private:
     // pid namespace (Bug 1151624).
     return 0;
   }
 
   static intptr_t SocketpairDatagramTrap(ArgsRef aArgs, void* aux) {
     auto fds = reinterpret_cast<int*>(aArgs.args[3]);
     // Return sequential packet sockets instead of the expected
     // datagram sockets; see bug 1355274 for details.
-    if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) != 0) {
-      return -errno;
-    }
-    return 0;
+    return ConvertError(socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds));
   }
 
 public:
   explicit ContentSandboxPolicy(SandboxBrokerClient* aBroker,
                                 const std::vector<int>& aSyscallWhitelist)
     : mBroker(aBroker),
       mSyscallWhitelist(aSyscallWhitelist) {}
   ~ContentSandboxPolicy() override = default;
@@ -1007,23 +1015,23 @@ class GMPSandboxPolicy : public SandboxP
     return fd;
   }
 
   static intptr_t SchedTrap(const sandbox::arch_seccomp_data& aArgs,
                             void* aux)
   {
     const pid_t tid = syscall(__NR_gettid);
     if (aArgs.args[0] == static_cast<uint64_t>(tid)) {
-      return syscall(aArgs.nr,
-                     0,
-                     aArgs.args[1],
-                     aArgs.args[2],
-                     aArgs.args[3],
-                     aArgs.args[4],
-                     aArgs.args[5]);
+      return DoSyscall(aArgs.nr,
+                       0,
+                       aArgs.args[1],
+                       aArgs.args[2],
+                       aArgs.args[3],
+                       aArgs.args[4],
+                       aArgs.args[5]);
     }
     SANDBOX_LOG_ERROR("unsupported tid in SchedTrap");
     return BlockedSyscallTrap(aArgs, nullptr);
   }
 
   static intptr_t UnameTrap(const sandbox::arch_seccomp_data& aArgs,
                             void* aux)
   {