sandbox/linux: refactor bpf_dsl dependency on die.h

This CL changes the bpf_dsl "Kill" builtin function to simply map to
SECCOMP_RET_KILL, rather than to a trap handler.  Additionally, it
changes the default PolicyCompiler behavior for handling impossible
conditions to use this behavior.

However, it also adds SetPanicFunc as a way to override this default
behavior, and SandboxBPF uses this to maintain Chrome's historical
behavior of printing an error message with SANDBOX_DIE.

Arguably the Policy object should actually be responsible for
providing a Panic function, but that change will require modifying
existing Policy classes elsewhere in the Chromium source tree, so
I'll look into that in a followup CL.

BUG=449357

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

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

10 files changed, 76 insertions, 25 deletions

diff --git a/sandbox/linux/bpf_dsl/bpf_dsl.cc b/sandbox/linux/bpf_dsl/bpf_dsl.cc
index 2c53ab5..f0ee0a2 100644
--- a/sandbox/linux/bpf_dsl/bpf_dsl.cc
+++ b/sandbox/linux/bpf_dsl/bpf_dsl.cc
@@ -10,17 +10,12 @@
 #include "base/memory/ref_counted.h"
 #include "sandbox/linux/bpf_dsl/bpf_dsl_impl.h"
 #include "sandbox/linux/bpf_dsl/policy_compiler.h"
