sandbox: Fix RedirectToUserSpacePolicyWrapper

sandbox/linux/seccomp-bpf/sandbox_bpf.cc

Index: sandbox/linux/seccomp-bpf/sandbox_bpf.cc
diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
index 83b98a663f9235fc63c409fe13d1577ba8d938d0..eb760bb3a1fd7ed9ece526ef3f7711eac5f03854 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
@@ -201,6 +201,9 @@ class RedirectToUserSpacePolicyWrapper : public SandboxBPFPolicy {
         wrapped_policy_->EvaluateSyscall(sandbox_compiler, system_call_number);
     if ((err.err() & SECCOMP_RET_ACTION) == SECCOMP_RET_ERRNO) {

[jln (very slow on Chromium), 2014/08/29 20:40:05]

Shouldn't use your new ChangeErrnoToTraps() method here as well,
unconditionally?

This could simplify this codepath.

[leecam, 2014/08/29 22:56:30]

Done.

       return ReturnErrnoViaTrap(sandbox_compiler, err.err() & SECCOMP_RET_DATA);
+    } else if (err.error_type() == ErrorCode::ET_COND) {
+      // Need to change all ERRNO ErrorCode in this Conditional to Traps
+      ChangeErrnoToTraps(&err, sandbox_compiler);
     }
     return err;
   }
@@ -215,6 +218,20 @@ class RedirectToUserSpacePolicyWrapper : public SandboxBPFPolicy {
     return sandbox_compiler->Trap(ReturnErrno, reinterpret_cast<void*>(err));
   }
 
+  void ChangeErrnoToTraps(ErrorCode* err, SandboxBPF* sandbox_compiler) const {

[jln (very slow on Chromium), 2014/08/29 20:40:05]

Because of the recursion you need to handle all cases here (trap, allow,
conditions, errno). Please document on top of this function what it does, and
use it unconditionally from EvaluateSyscall() above.

[leecam, 2014/08/29 22:56:31]

Done.

+    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);
+    } 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);
+    }
+    // Have a Trap or Allow, leave as they are.

[jln (very slow on Chromium), 2014/08/29 20:40:05]

To make this code more robust, check if it is a Trap or Allow (and do nothing)
and if not, CHECK(false) (or NOTREACHED());

[leecam, 2014/08/29 22:56:30]

Done.

+  }
+
   const SandboxBPFPolicy* wrapped_policy_;
   DISALLOW_COPY_AND_ASSIGN(RedirectToUserSpacePolicyWrapper);
 };