bpf_dsl: move more implementation details out of bpf_dsl.h

BUG=414363

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

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

sandbox/linux/bpf_dsl/bpf_dsl.cc

@@ -13,7 +13,6 @@
 #include "sandbox/linux/bpf_dsl/bpf_dsl_impl.h"
 #include "sandbox/linux/bpf_dsl/policy_compiler.h"
 #include "sandbox/linux/seccomp-bpf/errorcode.h"
-#include "sandbox/linux/seccomp-bpf/syscall_iterator.h"
 
 namespace sandbox {
 namespace bpf_dsl {
@@ -377,26 +376,6 @@ ResultExpr SandboxBPFDSLPolicy::InvalidSyscall() const {
   return Error(ENOSYS);
 }
 
-ErrorCode SandboxBPFDSLPolicy::EvaluateSyscall(PolicyCompiler* pc,
-                                               int sysno) const {
-  return EvaluateSyscall(sysno)->Compile(pc);
-}
-
-ErrorCode SandboxBPFDSLPolicy::InvalidSyscall(PolicyCompiler* pc) const {
-  return InvalidSyscall()->Compile(pc);
-}
-
-bool SandboxBPFDSLPolicy::HasUnsafeTraps() const {
-  for (SyscallIterator iter(false); !iter.Done();) {
-    uint32_t sysnum = iter.Next();
-    if (SyscallIterator::IsValid(sysnum) &&
-        EvaluateSyscall(sysnum)->HasUnsafeTraps()) {
-      return true;
-    }
-  }
-  return InvalidSyscall()->HasUnsafeTraps();
-}
-
 ResultExpr SandboxBPFDSLPolicy::Trap(TrapRegistry::TrapFnc trap_func,
                                      const void* aux) {
   return bpf_dsl::Trap(trap_func, aux);

sandbox/linux/bpf_dsl/bpf_dsl.h

@@ -16,14 +16,6 @@
 #include "sandbox/linux/bpf_dsl/trap_registry.h"
 #include "sandbox/sandbox_export.h"
 
-namespace sandbox {
-class ErrorCode;
-class Verifier;
-namespace bpf_dsl {
-class PolicyCompiler;
-}
-}
-
 // The sandbox::bpf_dsl namespace provides a domain-specific language
 // to make writing BPF policies more expressive.  In general, the
 // object types all have value semantics (i.e., they can be copied
@@ -127,14 +119,6 @@ class SANDBOX_EXPORT SandboxBPFDSLPolicy {
   static ResultExpr Trap(TrapRegistry::TrapFnc trap_func, const void* aux);
 
  private:
-  friend PolicyCompiler;
-  friend Verifier;
-
-  // Private methods used for compiling and verifying policies.
-  ErrorCode EvaluateSyscall(PolicyCompiler* pc, int sysno) const;
-  ErrorCode InvalidSyscall(PolicyCompiler* pc) const;
-  bool HasUnsafeTraps() const;
-
   DISALLOW_COPY_AND_ASSIGN(SandboxBPFDSLPolicy);
 };
 

sandbox/linux/bpf_dsl/bpf_dsl_impl.h

@@ -11,20 +11,24 @@
 
 namespace sandbox {
 class ErrorCode;
-class SandboxBPF;
 
 namespace bpf_dsl {
+class PolicyCompiler;
+
 namespace internal {
 
 // Internal interface implemented by BoolExpr implementations.
 class BoolExprImpl : public base::RefCounted<BoolExprImpl> {
  public:
-  BoolExprImpl() {}
+  // Compile uses |pc| to construct an ErrorCode that conditionally continues
+  // to either |true_ec| or |false_ec|, depending on whether the represented
+  // boolean expression is true or false.
   virtual ErrorCode Compile(PolicyCompiler* pc,
                             ErrorCode true_ec,
                             ErrorCode false_ec) const = 0;
 
  protected:
+  BoolExprImpl() {}
   virtual ~BoolExprImpl() {}
 
  private:
@@ -35,11 +39,16 @@ class BoolExprImpl : public base::RefCounted<BoolExprImpl> {
 // Internal interface implemented by ResultExpr implementations.
 class ResultExprImpl : public base::RefCounted<ResultExprImpl> {
  public:
-  ResultExprImpl() {}
+  // Compile uses |pc| to construct an ErrorCode analogous to the represented
+  // result expression.
   virtual ErrorCode Compile(PolicyCompiler* pc) const = 0;
+
+  // HasUnsafeTraps returns whether the result expression is or recursively
+  // contains an unsafe trap expression.
   virtual bool HasUnsafeTraps() const;
 
  protected:
+  ResultExprImpl() {}
   virtual ~ResultExprImpl() {}
 
  private:

sandbox/linux/bpf_dsl/policy_compiler.cc

@@ -13,6 +13,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "sandbox/linux/bpf_dsl/bpf_dsl.h"
+#include "sandbox/linux/bpf_dsl/bpf_dsl_impl.h"
 #include "sandbox/linux/seccomp-bpf/codegen.h"
 #include "sandbox/linux/seccomp-bpf/die.h"
 #include "sandbox/linux/seccomp-bpf/errorcode.h"
@@ -74,6 +75,17 @@ intptr_t BPFFailure(const struct arch_seccomp_data&, void* aux) {
   SANDBOX_DIE(static_cast<char*>(aux));
 }
 
+bool HasUnsafeTraps(const SandboxBPFDSLPolicy* policy) {
+  for (SyscallIterator iter(false); !iter.Done();) {
+    uint32_t sysnum = iter.Next();
+    if (SyscallIterator::IsValid(sysnum) &&
+        policy->EvaluateSyscall(sysnum)->HasUnsafeTraps()) {
+      return true;
+    }
+  }
+  return policy->InvalidSyscall()->HasUnsafeTraps();
+}
+
 }  // namespace
 
 struct PolicyCompiler::Range {
@@ -88,14 +100,14 @@ PolicyCompiler::PolicyCompiler(const SandboxBPFDSLPolicy* policy,
       registry_(registry),
       conds_(),
       gen_(),
-      has_unsafe_traps_(policy_->HasUnsafeTraps()) {
+      has_unsafe_traps_(HasUnsafeTraps(policy_)) {
 }
 
 PolicyCompiler::~PolicyCompiler() {
 }
 
 scoped_ptr<CodeGen::Program> PolicyCompiler::Compile() {
-  if (!IsDenied(policy_->InvalidSyscall(this))) {
+  if (!IsDenied(policy_->InvalidSyscall()->Compile(this))) {
     SANDBOX_DIE("Policies should deny invalid system calls.");
   }
 
@@ -112,7 +124,7 @@ scoped_ptr<CodeGen::Program> PolicyCompiler::Compile() {
     }
 
     for (int sysnum : kSyscallsRequiredForUnsafeTraps) {
-      if (!policy_->EvaluateSyscall(this, sysnum)
+      if (!policy_->EvaluateSyscall(sysnum)->Compile(this)
                .Equals(ErrorCode(ErrorCode::ERR_ALLOWED))) {
         SANDBOX_DIE(
             "Policies that use UnsafeTrap() must unconditionally allow all "
@@ -238,17 +250,17 @@ void PolicyCompiler::FindRanges(Ranges* ranges) {
   // deal with this disparity by enumerating from MIN_SYSCALL to MAX_SYSCALL,
   // and then verifying that the rest of the number range (both positive and
   // negative) all return the same ErrorCode.
-  const ErrorCode invalid_err = policy_->InvalidSyscall(this);
+  const ErrorCode invalid_err = policy_->InvalidSyscall()->Compile(this);
   uint32_t old_sysnum = 0;
   ErrorCode old_err = SyscallIterator::IsValid(old_sysnum)
-                          ? policy_->EvaluateSyscall(this, old_sysnum)
+                          ? policy_->EvaluateSyscall(old_sysnum)->Compile(this)
                           : invalid_err;
 
   for (SyscallIterator iter(false); !iter.Done();) {
     uint32_t sysnum = iter.Next();
     ErrorCode err =
         SyscallIterator::IsValid(sysnum)
-            ? policy_->EvaluateSyscall(this, static_cast<int>(sysnum))
+            ? policy_->EvaluateSyscall(static_cast<int>(sysnum))->Compile(this)
             : invalid_err;
     if (!err.Equals(old_err) || iter.Done()) {
       ranges->push_back(Range(old_sysnum, sysnum - 1, old_err));

sandbox/linux/seccomp-bpf/verifier.cc

@@ -9,6 +9,7 @@
 #include <limits>
 
 #include "sandbox/linux/bpf_dsl/bpf_dsl.h"
+#include "sandbox/linux/bpf_dsl/bpf_dsl_impl.h"
 #include "sandbox/linux/bpf_dsl/policy_compiler.h"
 #include "sandbox/linux/seccomp-bpf/errorcode.h"
 #include "sandbox/linux/seccomp-bpf/linux_seccomp.h"
@@ -340,8 +341,8 @@ bool Verifier::VerifyBPF(bpf_dsl::PolicyCompiler* compiler,
 #endif
 #endif
     ErrorCode code = iter.IsValid(sysnum)
-                         ? policy.EvaluateSyscall(compiler, sysnum)
-                         : policy.InvalidSyscall(compiler);
+                         ? policy.EvaluateSyscall(sysnum)->Compile(compiler)
+                         : policy.InvalidSyscall()->Compile(compiler);
     if (!VerifyErrorCode(compiler, program, &data, code, code, err)) {
       return false;
     }