Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(888)

Unified Diff: sandbox/linux/seccomp-bpf/sandbox_bpf.cc

Issue 494743003: sandbox: Add support for the new seccomp() system call in kernel 3.17. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Assert thread state Created 6 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..f6f1e7ef62d15d38b70e3e5404e2bd3a24150843 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,27 @@ SandboxBPF::SandboxStatus SandboxBPF::SupportsSeccompSandbox(int proc_fd) {
return status_;
}
+// static
+SandboxBPF::SandboxStatus
+SandboxBPF::SupportsSeccompThreadFilterSynchronization() {
+ // Applying NO_NEW_PRIVS, a BPF filter, and synchronizing the filter across
+ // the thread group are all handled atomically by this syscall.
+ int rv = syscall(__NR_seccomp);
jln (very slow on Chromium) 2014/08/21 21:04:42 Let's pass the NULL parameters explicitly. I'm not
Robert Sesek 2014/08/25 17:17:26 Done.
Robert Sesek 2014/08/25 17:49:01 Actually not done, I couldn't get this to work on
+
+ // The system call should have failed with EINVAL.
+ if (rv != -1) {
+ NOTREACHED();
+ return STATUS_UNKNOWN;
+ }
+
+ if (errno == EINVAL)
+ return STATUS_AVAILABLE;
+
+ // errno is probably ENOSYS, indicating the system call is not available.
+ DPCHECK(errno == ENOSYS);
+ return STATUS_UNSUPPORTED;
+}
+
void SandboxBPF::set_proc_fd(int proc_fd) { proc_fd_ = proc_fd; }
bool SandboxBPF::StartSandbox(SandboxThreadState thread_state) {
@@ -458,8 +481,24 @@ 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");
+ if (thread_state == PROCESS_SINGLE_THREADED) {
+ if (!IsSingleThreaded(proc_fd_)) {
+ SANDBOX_DIE("Cannot start sandbox; process is already multi-threaded");
+ return false;
+ }
+ } else if (thread_state == PROCESS_MULTI_THREADED) {
+ if (IsSingleThreaded(proc_fd_)) {
+ SANDBOX_DIE("Cannot start sandbox; "
jln (very slow on Chromium) 2014/08/21 21:04:41 This is fine, but I feel much less strongly about
Robert Sesek 2014/08/25 17:17:26 Done.
+ "process is not multi-threaded when reported as such");
+ return false;
+ }
+ if (SupportsSeccompThreadFilterSynchronization() != STATUS_AVAILABLE) {
+ SANDBOX_DIE("Cannot start sandbox; kernel does not support synchronizing "
+ "filters for a threadgroup");
+ return false;
+ }
+ } else {
+ SANDBOX_DIE("NOTREACHED()");
return false;
}
@@ -527,28 +566,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
+ // 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) {
jln (very slow on Chromium) 2014/08/21 21:04:42 It's a very nice safeguard to always use TSYNC and
Robert Sesek 2014/08/25 17:17:26 Done.
- // 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, reinterpret_cast<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_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
+ SANDBOX_DIE(quiet_ ? NULL : "Kernel refuses to turn on BPF filters");
}
}

Powered by Google App Engine
This is Rietveld 408576698