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

sandbox/linux/seccomp-bpf/sandbox_bpf.cc

Index: sandbox/linux/seccomp-bpf/sandbox_bpf.cc
diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
index f2e5bbdf1b08125678b8a3206b32ae80c2865456..c81c7472352ce4087a6ccabfc523e6501ef4481b 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
@@ -212,23 +212,28 @@ void Sandbox::installFilter() {
     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
@@ -236,14 +241,14 @@ void Sandbox::installFilter() {
   // 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
 
@@ -285,10 +290,10 @@ void Sandbox::installFilter() {
       // 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));
   }
 
@@ -296,12 +301,23 @@ void Sandbox::installFilter() {
   // 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;