Revert of Remove SandboxBPF's dependency on CompatibilityPolicy (https://codereview.chromium.org/290223002/)

Reason for revert:
Broke VerboseAPITesting in sandbox_linux_unittests on Linux: http://build.chromium.org/p/chromium.linux/buildstatus?builder=Linux%20Tests%20%28dbg%29%282%29%2832%29&number=13046

Original issue's description:
> Remove SandboxBPF's dependency on CompatibilityPolicy
> 
> SandboxBPF users are now required to always provide a SandboxBPFPolicy
> instead of a SyscallEvaluator.  CompatibilityPolicy can't be removed
> just yet though because it's still used by the deprecated BPF_TEST
> macros.
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271607

TBR=jln@chromium.org,mdempsky@chromium.org
NOTREECHECKS=true
NOTRY=true

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271621 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 104 insertions, 120 deletions

diff --git a/sandbox/linux/seccomp-bpf/demo.cc b/sandbox/linux/seccomp-bpf/demo.cc
index d9fd342..1cf45114 100644
--- a/sandbox/linux/seccomp-bpf/demo.cc
+++ b/sandbox/linux/seccomp-bpf/demo.cc
@@ -26,15 +26,12 @@
 #include <time.h>
 #include <unistd.h>
 
-#include "base/macros.h"
 #include "base/posix/eintr_wrapper.h"
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
-#include "sandbox/linux/seccomp-bpf/sandbox_bpf_policy.h"
 #include "sandbox/linux/services/linux_syscalls.h"
 
 using sandbox::ErrorCode;
 using sandbox::SandboxBPF;
-using sandbox::SandboxBPFPolicy;
 using sandbox::arch_seccomp_data;
 
 #define ERR EPERM
@@ -240,17 +237,7 @@ intptr_t DefaultHandler(const struct arch_seccomp_data& data, void *) {
   return -ERR;
 }
 