-#include "sandbox/linux/seccomp-bpf/die.h"
 #include "sandbox/linux/seccomp-bpf/errorcode.h"
 
 namespace sandbox {
 namespace bpf_dsl {
 namespace {
 
-intptr_t BPFFailure(const struct arch_seccomp_data&, void* aux) {
-  SANDBOX_DIE(static_cast<char*>(aux));
-}
-
 class AllowResultExprImpl : public internal::ResultExprImpl {
  public:
   AllowResultExprImpl() {}
@@ -57,6 +52,22 @@ class ErrorResultExprImpl : public internal::ResultExprImpl {
   DISALLOW_COPY_AND_ASSIGN(ErrorResultExprImpl);
 };
 
+class KillResultExprImpl : public internal::ResultExprImpl {
+ public:
+  KillResultExprImpl() {}
+
+  ErrorCode Compile(PolicyCompiler* pc) const override {
+    return ErrorCode(ErrorCode::ERR_KILL);
+  }
+
+  bool IsDeny() const override { return true; }
+
+ private:
+  ~KillResultExprImpl() override {}
+
+  DISALLOW_COPY_AND_ASSIGN(KillResultExprImpl);
+};
+
 class TraceResultExprImpl : public internal::ResultExprImpl {
  public:
   TraceResultExprImpl(uint16_t aux) : aux_(aux) {}
@@ -276,8 +287,8 @@ ResultExpr Error(int err) {
   return ResultExpr(new const ErrorResultExprImpl(err));
 }
 
-ResultExpr Kill(const char* msg) {
-  return Trap(BPFFailure, msg);
+ResultExpr Kill() {
+  return ResultExpr(new const KillResultExprImpl());
 }
 
 ResultExpr Trace(uint16_t aux) {
diff --git a/sandbox/linux/bpf_dsl/bpf_dsl.h b/sandbox/linux/bpf_dsl/bpf_dsl.h
index b8fe2f9..913ab9c 100644
--- a/sandbox/linux/bpf_dsl/bpf_dsl.h
+++ b/sandbox/linux/bpf_dsl/bpf_dsl.h
@@ -55,7 +55,7 @@
 //
 // More generally, the DSL currently supports the following grammar:
 //
-//   result = Allow() | Error(errno) | Kill(msg) | Trace(aux)
+//   result = Allow() | Error(errno) | Kill() | Trace(aux)
 //          | Trap(trap_func, aux) | UnsafeTrap(trap_func, aux)
 //          | If(bool, result)[.ElseIf(bool, result)].Else(result)
 //          | Switch(arg)[.Case(val, result)].Default(result)
@@ -89,8 +89,8 @@ SANDBOX_EXPORT ResultExpr Allow();
 // side effects.
 SANDBOX_EXPORT ResultExpr Error(int err);
 
-// Kill specifies a result to kill the program and print an error message.
-SANDBOX_EXPORT ResultExpr Kill(const char* msg);
+// Kill specifies a result to kill the process (task) immediately.
+SANDBOX_EXPORT ResultExpr Kill();
 
 // Trace specifies a result to notify a tracing process via the
 // PTRACE_EVENT_SECCOMP event and allow it to change or skip the system call.
diff --git a/sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc b/sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc
index 2667579..ab1a2c8 100644
--- a/sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc
+++ b/sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc
@@ -134,6 +134,10 @@ class PolicyEmulator {
     EXPECT_EQ(SECCOMP_RET_ERRNO | err, Emulate(data));
   }
 
+  void ExpectKill(const struct arch_seccomp_data& data) const {
+    EXPECT_EQ(SECCOMP_RET_KILL, Emulate(data));
+  }
+
  private:
   CodeGen::Program program_;
   FakeTrapRegistry traps_;
@@ -152,7 +156,7 @@ class BasicPolicy : public Policy {
     }
     if (sysno == __NR_setuid) {
       const Arg<uid_t> uid(0);
-      return If(uid != 42, Error(ESRCH)).Else(Error(ENOMEM));
+      return If(uid != 42, Kill()).Else(Allow());
     }
     return Allow();
   }
@@ -168,8 +172,8 @@ TEST(BPFDSL, Basic) {
   emulator.ExpectErrno(EPERM, FakeSyscall(__NR_getpgid, 0));
   emulator.ExpectErrno(EINVAL, FakeSyscall(__NR_getpgid, 1));
 
-  emulator.ExpectErrno(ENOMEM, FakeSyscall(__NR_setuid, 42));
-  emulator.ExpectErrno(ESRCH, FakeSyscall(__NR_setuid, 43));
+  emulator.ExpectAllow(FakeSyscall(__NR_setuid, 42));
+  emulator.ExpectKill(FakeSyscall(__NR_setuid, 43));
 }
 
 /* On IA-32, socketpair() is implemented via socketcall(). :-( */
diff --git a/sandbox/linux/bpf_dsl/policy_compiler.cc b/sandbox/linux/bpf_dsl/policy_compiler.cc
index f38232f..7a9d8ad 100644
--- a/sandbox/linux/bpf_dsl/policy_compiler.cc
+++ b/sandbox/linux/bpf_dsl/policy_compiler.cc
@@ -56,6 +56,10 @@ bool HasExactlyOneBit(uint64_t x) {
   return x != 0 && (x & (x - 1)) == 0;
 }
 
+ResultExpr DefaultPanic(const char* error) {
+  return Kill();
+}
+
 // A Trap() handler that returns an "errno" value. The value is encoded
 // in the "aux" parameter.
 intptr_t ReturnErrno(const struct arch_seccomp_data&, void* aux) {
@@ -88,6 +92,7 @@ PolicyCompiler::PolicyCompiler(const Policy* policy, TrapRegistry* registry)
     : policy_(policy),
       registry_(registry),
       escapepc_(0),
+      panic_func_(DefaultPanic),
       conds_(),
       gen_(),
       has_unsafe_traps_(HasUnsafeTraps(policy_)) {
@@ -137,6 +142,10 @@ void PolicyCompiler::DangerousSetEscapePC(uint64_t escapepc) {
   escapepc_ = escapepc;
 }
 
+void PolicyCompiler::SetPanicFunc(PanicFunc panic_func) {
+  panic_func_ = panic_func;
+}
+
 CodeGen::Node PolicyCompiler::AssemblePolicy() {
   // A compiled policy consists of three logical parts:
   //   1. Check that the "arch" field matches the expected architecture.
@@ -152,9 +161,9 @@ CodeGen::Node PolicyCompiler::CheckArch(CodeGen::Node passed) {
   // system call.
   return gen_.MakeInstruction(
       BPF_LD + BPF_W + BPF_ABS, SECCOMP_ARCH_IDX,
-      gen_.MakeInstruction(
-          BPF_JMP + BPF_JEQ + BPF_K, SECCOMP_ARCH, passed,
-          CompileResult(Kill("Invalid audit architecture in BPF filter"))));
+      gen_.MakeInstruction(BPF_JMP + BPF_JEQ + BPF_K, SECCOMP_ARCH, passed,
+                           CompileResult(panic_func_(
+                               "Invalid audit architecture in BPF filter"))));
 }
 
 CodeGen::Node PolicyCompiler::MaybeAddEscapeHatch(CodeGen::Node rest) {
@@ -209,7 +218,7 @@ CodeGen::Node PolicyCompiler::CheckSyscallNumber(CodeGen::Node passed) {
     // On Intel architectures, verify that system call numbers are in the
     // expected number range.
     CodeGen::Node invalidX32 =
-        CompileResult(Kill("Illegal mixing of system call ABIs"));
+        CompileResult(panic_func_("Illegal mixing of system call ABIs"));
     if (kIsX32) {
       // The newer x32 API always sets bit 30.
       return gen_.MakeInstruction(
@@ -445,7 +454,7 @@ CodeGen::Node PolicyCompiler::CondExpressionHalf(const ErrorCode& cond,
 }
 
 ErrorCode PolicyCompiler::Unexpected64bitArgument() {
-  return Kill("Unexpected 64bit argument detected")->Compile(this);
+  return panic_func_("Unexpected 64bit argument detected")->Compile(this);
 }
 
 ErrorCode PolicyCompiler::Error(int err) {
diff --git a/sandbox/linux/bpf_dsl/policy_compiler.h b/sandbox/linux/bpf_dsl/policy_compiler.h
index df38d4c..0e02343 100644
--- a/sandbox/linux/bpf_dsl/policy_compiler.h
+++ b/sandbox/linux/bpf_dsl/policy_compiler.h
@@ -15,6 +15,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "sandbox/linux/bpf_dsl/bpf_dsl_forward.h"
 #include "sandbox/linux/bpf_dsl/codegen.h"
+#include "sandbox/linux/bpf_dsl/trap_registry.h"
 #include "sandbox/linux/seccomp-bpf/errorcode.h"
 #include "sandbox/sandbox_export.h"
 
@@ -27,6 +28,8 @@ class Policy;
 // Linux kernel.
 class SANDBOX_EXPORT PolicyCompiler {
  public:
+  using PanicFunc = bpf_dsl::ResultExpr (*)(const char* error);
+
   PolicyCompiler(const Policy* policy, TrapRegistry* registry);
   ~PolicyCompiler();
 
@@ -38,6 +41,12 @@ class SANDBOX_EXPORT PolicyCompiler {
   // system calls, regardless of policy.
   void DangerousSetEscapePC(uint64_t escapepc);
 
+  // SetPanicFunc sets the callback function used for handling faulty
+  // system call conditions.  The default behavior is to immediately kill
+  // the process.
+  // TODO(mdempsky): Move this into Policy?
+  void SetPanicFunc(PanicFunc panic_func);
+
   // Error returns an ErrorCode to indicate the system call should fail with
   // the specified error number.
   ErrorCode Error(int err);
@@ -145,6 +154,7 @@ class SANDBOX_EXPORT PolicyCompiler {
   const Policy* policy_;
   TrapRegistry* registry_;
   uint64_t escapepc_;
+  PanicFunc panic_func_;
 
   Conds conds_;
   CodeGen gen_;
diff --git a/sandbox/linux/bpf_dsl/verifier.cc b/sandbox/linux/bpf_dsl/verifier.cc
index 417c663..a4b656f 100644
--- a/sandbox/linux/bpf_dsl/verifier.cc
+++ b/sandbox/linux/bpf_dsl/verifier.cc
@@ -368,12 +368,12 @@ uint32_t Verifier::EvaluateBPF(const std::vector<struct sock_filter>& program,
       case BPF_RET: {
         uint32_t r = Ret(&state, insn, err);
         switch (r & SECCOMP_RET_ACTION) {
-          case SECCOMP_RET_TRAP:
+          case SECCOMP_RET_ALLOW:
           case SECCOMP_RET_ERRNO:
+          case SECCOMP_RET_KILL:
           case SECCOMP_RET_TRACE:
-          case SECCOMP_RET_ALLOW:
+          case SECCOMP_RET_TRAP:
             break;
-          case SECCOMP_RET_KILL:     // We don't ever generate this
           case SECCOMP_RET_INVALID:  // Should never show up in BPF program
           default:
             *err = "Unexpected return code found in BPF program";
diff --git a/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc b/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc
index e884774..8f89ea2 100644
--- a/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc
+++ b/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc
@@ -1267,7 +1267,7 @@ ResultExpr AllBitTestPolicy::EvaluateSyscall(int sysno) const {
         .Case(9, HasAllBits64(0x300000000ULL))
         .Case(10, HasAllBits64(0x100000001ULL))
 #endif
-        .Default(Kill("Invalid test case number"));
+        .Default(Kill());
   }
   return Allow();
 }
@@ -1453,7 +1453,7 @@ ResultExpr AnyBitTestPolicy::EvaluateSyscall(int sysno) const {
         .Case(9, HasAnyBits64(0x300000000ULL))
         .Case(10, HasAnyBits64(0x100000001ULL))
 #endif
-        .Default(Kill("Invalid test case number"));
+        .Default(Kill());
   }
   return Allow();
 }
@@ -1598,7 +1598,7 @@ ResultExpr MaskedEqualTestPolicy::EvaluateSyscall(int sysno) const {
         .Case(1, MaskedEqual64(0x00ff00ff00000000, 0x005500aa00000000))
         .Case(2, MaskedEqual64(0x00ff00ff00ff00ff, 0x005500aa005500aa))
 #endif
-        .Default(Kill("Invalid test case number"));
+        .Default(Kill());
   }
 
   return Allow();
diff --git a/sandbox/linux/seccomp-bpf/errorcode.cc b/sandbox/linux/seccomp-bpf/errorcode.cc
index 9bb3ddb..e10a08f 100644
--- a/sandbox/linux/seccomp-bpf/errorcode.cc
+++ b/sandbox/linux/seccomp-bpf/errorcode.cc
@@ -18,6 +18,10 @@ ErrorCode::ErrorCode(int err) {
       err_ = SECCOMP_RET_ALLOW;
       error_type_ = ET_SIMPLE;
       break;
+    case ERR_KILL:
+      err_ = SECCOMP_RET_KILL;
+      error_type_ = ET_SIMPLE;
+      break;
     case ERR_MIN_ERRNO... ERR_MAX_ERRNO:
       err_ = SECCOMP_RET_ERRNO + err;
       error_type_ = ET_SIMPLE;
diff --git a/sandbox/linux/seccomp-bpf/errorcode.h b/sandbox/linux/seccomp-bpf/errorcode.h
index d887773..3b27aac 100644
--- a/sandbox/linux/seccomp-bpf/errorcode.h
+++ b/sandbox/linux/seccomp-bpf/errorcode.h
@@ -38,7 +38,10 @@ class SANDBOX_EXPORT ErrorCode {
     // tracer will be notified of a PTRACE_EVENT_SECCOMP and allowed to change
     // or skip the system call.  The lower 16 bits of err will be available to
     // the tracer via PTRACE_GETEVENTMSG.
-    ERR_TRACE   = 0x08000000,
+    ERR_TRACE = 0x08000000,
+
+    // Kill the process immediately.
+    ERR_KILL = 0x10000000,
 
     // Deny the system call with a particular "errno" value.
     // N.B.: It is also possible to return "0" here. That would normally
diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
index 77faba4..6c6912a 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
@@ -16,6 +16,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/third_party/valgrind/valgrind.h"
+#include "sandbox/linux/bpf_dsl/bpf_dsl.h"
 #include "sandbox/linux/bpf_dsl/codegen.h"
 #include "sandbox/linux/bpf_dsl/policy.h"
 #include "sandbox/linux/bpf_dsl/policy_compiler.h"
@@ -109,6 +110,14 @@ uint64_t EscapePC() {
   return static_cast<uint64_t>(static_cast<uintptr_t>(rv));
 }
 
+intptr_t SandboxPanicTrap(const struct arch_seccomp_data&, void* aux) {
+  SANDBOX_DIE(static_cast<const char*>(aux));
+}
+
+bpf_dsl::ResultExpr SandboxPanic(const char* error) {
+  return bpf_dsl::Trap(SandboxPanicTrap, error);
+}
+
 }  // namespace
 
 SandboxBPF::SandboxBPF(bpf_dsl::Policy* policy)
@@ -219,6 +228,7 @@ scoped_ptr<CodeGen::Program> SandboxBPF::AssembleFilter(
   if (Trap::SandboxDebuggingAllowedByUser()) {
     compiler.DangerousSetEscapePC(EscapePC());
   }
+  compiler.SetPanicFunc(SandboxPanic);
   return compiler.Compile(force_verification);
 }