sandbox: Add support for the new seccomp() system call in kernel 3.17.

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 6ecbca99a5f3c771a7dbeb8a0dd6050932536938..82bcca315564d5a25fa1c6dd6d6f32062330ae76 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
@@ -30,6 +30,7 @@
 #include "sandbox/linux/seccomp-bpf/syscall.h"
 #include "sandbox/linux/seccomp-bpf/syscall_iterator.h"
 #include "sandbox/linux/seccomp-bpf/verifier.h"
+#include "sandbox/linux/services/linux_syscalls.h"
 
 namespace sandbox {
 
@@ -378,6 +379,7 @@ bool SandboxBPF::KernelSupportSeccompBPF() {
              scoped_ptr<SandboxBPFPolicy>(new AllowAllPolicy()));
 }
 
+// static
 SandboxBPF::SandboxStatus SandboxBPF::SupportsSeccompSandbox(int proc_fd) {
   // It the sandbox is currently active, we clearly must have support for
   // sandboxing.
@@ -433,6 +435,22 @@ SandboxBPF::SandboxStatus SandboxBPF::SupportsSeccompSandbox(int proc_fd) {
   return status_;
 }
 
+// static
+SandboxBPF::SandboxStatus
+SandboxBPF::SupportsSeccompThreadFilterSynchronization() {
+  int rv = syscall(__NR_seccomp);

[jln (very slow on Chromium), 2014/08/20 21:34:20]

Do you want to add a comment saying that synchronization and the syscall have
been added together?

[Robert Sesek, 2014/08/21 16:50:18]

Done.

+
+  // The system call should have failed with EINVAL.
+  if (rv != -1)
+    return STATUS_UNKNOWN;

[jln (very slow on Chromium), 2014/08/20 21:34:20]

Maybe add a NOTREACHED() here?

[Robert Sesek, 2014/08/21 16:50:17]

Done.

+
+  if (errno == EINVAL)
+    return STATUS_AVAILABLE;
+
+  // errno is probably ENOSYS, indicating the system call is not available.
+  return STATUS_UNSUPPORTED;

[jln (very slow on Chromium), 2014/08/20 21:34:20]

DCHECK(ENOSYS == errno) maybe?

[Robert Sesek, 2014/08/21 16:50:18]

Done.

+}
+
 void SandboxBPF::set_proc_fd(int proc_fd) { proc_fd_ = proc_fd; }
 
 bool SandboxBPF::StartSandbox(SandboxThreadState thread_state) {
@@ -458,9 +476,16 @@ bool SandboxBPF::StartSandbox(SandboxThreadState thread_state) {
     // In the future, we might want to tighten this requirement.
   }
 
-  if (thread_state == PROCESS_SINGLE_THREADED && !IsSingleThreaded(proc_fd_)) {
-    SANDBOX_DIE("Cannot start sandbox, if process is already multi-threaded");
-    return false;
+  if (SupportsSeccompThreadFilterSynchronization() == STATUS_AVAILABLE) {
+    // The kernel supports the seccomp system call, so use that code path.
+    thread_state = PROCESS_MULTI_THREADED;

[jln (very slow on Chromium), 2014/08/20 21:34:20]

I would still want us to crash if the caller passes PROCESS_SINGLE_THREADED from
a multi-threaded process.

It may be best to save the result of
SupportsSeccompThreadFilterSynchronization() in a auxiliary const bool instead.

Then decide whether to call the old or new interface based on that bool.

[Robert Sesek, 2014/08/21 16:50:18]

I actually think SandboxThreadState should just go away, since we have an easy
way to test whether or not a system supports tsync. Back when we thought the
interface was going to be prctl(), this wasn't the case.

[jln (very slow on Chromium), 2014/08/21 18:47:09]

At a high level, paranoia dictates to carefully assert that the situation is
exactly as the caller think it is. This is especially true while TSYNC is not a
strict Chrome run-time requirement. I can imagine this changing in a year or so.
In more details:

- I think the caller should always know whether it is monothreaded or
multithreaded. And we should enforce this with a sanity check.
a. Callers that use PROCESS_SINGLE_THREADED are guaranteed to lead to a working
sandbox on every kernel, not just the ones with TSYNC. This is especially
important / useful on Linux (presumably we'll have TSYNC everywhere on Chrome
OS).
b. More problems can exist with multi-threaded initialization. Other threads
could race to open new resources that are not accounted for (moving "open
resources" sanity checks to after the sandbox is engaged is tricky in some
cases, even though we already to it for most file descriptors).

- Thread detection is not very reliable. It would be easy to make an error where
thread detection is not working at all, except on bots / machines with TSYNC
support.

For instance, for security reasons, we only have /proc available after the
setuid sandbox is engaged in DEBUG mode.

In this case, we would be in a situation where this code would detect multiple
threads but not fail because of TSYNC on our bots, but wouldn't detect existing
threads in the field where TSYNC is not available.

- We are dicussing the possibiliy of convoluted CFI + seccomp sandboxes where
one thread would be "trusted" and not under seccomp-bpf. To support this, we may
want to add a new PROCESS_UNTRUSTED_THREAD or something along these lines to
engage seccomp-bpf on one thread in a multi-threaded process.

+  } else {
+    // The kernel doesn't support the seccomp system call, so the process
+    // must not be multi-threaded.
+    if (!IsSingleThreaded(proc_fd_) || thread_state == PROCESS_MULTI_THREADED) {
+      SANDBOX_DIE("Cannot start sandbox, if process is already multi-threaded");
+      return false;
+    }
   }
 
   // We no longer need access to any files in /proc. We want to do this
@@ -527,28 +552,23 @@ void SandboxBPF::InstallFilter(SandboxThreadState thread_state) {
   conds_ = NULL;
   policy_.reset();
 
-  // Install BPF filter program
-  if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
-    SANDBOX_DIE(quiet_ ? NULL : "Kernel refuses to enable no-new-privs");
-  } else {
-    if (prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
-      SANDBOX_DIE(quiet_ ? NULL : "Kernel refuses to turn on BPF filters");
-    }
-  }
-
-  // TODO(rsesek): Always try to engage the sandbox with the
-  // PROCESS_MULTI_THREADED path first, and if that fails, assert that the
-  // process IsSingleThreaded() or SANDBOX_DIE.
-
+  // Install BPF filter program. If the thread state indicates multi-threading

[jln (very slow on Chromium), 2014/08/20 21:34:20]

Should we still try to enable NO_NEW_PRIV via prctl first? This granularity
could be useful in understanding failures (also it makes the code below a bit
less nested).

[Robert Sesek, 2014/08/21 16:50:18]

Done.

+  // support, then the kernel hass the seccomp system call. Otherwise, fall
+  // back on prctl, which requires the process to be single-threaded.
   if (thread_state == PROCESS_MULTI_THREADED) {
-    // TODO(rsesek): Move these to a more reasonable place once the kernel
-    // patch has landed upstream and these values are formalized.
-    #define PR_SECCOMP_EXT 41
-    #define SECCOMP_EXT_ACT 1
-    #define SECCOMP_EXT_ACT_TSYNC 1
-    if (prctl(PR_SECCOMP_EXT, SECCOMP_EXT_ACT, SECCOMP_EXT_ACT_TSYNC, 0, 0)) {
-      SANDBOX_DIE(quiet_ ? NULL : "Kernel refuses to synchronize threadgroup "
-                                  "BPF filters.");
+    int rv = syscall(__NR_seccomp, SECCOMP_SET_MODE_FILTER,
+        SECCOMP_FILTER_FLAG_TSYNC, (const char*)&prog);
+    if (rv) {
+      SANDBOX_DIE(quiet_ ? NULL :
+          "Kernel refuses to turn on and synchronize threads for BPF filters");
+    }
+  } else {
+    if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
+      SANDBOX_DIE(quiet_ ? NULL : "Kernel refuses to enable no-new-privs");
+    } else {
+      if (prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
+        SANDBOX_DIE(quiet_ ? NULL : "Kernel refuses to turn on BPF filters");
+      }
     }
   }