Added a new Verifier class to the BPF compiler.

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"
+#include "sandbox/linux/seccomp-bpf/verifier.h"
 
 // The kernel gives us a sandbox, we turn it into a playground :-)
 // This is version 2 of the playground; version 1 was built on top of
 // pre-BPF seccomp mode.
 namespace playground2 {
 
 Sandbox::ErrorCode Sandbox::probeEvaluator(int signo) {
   switch (signo) {
   case __NR_getpid:
     // Return EPERM so that we can check that the filter actually ran.
@@ skipping 232 matching lines @@
 
   // Evaluate all possible system calls and depending on their
   // exit codes generate a BPF filter.
   // This is very inefficient right now. We need to be much smarter
   // eventually.
   // We currently incur a O(N) overhead on each system call, with N
   // being the number of system calls. It is easy to get this down to
   // O(log_2(M)) with M being the number of system calls that need special
   // treatment.
   EvaluateSyscall evaluateSyscall = evaluators_.begin()->first;
-  for (int sysnum = MIN_SYSCALL; sysnum <= MAX_SYSCALL; ++sysnum) {
+  for (int sysnum = MIN_SYSCALL; sysnum <= MAX_SYSCALL+1; ++sysnum) {

[Chris Evans, 2012/06/11 19:20:41]

Should sysnum be signed?

[Markus (顧孟勤), 2012/06/11 20:58:43]

Yes, a future change list is actually changing it to "uint32_t". That's going to
make reasoning about the correctness a little easier, even if it doesn't really
change behavior.

I just pulled this change forward.

     ErrorCode err = evaluateSyscall(sysnum);
     int ret;
     switch (err) {
     case SB_INSPECT_ARG_1...SB_INSPECT_ARG_6:
       die("Not implemented");
     case SB_TRAP:
       ret = SECCOMP_RET_TRAP;
       break;
     case SB_ALLOWED:
       ret = SECCOMP_RET_ALLOW;
       break;
     default:
       if (err >= static_cast<ErrorCode>(1) &&
           err <= static_cast<ErrorCode>(4096)) {
         // We limit errno values to a reasonable range. In fact, the Linux ABI
         // doesn't support errno values outside of this range.
         ret = SECCOMP_RET_ERRNO + err;
       } else {
         die("Invalid ErrorCode reported by sandbox system call evaluator");
       }
       break;
     }
-    program.push_back((struct sock_filter)
-      BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, sysnum, 0, 1));
+    if (sysnum <= MAX_SYSCALL) {
+      // We compute the default behavior (e.g. fail open or fail closed) by
+      // calling the system call evaluator with a system call bigger than
+      // MAX_SYSCALL.
+      // In other words, the very last iteration in our loop becomes the
+      // fallback case and we don't need to do any comparisons.
+      program.push_back((struct sock_filter)
+        BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, sysnum, 0, 1));
+    }
     program.push_back((struct sock_filter)
       BPF_STMT(BPF_RET+BPF_K, ret));
   }
 
-  // Everything that isn't allowed is forbidden. Eventually, we would
-  // like to have a way to log forbidden calls, when in debug mode.
-  // TODO: raise a suitable SIGSYS signal
-  program.push_back((struct sock_filter)
-    BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_KILL));
+  // Make sure compilation resulted in BPF program that executes
+  // correctly. Otherwise, there is an internal error in our BPF compiler.
+  // There is really nothing the caller can do until the bug is fixed.
+  const char *err;

[Chris Evans, 2012/06/11 19:20:41]

Initialize to NULL for safety, since it's a pointer?

[Markus (顧孟勤), 2012/06/11 20:58:43]

Done.

+  if (!Verifier::verifyBPF(program, evaluators_, &err)) {

[Chris Evans, 2012/06/11 19:20:41]

I think we should only do this step under #ifdef DEBUG. That would likely be
enough to catch any problems pre-production, without any runtime cost in
production. Having just read verifier.cc, the runtime cost seems non-trivial.

[Markus (顧孟勤), 2012/06/11 20:58:43]

I am on the fence. Execution time various somewhere between 500us and 2ms,
depending on cache effects and on what else the machine is doing.

Does this matter in production? I don't know. On the one hand I like the extra
security of always running the verifier and I suspect that process start-up
already dwarfs the cost of sandbox start-up.

But on the other hand, I realize that people do strange things, and maybe we are
in fact starting hundred of sandbox'd processes per second -- in that case, an
extra millisecond is noticeable.

I put it behind an #ifdef for now. We might want to revisit in the future,
though.

+    die(err);
+  }
 
   // Install BPF filter program
   const struct sock_fprog prog = { program.size(), &program[0] };
   if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) ||
       prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
     goto filter_failed;
   }
 
   return;
 }
@@ skipping 25 matching lines @@
 
   ctx->uc_mcontext.gregs[REG_RESULT] = reinterpret_cast<greg_t>(rc);
   errno                              = old_errno;
   return;
 }
 
 
 bool Sandbox::suppressLogging_          = false;
 Sandbox::SandboxStatus Sandbox::status_ = STATUS_UNKNOWN;
 int    Sandbox::proc_fd_                = -1;
-std::vector<std::pair<Sandbox::EvaluateSyscall,
-                      Sandbox::EvaluateArguments> > Sandbox::evaluators_;
+Sandbox::Evaluators Sandbox::evaluators_;
 
 }  // namespace