-class DemoPolicy : public SandboxBPFPolicy {
- public:
-  DemoPolicy() {}
-  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
-                                    int sysno) const OVERRIDE;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(DemoPolicy);
-};
-
-ErrorCode DemoPolicy::EvaluateSyscall(SandboxBPF* sandbox, int sysno) const {
+ErrorCode Evaluator(SandboxBPF* sandbox, int sysno, void *) {
   switch (sysno) {
 #if defined(__NR_accept)
   case __NR_accept: case __NR_accept4:
@@ -433,7 +420,7 @@ int main(int argc, char *argv[]) {
   }
   SandboxBPF sandbox;
   sandbox.set_proc_fd(proc_fd);
-  sandbox.SetSandboxPolicy(new DemoPolicy());
+  sandbox.SetSandboxPolicyDeprecated(Evaluator, NULL);
   if (!sandbox.StartSandbox(SandboxBPF::PROCESS_SINGLE_THREADED)) {
     fprintf(stderr, "StartSandbox() failed");
     _exit(1);
diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
index c5c6f61..18bd30f 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
@@ -22,10 +22,10 @@
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
-#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/posix/eintr_wrapper.h"
 #include "sandbox/linux/seccomp-bpf/codegen.h"
+#include "sandbox/linux/seccomp-bpf/sandbox_bpf_compatibility_policy.h"
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf_policy.h"
 #include "sandbox/linux/seccomp-bpf/syscall.h"
 #include "sandbox/linux/seccomp-bpf/syscall_iterator.h"
@@ -57,26 +57,20 @@ void WriteFailedStderrSetupMessage(int out_fd) {
 
 // We define a really simple sandbox policy. It is just good enough for us
 // to tell that the sandbox has actually been activated.
-class ProbePolicy : public SandboxBPFPolicy {
- public:
-  ProbePolicy() {}
-  virtual ErrorCode EvaluateSyscall(SandboxBPF*, int sysnum) const OVERRIDE {
-    switch (sysnum) {
-      case __NR_getpid:
-        // Return EPERM so that we can check that the filter actually ran.
-        return ErrorCode(EPERM);
-      case __NR_exit_group:
-        // Allow exit() with a non-default return code.
-        return ErrorCode(ErrorCode::ERR_ALLOWED);
-      default:
-        // Make everything else fail in an easily recognizable way.
-        return ErrorCode(EINVAL);
-    }
+ErrorCode ProbeEvaluator(SandboxBPF*, int sysnum, void*) __attribute__((const));
+ErrorCode ProbeEvaluator(SandboxBPF*, int sysnum, void*) {
+  switch (sysnum) {
+    case __NR_getpid:
+      // Return EPERM so that we can check that the filter actually ran.
+      return ErrorCode(EPERM);
+    case __NR_exit_group:
+      // Allow exit() with a non-default return code.
+      return ErrorCode(ErrorCode::ERR_ALLOWED);
+    default:
+      // Make everything else fail in an easily recognizable way.
+      return ErrorCode(EINVAL);
   }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ProbePolicy);
-};
+}
 
 void ProbeProcess(void) {
   if (syscall(__NR_getpid) < 0 && errno == EPERM) {
@@ -84,17 +78,10 @@ void ProbeProcess(void) {
   }
 }
 
-class AllowAllPolicy : public SandboxBPFPolicy {
- public:
-  AllowAllPolicy() {}
-  virtual ErrorCode EvaluateSyscall(SandboxBPF*, int sysnum) const OVERRIDE {
-    DCHECK(SandboxBPF::IsValidSyscallNumber(sysnum));
-    return ErrorCode(ErrorCode::ERR_ALLOWED);
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(AllowAllPolicy);
-};
+ErrorCode AllowAllEvaluator(SandboxBPF*, int sysnum, void*) {
+  DCHECK(SandboxBPF::IsValidSyscallNumber(sysnum));
+  return ErrorCode(ErrorCode::ERR_ALLOWED);
+}
 
 void TryVsyscallProcess(void) {
   time_t current_time;
@@ -252,7 +239,8 @@ bool SandboxBPF::IsValidSyscallNumber(int sysnum) {
 }
 
 bool SandboxBPF::RunFunctionInPolicy(void (*code_in_sandbox)(),
-                                     scoped_ptr<SandboxBPFPolicy> policy) {
+                                     EvaluateSyscall syscall_evaluator,
+                                     void* aux) {
   // Block all signals before forking a child process. This prevents an
   // attacker from manipulating our test by sending us an unexpected signal.
   sigset_t old_mask, new_mask;
@@ -322,7 +310,7 @@ bool SandboxBPF::RunFunctionInPolicy(void (*code_in_sandbox)(),
 #endif
     }
 
-    SetSandboxPolicy(policy.release());
+    SetSandboxPolicyDeprecated(syscall_evaluator, aux);
     if (!StartSandbox(PROCESS_SINGLE_THREADED)) {
       SANDBOX_DIE(NULL);
     }
@@ -371,11 +359,8 @@ bool SandboxBPF::RunFunctionInPolicy(void (*code_in_sandbox)(),
 }
 
 bool SandboxBPF::KernelSupportSeccompBPF() {
-  return RunFunctionInPolicy(ProbeProcess,
-                             scoped_ptr<SandboxBPFPolicy>(new ProbePolicy())) &&
-         RunFunctionInPolicy(
-             TryVsyscallProcess,
-             scoped_ptr<SandboxBPFPolicy>(new AllowAllPolicy()));
+  return RunFunctionInPolicy(ProbeProcess, ProbeEvaluator, 0) &&
+         RunFunctionInPolicy(TryVsyscallProcess, AllowAllEvaluator, 0);
 }
 
 SandboxBPF::SandboxStatus SandboxBPF::SupportsSeccompSandbox(int proc_fd) {
@@ -490,6 +475,15 @@ void SandboxBPF::PolicySanityChecks(SandboxBPFPolicy* policy) {
   return;
 }
 
+// Deprecated API, supported with a wrapper to the new API.
+void SandboxBPF::SetSandboxPolicyDeprecated(EvaluateSyscall syscall_evaluator,
+                                            void* aux) {
+  if (sandbox_has_started_ || !conds_) {
+    SANDBOX_DIE("Cannot change policy after sandbox has started");
+  }
+  SetSandboxPolicy(new CompatibilityPolicy<void>(syscall_evaluator, aux));
+}
+
 // Don't take a scoped_ptr here, polymorphism make their use awkward.
 void SandboxBPF::SetSandboxPolicy(SandboxBPFPolicy* policy) {
   DCHECK(!policy_);
diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf.h b/sandbox/linux/seccomp-bpf/sandbox_bpf.h
index 9bb414a..923a9f3 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf.h
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf.h
@@ -65,6 +65,14 @@ class SANDBOX_EXPORT SandboxBPF {
     PROCESS_MULTI_THREADED,   // The program may be multi-threaded.
   };
 
+  // When calling setSandboxPolicy(), the caller can provide an arbitrary
+  // pointer in |aux|. This pointer will then be forwarded to the sandbox
+  // policy each time a call is made through an EvaluateSyscall function
+  // pointer.  One common use case would be to pass the "aux" pointer as an
+  // argument to Trap() functions.
+  typedef ErrorCode (*EvaluateSyscall)(SandboxBPF* sandbox_compiler,
+                                       int system_call_number,
+                                       void* aux);
   // A vector of BPF instructions that need to be installed as a filter
   // program in the kernel.
   typedef std::vector<struct sock_filter> Program;
@@ -101,6 +109,20 @@ class SANDBOX_EXPORT SandboxBPF {
   // eventually close it when "StartSandbox()" executes.
   void set_proc_fd(int proc_fd);
 
+  // The system call evaluator function is called with the system
+  // call number. It can decide to allow the system call unconditionally
+  // by returning ERR_ALLOWED; it can deny the system call unconditionally by
+  // returning an appropriate "errno" value; or it can request inspection
+  // of system call argument(s) by returning a suitable ErrorCode.
+  // The "aux" parameter can be used to pass optional data to the system call
+  // evaluator. There are different possible uses for this data, but one of the
+  // use cases would be for the policy to then forward this pointer to a Trap()
+  // handler. In this case, of course, the data that is pointed to must remain
+  // valid for the entire time that Trap() handlers can be called; typically,
+  // this would be the lifetime of the program.
+  // DEPRECATED: use the policy interface below.
+  void SetSandboxPolicyDeprecated(EvaluateSyscall syscallEvaluator, void* aux);
+
   // Set the BPF policy as |policy|. Ownership of |policy| is transfered here
   // to the sandbox object.
   void SetSandboxPolicy(SandboxBPFPolicy* policy);
@@ -207,7 +229,8 @@ class SANDBOX_EXPORT SandboxBPF {
   // policy. The caller has to make sure that "this" has not yet been
   // initialized with any other policies.
   bool RunFunctionInPolicy(void (*code_in_sandbox)(),
-                           scoped_ptr<SandboxBPFPolicy> policy);
+                           EvaluateSyscall syscall_evaluator,
+                           void* aux);
 
   // Performs a couple of sanity checks to verify that the kernel supports the
   // features that we need for successful sandboxing.
diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc b/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
index b5bfd35..3b7470b 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
@@ -22,7 +22,6 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
-#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "build/build_config.h"
 #include "sandbox/linux/seccomp-bpf/bpf_tests.h"
@@ -85,38 +84,29 @@ intptr_t FakeGetPid(const struct arch_seccomp_data& args, void* aux) {
   return (*pid_ptr)++;
 }
 
-class VerboseAPITestingPolicy : public SandboxBPFPolicy {
- public:
-  VerboseAPITestingPolicy(pid_t* pid_ptr) : pid_ptr_(pid_ptr) {}
-
-  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
-                                    int sysno) const OVERRIDE {
-    DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
-    if (sysno == __NR_getpid) {
-      return sandbox->Trap(FakeGetPid, pid_ptr_);
-    }
+ErrorCode VerboseAPITestingPolicy(SandboxBPF* sandbox, int sysno, void* aux) {
+  if (!SandboxBPF::IsValidSyscallNumber(sysno)) {
+    return ErrorCode(ENOSYS);
+  } else if (sysno == __NR_getpid) {
+    return sandbox->Trap(FakeGetPid, aux);
+  } else {
     return ErrorCode(ErrorCode::ERR_ALLOWED);
   }
-
- private:
-  pid_t* pid_ptr_;
-  DISALLOW_COPY_AND_ASSIGN(VerboseAPITestingPolicy);
-};
+}
 
 SANDBOX_TEST(SandboxBPF, DISABLE_ON_TSAN(VerboseAPITesting)) {
   if (SandboxBPF::SupportsSeccompSandbox(-1) ==
       sandbox::SandboxBPF::STATUS_AVAILABLE) {
-    pid_t pid;
-
+    pid_t test_var = 0;
     SandboxBPF sandbox;
-    sandbox.SetSandboxPolicy(new VerboseAPITestingPolicy(&pid));
+    sandbox.SetSandboxPolicyDeprecated(VerboseAPITestingPolicy, &test_var);
     BPF_ASSERT(sandbox.StartSandbox(SandboxBPF::PROCESS_SINGLE_THREADED));
 
-    BPF_ASSERT_EQ(0, pid);
-    BPF_ASSERT_EQ(0, syscall(__NR_getpid));
-    BPF_ASSERT_EQ(1, pid);
-    BPF_ASSERT_EQ(1, syscall(__NR_getpid));
-    BPF_ASSERT_EQ(2, pid);
+    BPF_ASSERT(test_var == 0);
+    BPF_ASSERT(syscall(__NR_getpid) == 0);
+    BPF_ASSERT(test_var == 1);
+    BPF_ASSERT(syscall(__NR_getpid) == 1);
+    BPF_ASSERT(test_var == 2);
 
     // N.B.: Any future call to getpid() would corrupt the stack.
     //       This is OK. The SANDBOX_TEST() macro is guaranteed to
@@ -294,53 +284,43 @@ BPF_TEST(SandboxBPF, ErrnoTest, ErrnoTestPolicy) {
 
 // Testing the stacking of two sandboxes
 
-class StackingPolicyPartOne : public SandboxBPFPolicy {
- public:
-  StackingPolicyPartOne() {}
-  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
-                                    int sysno) const OVERRIDE {
-    DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
-    switch (sysno) {
-      case __NR_getppid:
-        return sandbox->Cond(0,
-                             ErrorCode::TP_32BIT,
-                             ErrorCode::OP_EQUAL,
-                             0,
-                             ErrorCode(ErrorCode::ERR_ALLOWED),
-                             ErrorCode(EPERM));
-      default:
-        return ErrorCode(ErrorCode::ERR_ALLOWED);
-    }
+ErrorCode StackingPolicyPartOne(SandboxBPF* sandbox, int sysno, void*) {
+  if (!SandboxBPF::IsValidSyscallNumber(sysno)) {
+    return ErrorCode(ENOSYS);
   }
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(StackingPolicyPartOne);
-};
+  switch (sysno) {
+    case __NR_getppid:
+      return sandbox->Cond(0,
+                           ErrorCode::TP_32BIT,
+                           ErrorCode::OP_EQUAL,
+                           0,
+                           ErrorCode(ErrorCode::ERR_ALLOWED),
+                           ErrorCode(EPERM));
+    default:
+      return ErrorCode(ErrorCode::ERR_ALLOWED);
+  }
+}
 
-class StackingPolicyPartTwo : public SandboxBPFPolicy {
- public:
-  StackingPolicyPartTwo() {}
-  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
-                                    int sysno) const OVERRIDE {
-    DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
-    switch (sysno) {
-      case __NR_getppid:
-        return sandbox->Cond(0,
-                             ErrorCode::TP_32BIT,
-                             ErrorCode::OP_EQUAL,
-                             0,
-                             ErrorCode(EINVAL),
-                             ErrorCode(ErrorCode::ERR_ALLOWED));
-      default:
-        return ErrorCode(ErrorCode::ERR_ALLOWED);
-    }
+ErrorCode StackingPolicyPartTwo(SandboxBPF* sandbox, int sysno, void*) {
+  if (!SandboxBPF::IsValidSyscallNumber(sysno)) {
+    return ErrorCode(ENOSYS);
   }
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(StackingPolicyPartTwo);
-};
+  switch (sysno) {
+    case __NR_getppid:
+      return sandbox->Cond(0,
+                           ErrorCode::TP_32BIT,
+                           ErrorCode::OP_EQUAL,
+                           0,
+                           ErrorCode(EINVAL),
+                           ErrorCode(ErrorCode::ERR_ALLOWED));
+    default:
+      return ErrorCode(ErrorCode::ERR_ALLOWED);
+  }
+}
 
-BPF_TEST_C(SandboxBPF, StackingPolicy, StackingPolicyPartOne) {
+BPF_TEST(SandboxBPF, StackingPolicy, StackingPolicyPartOne) {
   errno = 0;
   BPF_ASSERT(syscall(__NR_getppid, 0) > 0);
   BPF_ASSERT(errno == 0);
@@ -351,7 +331,7 @@ BPF_TEST_C(SandboxBPF, StackingPolicy, StackingPolicyPartOne) {
   // Stack a second sandbox with its own policy. Verify that we can further
   // restrict filters, but we cannot relax existing filters.
   SandboxBPF sandbox;
-  sandbox.SetSandboxPolicy(new StackingPolicyPartTwo());
+  sandbox.SetSandboxPolicyDeprecated(StackingPolicyPartTwo, NULL);
   BPF_ASSERT(sandbox.StartSandbox(SandboxBPF::PROCESS_SINGLE_THREADED));
 
   errno = 0;