sandbox: Fix RedirectToUserSpacePolicyWrapper

This fixes RedirectToUserSpacePolicyWrapper to
handle ERRNO ErrorCodes that are behind Conditional
ErrorCodes.

BUG=408953
TEST=sandbox_linux_unittests: Added new test

Review URL: https://codereview.chromium.org/524603002

Cr-Commit-Position: refs/heads/master@{#293116}

sandbox/linux/seccomp-bpf/sandbox_bpf.cc

@@ -199,9 +199,7 @@ class RedirectToUserSpacePolicyWrapper : public SandboxBPFPolicy {
                                     int system_call_number) const OVERRIDE {
     ErrorCode err =
         wrapped_policy_->EvaluateSyscall(sandbox_compiler, system_call_number);
-    if ((err.err() & SECCOMP_RET_ACTION) == SECCOMP_RET_ERRNO) {
-      return ReturnErrnoViaTrap(sandbox_compiler, err.err() & SECCOMP_RET_DATA);
-    }
+    ChangeErrnoToTraps(&err, sandbox_compiler);
     return err;
   }
 
@@ -215,6 +213,29 @@ class RedirectToUserSpacePolicyWrapper : public SandboxBPFPolicy {
     return sandbox_compiler->Trap(ReturnErrno, reinterpret_cast<void*>(err));
   }
 
+  // ChangeErrnoToTraps recursivly iterates through the ErrorCode
+  // converting any ERRNO to a userspace trap
+  void ChangeErrnoToTraps(ErrorCode* err, SandboxBPF* sandbox_compiler) const {
+    if (err->error_type() == ErrorCode::ET_SIMPLE &&
+        (err->err() & SECCOMP_RET_ACTION) == SECCOMP_RET_ERRNO) {
+      // Have an errno, need to change this to a trap
+      *err =
+          ReturnErrnoViaTrap(sandbox_compiler, err->err() & SECCOMP_RET_DATA);
+      return;
+    } else if (err->error_type() == ErrorCode::ET_COND) {
+      // Need to explore both paths
+      ChangeErrnoToTraps((ErrorCode*)err->passed(), sandbox_compiler);
+      ChangeErrnoToTraps((ErrorCode*)err->failed(), sandbox_compiler);
+      return;
+    } else if (err->error_type() == ErrorCode::ET_TRAP) {
+      return;
+    } else if (err->error_type() == ErrorCode::ET_SIMPLE &&
+               (err->err() & SECCOMP_RET_ACTION) == SECCOMP_RET_ALLOW) {
+      return;
+    }
+    NOTREACHED();
+  }
+
   const SandboxBPFPolicy* wrapped_policy_;
   DISALLOW_COPY_AND_ASSIGN(RedirectToUserSpacePolicyWrapper);
 };
@@ -1035,6 +1056,19 @@ ErrorCode SandboxBPF::UnsafeTrap(Trap::TrapFnc fnc, const void* aux) {
   return Trap::MakeTrap(fnc, aux, false /* Unsafe Trap */);
 }
 
