Avoid all library calls (including calls to destructors) after enabling the sandbox.

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.
@@ skipping 194 matching lines @@
       sigprocmask(SIG_UNBLOCK, &mask, NULL)) {
     goto filter_failed;
   }
 
   // We can't handle stacked evaluators, yet. We'll get there eventually
   // though. Hang tight.
   if (evaluators_.size() != 1) {
     die("Not implemented");
   }
 
+  // Assemble the BPF filter program.
+  Program *program = new Program();

[jln (very slow on Chromium), 2012/06/12 19:16:23]

I would love if we didn't have to keep track (and delete() manually) a pointer.
Obviously we cannot used scoped_ptr here, but I think we could have "Program
program;" as before, but in an inside block {} that closes before we install the
policy (and after we copy the program to the stack obviously).

This would rely on the same C++ guarantee as the one we have for Errorcode in
the for() loop below, etc.

[Markus (顧孟勤), 2012/06/12 19:35:05]

This is tricky, and I don't think I can do it without writing even crazier code.

The problem is that we eventually need to allocate the "bpf" array on the stack.
And when we create it, we still need access to "program".

This results in two mutually exclusive requirements for block scoping.

So, I think, explicit management of the heap allocated object is required. Feel
free to correct me, if you can see an alternative that eludes me.

[jln (very slow on Chromium), 2012/06/12 20:22:51]

You're right, this becomes even more confusing.

+  if (!program) {
+    die("Out of memory");
+  }
+
   // If the architecture doesn't match SECCOMP_ARCH, disallow the
   // system call.
-  std::vector<struct sock_filter> program;
-  program.push_back((struct sock_filter)
+  program->push_back((struct sock_filter)
     BPF_STMT(BPF_LD+BPF_W+BPF_ABS, offsetof(struct arch_seccomp_data, arch)));
-  program.push_back((struct sock_filter)
+  program->push_back((struct sock_filter)
     BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, SECCOMP_ARCH, 1, 0));
 
   // TODO: Instead of killing outright, we should raise a SIGSYS and
   //       report a useful error message. SIGKILL cannot be trapped by the
   //       debugger and essentially makes the program fail in a way that is
   //       almost impossible to debug.
-  program.push_back((struct sock_filter)
+  program->push_back((struct sock_filter)
     BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_KILL));
 
   // Grab the system call number, so that we can implement jump tables.
-  program.push_back((struct sock_filter)
+  program->push_back((struct sock_filter)
     BPF_STMT(BPF_LD+BPF_W+BPF_ABS, offsetof(struct arch_seccomp_data, nr)));
 
   // On Intel architectures, verify that system call numbers are in the
   // expected number range. The older i386 and x86-64 APIs clear bit 30
   // on all system calls. The newer x86-32 API always sets bit 30.
 #if defined(__i386__) || defined(__x86_64__)
 #if defined(__x86_64__) && defined(__ILP32__)
-  program.push_back((struct sock_filter)
+  program->push_back((struct sock_filter)
     BPF_JUMP(BPF_JMP+BPF_JSET+BPF_K, 0x40000000, 1, 0));
 #else
-  program.push_back((struct sock_filter)
+  program->push_back((struct sock_filter)
     BPF_JUMP(BPF_JMP+BPF_JSET+BPF_K, 0x40000000, 0, 1));
 #endif
   // TODO: raise a suitable SIGSYS signal
-  program.push_back((struct sock_filter)
+  program->push_back((struct sock_filter)
     BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_KILL));
 #endif
 
   // 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
@@ skipping 21 matching lines @@
         die("Invalid ErrorCode reported by sandbox system call evaluator");
       }
       break;
     }
     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)
+      program->push_back((struct sock_filter)
         BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, sysnum, 0, 1));
     }
-    program.push_back((struct sock_filter)
+    program->push_back((struct sock_filter)
       BPF_STMT(BPF_RET+BPF_K, ret));
   }
 
   // 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;
-  if (!Verifier::verifyBPF(program, evaluators_, &err)) {
+  if (!Verifier::verifyBPF(*program, evaluators_, &err)) {
     die(err);
   }
 
+  // We want to be very careful in not imposing any requirements on the
+  // policies that are set with setSandboxPolicy(). This means, as soon as
+  // the sandbox is active, we shouldn't be relying on libraries that could
+  // be making system calls. This, for example, means we should avoid
+  // using the heap and we should avoid using STL functions.
+  // Temporarily copy the contents of the "program" vector into a
+  // stack-allocated array.

[jln (very slow on Chromium), 2012/06/12 19:16:23]

Please explain further that you're trying to make sure that the program vector
gets de-allocated from the heap before installing the filter. As-is, I don't
think it'll be clear to reviewers.

[Markus (顧孟勤), 2012/06/12 19:35:05]

Sure, I can add a little more to the comment :-) This is subtle code, and more
comments are always helpful.

+  struct sock_filter bpf[program->size()];
+  const struct sock_fprog prog = { program->size(), bpf };
+  memcpy(bpf, &(*program)[0], sizeof(bpf));

[jln (very slow on Chromium), 2012/06/12 19:16:23]

Would a static_cast of program be cleaner here ?

[Markus (顧孟勤), 2012/06/12 19:35:05]

I am not convinced a cast would actually do the right thing. I believe we have
to make sure we execute the vector::operator[](). Feel free to correct me, if
you can find a something in the STL documentation that says otherwise.

[jln (very slow on Chromium), 2012/06/12 20:22:51]

The correct solution would be to use the new vector::data(). I presume all of
our toolchains already support it, but feel free to stick to the current tired
method.

+  delete program;
+
   // 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;
 }
 
 void Sandbox::sigSys(int nr, siginfo_t *info, void *void_context) {
   if (nr != SIGSYS || info->si_code != SYS_SECCOMP || !void_context) {
@@ skipping 25 matching lines @@
   return;
 }
 
 
 bool Sandbox::suppressLogging_          = false;
 Sandbox::SandboxStatus Sandbox::status_ = STATUS_UNKNOWN;
 int    Sandbox::proc_fd_                = -1;
 Sandbox::Evaluators Sandbox::evaluators_;
 
 }  // namespace