sandbox: Fix RedirectToUserSpacePolicyWrapper

sandbox/linux/seccomp-bpf/sandbox_bpf.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
 
 // Some headers on Android are missing cdefs: crbug.com/172337.
 // (We can't use OS_ANDROID here since build_config.h is not included).
 #if defined(ANDROID)
 #include <sys/cdefs.h>
@@ skipping 181 matching lines @@
   explicit RedirectToUserSpacePolicyWrapper(
       const SandboxBPFPolicy* wrapped_policy)
       : wrapped_policy_(wrapped_policy) {
     DCHECK(wrapped_policy_);
   }
 
   virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox_compiler,
                                     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) {

[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;
   }
 
   virtual ErrorCode InvalidSyscall(
       SandboxBPF* sandbox_compiler) const OVERRIDE {
     return ReturnErrnoViaTrap(sandbox_compiler, ENOSYS);
   }
 
  private:
   ErrorCode ReturnErrnoViaTrap(SandboxBPF* sandbox_compiler, int err) const {
     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);
 };
 
 intptr_t BPFFailure(const struct arch_seccomp_data&, void* aux) {
   SANDBOX_DIE(static_cast<char*>(aux));
 }
 
 }  // namespace
 
@@ skipping 831 matching lines @@
                    &*conds_->insert(failed).first);
 }
 
 ErrorCode SandboxBPF::Kill(const char* msg) {
   return Trap(BPFFailure, const_cast<char*>(msg));
 }
 
 SandboxBPF::SandboxStatus SandboxBPF::status_ = STATUS_UNKNOWN;
 
 }  // namespace sandbox