+bool SandboxBPF::IsRequiredForUnsafeTrap(int sysno) {
+  return (sysno == __NR_rt_sigprocmask || sysno == __NR_rt_sigreturn
+#if defined(__NR_sigprocmask)
+          ||
+          sysno == __NR_sigprocmask
+#endif
+#if defined(__NR_sigreturn)
+          ||
+          sysno == __NR_sigreturn
+#endif
+          );
+}
+
 intptr_t SandboxBPF::ForwardSyscall(const struct arch_seccomp_data& args) {
   return Syscall::Call(args.nr,
                        static_cast<intptr_t>(args.args[0]),

sandbox/linux/seccomp-bpf/sandbox_bpf.h

@@ -130,6 +130,10 @@ class SANDBOX_EXPORT SandboxBPF {
   //   entire sandbox should be considered compromised.
   ErrorCode UnsafeTrap(Trap::TrapFnc fnc, const void* aux);
 
+  // UnsafeTraps require some syscalls to always be allowed.
+  // This helper function returns true for these calls.
+  static bool IsRequiredForUnsafeTrap(int sysno);
+
   // From within an UnsafeTrap() it is often useful to be able to execute
   // the system call that triggered the trap. The ForwardSyscall() method
   // makes this easy. It is more efficient than calling glibc's syscall()

sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc

@@ -504,16 +504,7 @@ ErrorCode GreyListedPolicy(SandboxBPF* sandbox, int sysno, int* aux) {
 
   // Some system calls must always be allowed, if our policy wants to make
   // use of UnsafeTrap()
-  if (sysno == __NR_rt_sigprocmask || sysno == __NR_rt_sigreturn
-#if defined(__NR_sigprocmask)
-      ||
-      sysno == __NR_sigprocmask
-#endif
-#if defined(__NR_sigreturn)
-      ||
-      sysno == __NR_sigreturn
-#endif
-      ) {
+  if (SandboxBPF::IsRequiredForUnsafeTrap(sysno)) {
     return ErrorCode(ErrorCode::ERR_ALLOWED);
   } else if (sysno == __NR_getpid) {
     // Disallow getpid()
@@ -637,18 +628,8 @@ ErrorCode RedirectAllSyscallsPolicy::EvaluateSyscall(SandboxBPF* sandbox,
 
   // Some system calls must always be allowed, if our policy wants to make
   // use of UnsafeTrap()
-  if (sysno == __NR_rt_sigprocmask || sysno == __NR_rt_sigreturn
-#if defined(__NR_sigprocmask)
-      ||
-      sysno == __NR_sigprocmask
-#endif
-#if defined(__NR_sigreturn)
-      ||
-      sysno == __NR_sigreturn
-#endif
-      ) {
+  if (SandboxBPF::IsRequiredForUnsafeTrap(sysno))
     return ErrorCode(ErrorCode::ERR_ALLOWED);
-  }
   return sandbox->UnsafeTrap(AllowRedirectedSyscall, NULL);
 }
 
@@ -2261,6 +2242,75 @@ SANDBOX_DEATH_TEST(SandboxBPF, StartSingleThreadedAsMultiThreaded,
 }
 #endif  // !defined(THREAD_SANITIZER)
 
+// A stub handler for the UnsafeTrap. Never called.
+intptr_t NoOpHandler(const struct arch_seccomp_data& args, void*) {
+  return -1;
+}
+
+class UnsafeTrapWithCondPolicy : public SandboxBPFPolicy {
+ public:
+  UnsafeTrapWithCondPolicy() {}
+  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
+                                    int sysno) const OVERRIDE {
+    DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
+    setenv(kSandboxDebuggingEnv, "t", 0);
+    Die::SuppressInfoMessages(true);
+
+    if (SandboxBPF::IsRequiredForUnsafeTrap(sysno))
+      return ErrorCode(ErrorCode::ERR_ALLOWED);
+
+    switch (sysno) {
+      case __NR_uname:
+        return sandbox->Cond(0,
+                             ErrorCode::TP_32BIT,
+                             ErrorCode::OP_EQUAL,
+                             0,
+                             ErrorCode(ErrorCode::ERR_ALLOWED),
+                             ErrorCode(EPERM));
+      case __NR_setgid:
+        return sandbox->Cond(0,
+                             ErrorCode::TP_32BIT,
+                             ErrorCode::OP_EQUAL,
+                             100,
+                             ErrorCode(ErrorCode(ENOMEM)),
+                             sandbox->Cond(0,
+                                           ErrorCode::TP_32BIT,
+                                           ErrorCode::OP_EQUAL,
+                                           200,
+                                           ErrorCode(ENOSYS),
+                                           ErrorCode(EPERM)));
+      case __NR_close:
+      case __NR_exit_group:
+      case __NR_write:
+        return ErrorCode(ErrorCode::ERR_ALLOWED);
+      case __NR_getppid:
+        return sandbox->UnsafeTrap(NoOpHandler, NULL);
+      default:
+        return ErrorCode(EPERM);
+    }
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(UnsafeTrapWithCondPolicy);
+};
+
+BPF_TEST_C(SandboxBPF, UnsafeTrapWithCond, UnsafeTrapWithCondPolicy) {
+  BPF_ASSERT_EQ(-1, syscall(__NR_uname, 0));
+  BPF_ASSERT_EQ(EFAULT, errno);
+
+  BPF_ASSERT_EQ(-1, syscall(__NR_uname, 1));
+  BPF_ASSERT_EQ(EPERM, errno);
+
+  BPF_ASSERT_EQ(-1, syscall(__NR_setgid, 100));
+  BPF_ASSERT_EQ(ENOMEM, errno);
+
+  BPF_ASSERT_EQ(-1, syscall(__NR_setgid, 200));
+  BPF_ASSERT_EQ(ENOSYS, errno);
+
+  BPF_ASSERT_EQ(-1, syscall(__NR_setgid, 300));
+  BPF_ASSERT_EQ(EPERM, errno);
+}
+
 }  // namespace
 
 }  // namespace sandbox