Add SandboxBPFPolicy::InvalidSyscall() to simplify writing policies

Instead of requiring every BPF policy to check IsValidSyscallNumber()
and return a fixed value when it's true, provide an alternative entry
point and guarantee that EvaluateSyscall() will only be called for
valid syscall numbers.

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

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

sandbox/linux/sandbox_linux.gypi

@@ -129,6 +129,7 @@
         'seccomp-bpf/sandbox_bpf.cc',
         'seccomp-bpf/sandbox_bpf.h',
         'seccomp-bpf/sandbox_bpf_compatibility_policy.h',
+        'seccomp-bpf/sandbox_bpf_policy.cc',
         'seccomp-bpf/sandbox_bpf_policy.h',
         'seccomp-bpf/syscall.cc',
         'seccomp-bpf/syscall.h',

sandbox/linux/seccomp-bpf/sandbox_bpf.cc

@@ -79,9 +79,7 @@ void ProbeProcess(void) {
 }
 
 ErrorCode AllowAllEvaluator(SandboxBPF*, int sysnum, void*) {
-  if (!SandboxBPF::IsValidSyscallNumber(sysnum)) {
-    return ErrorCode(ENOSYS);
-  }
+  DCHECK(SandboxBPF::IsValidSyscallNumber(sysnum));
   return ErrorCode(ErrorCode::ERR_ALLOWED);
 }
 
@@ -188,13 +186,21 @@ class RedirectToUserSpacePolicyWrapper : public SandboxBPFPolicy {
     ErrorCode err =
         wrapped_policy_->EvaluateSyscall(sandbox_compiler, system_call_number);
     if ((err.err() & SECCOMP_RET_ACTION) == SECCOMP_RET_ERRNO) {
-      return sandbox_compiler->Trap(
-          ReturnErrno, reinterpret_cast<void*>(err.err() & SECCOMP_RET_DATA));
+      return ReturnErrnoViaTrap(sandbox_compiler, err.err() & SECCOMP_RET_DATA);
     }
     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));
+  }
+
   const SandboxBPFPolicy* wrapped_policy_;
   DISALLOW_COPY_AND_ASSIGN(RedirectToUserSpacePolicyWrapper);
 };
@@ -463,13 +469,8 @@ bool SandboxBPF::StartSandbox(SandboxThreadState thread_state) {
 }
 
 void SandboxBPF::PolicySanityChecks(SandboxBPFPolicy* policy) {
-  for (SyscallIterator iter(true); !iter.Done();) {
-    uint32_t sysnum = iter.Next();
-    if (!IsDenied(policy->EvaluateSyscall(this, sysnum))) {
-      SANDBOX_DIE(
-          "Policies should deny system calls that are outside the "
-          "expected range (typically MIN_SYSCALL..MAX_SYSCALL)");
-    }
+  if (!IsDenied(policy->InvalidSyscall(this))) {
+    SANDBOX_DIE("Policies should deny invalid system calls.");
   }
   return;
 }
@@ -745,20 +746,18 @@ void SandboxBPF::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);
   uint32_t old_sysnum = 0;
-  ErrorCode old_err = policy_->EvaluateSyscall(this, old_sysnum);
-  ErrorCode invalid_err = policy_->EvaluateSyscall(this, MIN_SYSCALL - 1);
+  ErrorCode old_err = IsValidSyscallNumber(old_sysnum)
+                          ? policy_->EvaluateSyscall(this, old_sysnum)
+                          : invalid_err;
 
   for (SyscallIterator iter(false); !iter.Done();) {
     uint32_t sysnum = iter.Next();
-    ErrorCode err = policy_->EvaluateSyscall(this, static_cast<int>(sysnum));
-    if (!iter.IsValid(sysnum) && !invalid_err.Equals(err)) {
-      // A proper sandbox policy should always treat system calls outside of
-      // the range MIN_SYSCALL..MAX_SYSCALL (i.e. anything that returns
-      // "false" for SyscallIterator::IsValid()) identically. Typically, all
-      // of these system calls would be denied with the same ErrorCode.
-      SANDBOX_DIE("Invalid seccomp policy");
-    }
+    ErrorCode err =
+        IsValidSyscallNumber(sysnum)
+            ? policy_->EvaluateSyscall(this, static_cast<int>(sysnum))
+            : invalid_err;
     if (!err.Equals(old_err) || iter.Done()) {
       ranges->push_back(Range(old_sysnum, sysnum - 1, old_err));
       old_sysnum = sysnum;

sandbox/linux/seccomp-bpf/sandbox_bpf_compatibility_policy.h

@@ -28,6 +28,7 @@ class CompatibilityPolicy : public SandboxBPFPolicy {
 
   virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox_compiler,
                                     int system_call_number) const OVERRIDE {
+    DCHECK(SandboxBPF::IsValidSyscallNumber(system_call_number));
     return syscall_evaluator_(sandbox_compiler, system_call_number, aux_);
   }
 

sandbox/linux/seccomp-bpf/sandbox_bpf_policy.cc

+// Copyright 2014 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_policy.h"
+
+#include <errno.h>
+
+#include "sandbox/linux/seccomp-bpf/errorcode.h"
+
+namespace sandbox {
+
+ErrorCode SandboxBPFPolicy::InvalidSyscall(SandboxBPF* sandbox_compiler) const {
+  return ErrorCode(ENOSYS);
+}
+
+}  // namespace sandbox

sandbox/linux/seccomp-bpf/sandbox_bpf_policy.h

@@ -6,6 +6,7 @@
 #define SANDBOX_LINUX_SECCOMP_BPF_SANDBOX_BPF_POLICY_H_
 
 #include "base/basictypes.h"
+#include "sandbox/sandbox_export.h"
 
 namespace sandbox {
 
@@ -13,7 +14,7 @@ class ErrorCode;
 class SandboxBPF;
 
 // This is the interface to implement to define a BPF sandbox policy.
-class SandboxBPFPolicy {
+class SANDBOX_EXPORT SandboxBPFPolicy {
  public:
   SandboxBPFPolicy() {}
   virtual ~SandboxBPFPolicy() {}
@@ -23,9 +24,14 @@ class SandboxBPFPolicy {
   // 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.
+  // Will only be called for valid system call numbers.
   virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox_compiler,
                                     int system_call_number) const = 0;
 
+  // The InvalidSyscall method specifies the behavior used for invalid
+  // system calls.  The default implementation is to return ENOSYS.
+  virtual ErrorCode InvalidSyscall(SandboxBPF* sandbox_compiler) const;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(SandboxBPFPolicy);
 };

sandbox/linux/seccomp-bpf/verifier.cc

@@ -387,7 +387,9 @@ bool Verifier::VerifyBPF(SandboxBPF* sandbox,
     }
 #endif
 #endif
-    ErrorCode code = policy.EvaluateSyscall(sandbox, sysnum);
+    ErrorCode code = iter.IsValid(sysnum)
+                         ? policy.EvaluateSyscall(sandbox, sysnum)
+                         : policy.InvalidSyscall(sandbox);
     if (!VerifyErrorCode(sandbox, program, &data, code, code, err)) {
       return false;
     }