NaCl: enable a real seccomp-bpf sandbox.

chrome/nacl/nacl_sandbox_linux.cc

Index: chrome/nacl/nacl_sandbox_linux.cc
diff --git a/chrome/nacl/nacl_sandbox_linux.cc b/chrome/nacl/nacl_sandbox_linux.cc
index 19585eaf26d693bbd8daeb85194820ba90774d6f..1d48f48e4ee55dede4850970658c92f94d3c7e78 100644
--- a/chrome/nacl/nacl_sandbox_linux.cc
+++ b/chrome/nacl/nacl_sandbox_linux.cc
@@ -19,26 +19,91 @@ using playground2::Sandbox;
 
 namespace {
 
-// This policy does very little:
-// - Any invalid system call for the current architecture is handled by
-//   the baseline policy.
-// - ptrace() is denied.
-// - Anything else is allowed.
-// Note that the seccomp-bpf sandbox always prevents cross-architecture
-// system calls (on x86, long/compatibility/x32).
-// So even this trivial policy has a security benefit.
+// On arm and x86_64, System V shared memory calls have each their own system

[Mark Seaborn, 2013/07/23 17:13:03]

Nit: 'arm' -> 'ARM'

[jln (very slow on Chromium), 2013/07/23 18:56:26]

Done.

+// call, while on i386 they are multiplexed.
+#if defined(__x86_64__) || defined(__arm__)
+bool IsSystemVSharedMemory(int sysno) {
+  switch (sysno) {
+    case __NR_shmat:
+    case __NR_shmctl:
+    case __NR_shmdt:
+    case __NR_shmget:
+      return true;
+    default:
+      return false;
+  }
+}
+#endif
+
+#if defined(__i386__)
+// Big system V multiplexing system call.
+bool IsSystemVIpc(int sysno) {
+  switch (sysno) {
+    case __NR_ipc:
+      return true;
+    default:
+      return false;
+  }
+}
+#endif
+
 ErrorCode NaClBpfSandboxPolicy(
-    playground2::Sandbox* sb, int sysnum, void* aux) {
+    playground2::Sandbox* sb, int sysno, void* aux) {
   const playground2::BpfSandboxPolicyCallback baseline_policy =
       content::GetBpfSandboxBaselinePolicy();
-  if (!playground2::Sandbox::IsValidSyscallNumber(sysnum)) {
-    return baseline_policy.Run(sb, sysnum, aux);
-  }
-  switch (sysnum) {
+  switch (sysno) {
+    // TODO(jln): NaClGdbDebugStubTest.Breakpoint needs the following 4 system

[Mark Seaborn, 2013/07/23 17:13:03]

Can you change this to: "NaCl's GDB debug stub uses the following socket system
calls".  Saying "the following 4" is very likely to get out of date as the code
is changed.

[jln (very slow on Chromium), 2013/07/23 18:56:26]

Done.

+    // calls, see if it can be restricted a bit.
+#if defined(__x86_64__) || defined(__arm__)
+    // transport_common.cc needs this.
+    case __NR_accept:
+    case __NR_setsockopt:
+#elif defined(__i386__)
+    case __NR_socketcall:
+#endif
+    // trusted/service_runtime/linux/thread_suspension.cc needs this.

[Mark Seaborn, 2013/07/23 17:13:03]

Make this "trusted/service_runtime/linux/thread_suspension.c needs sigwait() and
is used by NaCl's GDB debug stub".  (Note that it's ".c" not ".cc".)

[jln (very slow on Chromium), 2013/07/23 18:56:26]

Done.

[jln (very slow on Chromium), 2013/07/23 18:56:26]

Done.

+    case __NR_rt_sigtimedwait:
+#if defined(__i386__)
+    // Needed on i386 to set-up the custom segments.
+    case __NR_modify_ldt:
+#endif
+    // NaClAddrSpaceBeforeAlloc needs this.

[Mark Seaborn, 2013/07/23 17:13:03]

"this" -> "prlimit64"

[jln (very slow on Chromium), 2013/07/23 18:56:26]

Done.

[jln (very slow on Chromium), 2013/07/23 18:56:26]

Done.

+    case __NR_prlimit64:
+    // NaCl uses custom signal stacks.
+    case __NR_sigaltstack:
+    // Below is fairly similar to the policy for a Chromium renderer.
+    // TODO(jln): restrict clone(), ioctl() and prctl().
+    case __NR_ioctl:
+#if defined(__i386__) || defined(__x86_64__)
+    case __NR_getrlimit:
+#endif
+#if defined(__i386__) || defined(__arm__)
+    case __NR_ugetrlimit:
+#endif
+    case __NR_pread64:
+    case __NR_pwrite64:
+    case __NR_sched_get_priority_max:
+    case __NR_sched_get_priority_min:
+    case __NR_sched_getaffinity:
+    case __NR_sched_getparam:
+    case __NR_sched_getscheduler:
+    case __NR_sched_setscheduler:
+    case __NR_setpriority:
+    case __NR_sysinfo:
+    case __NR_uname:
+      return ErrorCode(ErrorCode::ERR_ALLOWED);
     case __NR_ptrace:
       return ErrorCode(EPERM);
     default:
-      return ErrorCode(ErrorCode::ERR_ALLOWED);
+      // TODO(jln): look into getting rid of System V shared memory.
+#if defined(__x86_64__) || defined(__arm__)
+      if (IsSystemVSharedMemory(sysno))
+        return ErrorCode(ErrorCode::ERR_ALLOWED);
+#elif defined(__i386__)
+      if (IsSystemVIpc(sysno))
+        return ErrorCode(ErrorCode::ERR_ALLOWED);
+#endif
+      return baseline_policy.Run(sb, sysno, aux);
   }
   NOTREACHED();
   // GCC wants this.