SECCOMP-BPF: Refactor the BPF sandbox API to use fewer "static" fields and methods.

content/common/sandbox_seccomp_bpf_linux.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 <asm/unistd.h>
 #include <dlfcn.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/audit.h>
 #include <linux/filter.h>
 #include <signal.h>
 #include <string.h>
+#include <sys/ioctl.h>
 #include <sys/prctl.h>
+#include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <ucontext.h>
 #include <unistd.h>
 
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 12 matching lines @@
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
 #include "sandbox/linux/services/linux_syscalls.h"
 
 using playground2::arch_seccomp_data;
 using playground2::ErrorCode;
 using playground2::Sandbox;
 using sandbox::BrokerProcess;
 
 namespace {
 
-void StartSandboxWithPolicy(Sandbox::EvaluateSyscall syscall_policy,
+void StartSandboxWithPolicy(Sandbox *sandbox,
+                            Sandbox::EvaluateSyscall syscall_policy,
                             BrokerProcess* broker_process);
 
 inline bool RunningOnASAN() {
 #if defined(ADDRESS_SANITIZER)
   return true;
 #else
   return false;
 #endif
 }
 
@@ skipping 1166 matching lines @@
 #if defined(__arm__)
       IsArmPciConfig(sysno) ||
 #endif
       IsTimer(sysno)) {
     return true;
   } else {
     return false;
   }
 }
 
-ErrorCode BaselinePolicy(int sysno) {
+ErrorCode BaselinePolicy(Sandbox *sandbox, int sysno) {
 #if defined(__x86_64__) || defined(__arm__)
   if (sysno == __NR_socketpair) {
     // Only allow AF_UNIX, PF_UNIX. Crash if anything else is seen.
     COMPILE_ASSERT(AF_UNIX == PF_UNIX, af_unix_pf_unix_different);
-    return Sandbox::Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL, AF_UNIX,
+    return sandbox->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL, AF_UNIX,
                          ErrorCode(ErrorCode::ERR_ALLOWED),
-                         Sandbox::Trap(CrashSIGSYS_Handler, NULL));
+                         sandbox->Trap(CrashSIGSYS_Handler, NULL));
   }
 #endif
   if (IsBaselinePolicyAllowed(sysno)) {
     return ErrorCode(ErrorCode::ERR_ALLOWED);
   }
 
 #if defined(__i386__)
   // socketcall(2) should be tightened.
   if (IsSocketCall(sysno)) {
     return ErrorCode(ErrorCode::ERR_ALLOWED);
   }
 #endif
 
   // TODO(jln): some system calls in those sets are not supposed to
   // return ENOENT. Return the appropriate error.
   if (IsFileSystem(sysno) || IsCurrentDirectory(sysno)) {
     return ErrorCode(ENOENT);
   }
 
   if (IsUmask(sysno) || IsDeniedFileSystemAccessViaFd(sysno) ||
       IsDeniedGetOrModifySocket(sysno)) {
     return ErrorCode(EPERM);
   }
 
   if (IsBaselinePolicyWatched(sysno)) {
     // Previously unseen syscalls. TODO(jln): some of these should
     // be denied gracefully right away.
-    return Sandbox::Trap(CrashSIGSYS_Handler, NULL);
+    return sandbox->Trap(CrashSIGSYS_Handler, NULL);
   }
   // In any other case crash the program with our SIGSYS handler
-  return Sandbox::Trap(CrashSIGSYS_Handler, NULL);
+  return sandbox->Trap(CrashSIGSYS_Handler, NULL);
 }
 
 // x86_64/i386 for now. Needs to be adapted and tested for ARM.
-ErrorCode GpuProcessPolicy(int sysno, void *broker_process) {
+ErrorCode GpuProcessPolicy(Sandbox *sandbox, int sysno,
+                           void *broker_process) {
   switch(sysno) {
     case __NR_ioctl:
 #if defined(ADDRESS_SANITIZER)
     // Allow to call sched_getaffinity under AddressSanitizer.
     case __NR_sched_getaffinity:
 #endif
       return ErrorCode(ErrorCode::ERR_ALLOWED);
     case __NR_open:
     case __NR_openat:
-        return Sandbox::Trap(GpuOpenSIGSYS_Handler, broker_process);
+        return sandbox->Trap(GpuOpenSIGSYS_Handler, broker_process);
     default:
       if (IsEventFd(sysno))
         return ErrorCode(ErrorCode::ERR_ALLOWED);
 
       // Default on the baseline policy.
-      return BaselinePolicy(sysno);
+      return BaselinePolicy(sandbox, sysno);
   }
 }
 
 // x86_64/i386 for now. Needs to be adapted and tested for ARM.
 // A GPU broker policy is the same as a GPU policy with open and
 // openat allowed.
-ErrorCode GpuBrokerProcessPolicy(int sysno, void*) {
+ErrorCode GpuBrokerProcessPolicy(Sandbox *sandbox, int sysno, void *aux) {
+  // "aux" would typically be NULL, when called from
+  // "EnableGpuBrokerPolicyCallBack"
   switch(sysno) {
     case __NR_open:
     case __NR_openat:
       return ErrorCode(ErrorCode::ERR_ALLOWED);
     default:
-      return GpuProcessPolicy(sysno, NULL);
+      return GpuProcessPolicy(sandbox, sysno, aux);
   }
 }
 
 // Allow clone for threads, crash if anything else is attempted.
 // Don't restrict on ASAN.
-ErrorCode RestrictCloneToThreads() {
+ErrorCode RestrictCloneToThreads(Sandbox *sandbox) {
   // Glibc's pthread.
   if (!RunningOnASAN()) {
-    return Sandbox::Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
+    return sandbox->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
         CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND |
         CLONE_THREAD | CLONE_SYSVSEM | CLONE_SETTLS |
         CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID,
         ErrorCode(ErrorCode::ERR_ALLOWED),
-        Sandbox::Trap(ReportCloneFailure, NULL));
+        sandbox->Trap(ReportCloneFailure, NULL));
   } else {
     return ErrorCode(ErrorCode::ERR_ALLOWED);
   }
 }
 
-ErrorCode RestrictPrctl() {
+ErrorCode RestrictPrctl(Sandbox *sandbox) {
   // Allow PR_SET_NAME, PR_SET_DUMPABLE, PR_GET_DUMPABLE. Will need to add
   // seccomp compositing in the future.
   // PR_SET_PTRACER is used by breakpad but not needed anymore.
-  return Sandbox::Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
+  return sandbox->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
                        PR_SET_NAME, ErrorCode(ErrorCode::ERR_ALLOWED),
-         Sandbox::Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
+         sandbox->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
                        PR_SET_DUMPABLE, ErrorCode(ErrorCode::ERR_ALLOWED),
-         Sandbox::Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
+         sandbox->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
                        PR_GET_DUMPABLE, ErrorCode(ErrorCode::ERR_ALLOWED),
-         Sandbox::Trap(ReportPrctlFailure, NULL))));
+         sandbox->Trap(ReportPrctlFailure, NULL))));
 }
 
-ErrorCode RestrictIoctl() {
+ErrorCode RestrictIoctl(Sandbox *sandbox) {
   // Allow TCGETS and FIONREAD, trap to ReportIoctlFailure otherwise.
-  return Sandbox::Cond(1, ErrorCode::TP_64BIT, ErrorCode::OP_EQUAL, TCGETS,
+  return sandbox->Cond(1, ErrorCode::TP_64BIT, ErrorCode::OP_EQUAL, TCGETS,
                        ErrorCode(ErrorCode::ERR_ALLOWED),
-         Sandbox::Cond(1, ErrorCode::TP_64BIT, ErrorCode::OP_EQUAL, FIONREAD,
+         sandbox->Cond(1, ErrorCode::TP_64BIT, ErrorCode::OP_EQUAL, FIONREAD,
                        ErrorCode(ErrorCode::ERR_ALLOWED),
-                       Sandbox::Trap(ReportIoctlFailure, NULL)));
+                       sandbox->Trap(ReportIoctlFailure, NULL)));
 }
 
-ErrorCode RendererOrWorkerProcessPolicy(int sysno, void *) {
+ErrorCode RendererOrWorkerProcessPolicy(Sandbox *sandbox, int sysno, void *) {
   switch (sysno) {
     case __NR_clone:
-      return RestrictCloneToThreads();
+      return RestrictCloneToThreads(sandbox);
     case __NR_ioctl:
       // Restrict IOCTL on x86_64 on Linux but not Chrome OS.
       if (IsArchitectureX86_64() && !IsChromeOS()) {
-        return RestrictIoctl();
+        return RestrictIoctl(sandbox);
       } else {
         return ErrorCode(ErrorCode::ERR_ALLOWED);
       }
     case __NR_prctl:
-      return RestrictPrctl();
+      return RestrictPrctl(sandbox);
     // Allow the system calls below.
     case __NR_fdatasync:
     case __NR_fsync:
 #if defined(__i386__) || defined(__x86_64__)
     case __NR_getrlimit:
 #endif
     case __NR_mremap:   // See crbug.com/149834.
     case __NR_pread64:
     case __NR_pwrite64:
 #if defined(ADDRESS_SANITIZER)
@@ skipping 17 matching lines @@
 #if defined(__x86_64__) || defined(__arm__)
       if (IsSystemVSharedMemory(sysno))
         return ErrorCode(ErrorCode::ERR_ALLOWED);
 #endif
 #if defined(__i386__)
       if (IsSystemVIpc(sysno))
         return ErrorCode(ErrorCode::ERR_ALLOWED);
 #endif
 
       // Default on the baseline policy.
-      return BaselinePolicy(sysno);
+      return BaselinePolicy(sandbox, sysno);
   }
 }
 
-ErrorCode FlashProcessPolicy(int sysno, void *) {
+ErrorCode FlashProcessPolicy(Sandbox *sandbox, int sysno, void *) {
   switch (sysno) {
     case __NR_sched_getaffinity:
     case __NR_sched_setscheduler:
     case __NR_times:
       return ErrorCode(ErrorCode::ERR_ALLOWED);
     case __NR_ioctl:
       return ErrorCode(ENOTTY);  // Flash Access.
     default:
       // These need further tightening.
 #if defined(__x86_64__) || defined(__arm__)
       if (IsSystemVSharedMemory(sysno))
         return ErrorCode(ErrorCode::ERR_ALLOWED);
 #endif
 #if defined(__i386__)
       if (IsSystemVIpc(sysno))
         return ErrorCode(ErrorCode::ERR_ALLOWED);
 #endif
 
       // Default on the baseline policy.
-      return BaselinePolicy(sysno);
+      return BaselinePolicy(sandbox, sysno);
   }
 }
 
-ErrorCode BlacklistDebugAndNumaPolicy(int sysno, void *) {
+ErrorCode BlacklistDebugAndNumaPolicy(Sandbox *sandbox, int sysno, void *) {
   if (!Sandbox::IsValidSyscallNumber(sysno)) {
     // TODO(jln) we should not have to do that in a trivial policy.
     return ErrorCode(ENOSYS);
   }
 
   if (IsDebug(sysno) || IsNuma(sysno))
-    return Sandbox::Trap(CrashSIGSYS_Handler, NULL);
+    return sandbox->Trap(CrashSIGSYS_Handler, NULL);
 
   return ErrorCode(ErrorCode::ERR_ALLOWED);
 }
 
 // Allow all syscalls.
 // This will still deny x32 or IA32 calls in 64 bits mode or
 // 64 bits system calls in compatibility mode.
-ErrorCode AllowAllPolicy(int sysno, void *) {
+ErrorCode AllowAllPolicy(Sandbox *, int sysno, void *) {
   if (!Sandbox::IsValidSyscallNumber(sysno)) {
     // TODO(jln) we should not have to do that in a trivial policy.
     return ErrorCode(ENOSYS);
   } else {
     return ErrorCode(ErrorCode::ERR_ALLOWED);
   }
 }
 
 bool EnableGpuBrokerPolicyCallBack() {
-  StartSandboxWithPolicy(GpuBrokerProcessPolicy, NULL);
+  Sandbox sandbox;
+  StartSandboxWithPolicy(&sandbox, GpuBrokerProcessPolicy, NULL);
   return true;
 }
 
 // Start a broker process to handle open() inside the sandbox.
 void InitGpuBrokerProcess(BrokerProcess** broker_process) {
   static const char kDriRcPath[] = "/etc/drirc";
   static const char kDriCard0Path[] = "/dev/dri/card0";
 
   CHECK(broker_process);
   CHECK(*broker_process == NULL);
@@ skipping 63 matching lines @@
   if (process_type == switches::kUtilityProcess) {
     return BlacklistDebugAndNumaPolicy;
   }
 
   NOTREACHED();
   // This will be our default if we need one.
   return AllowAllPolicy;
 }
 
 // broker_process can be NULL if there is no need for one.
-void StartSandboxWithPolicy(Sandbox::EvaluateSyscall syscall_policy,
+void StartSandboxWithPolicy(Sandbox *sandbox,
+                            Sandbox::EvaluateSyscall syscall_policy,
                             BrokerProcess* broker_process) {
-
-  Sandbox::SetSandboxPolicy(syscall_policy, broker_process);
-  Sandbox::StartSandbox();
+  sandbox->SetSandboxPolicy(syscall_policy, broker_process);

[jln (very slow on Chromium), 2013/02/20 01:35:49]

It looks like this is where the Sandbox object should be  created instead ?
(Please add a comment about the life time of this object as mentioned below).

+  sandbox->StartSandbox();
 }
 
 // Initialize the seccomp-bpf sandbox.
 bool StartBpfSandbox(const CommandLine& command_line,
                      const std::string& process_type) {
   Sandbox::EvaluateSyscall syscall_policy =
       GetProcessSyscallPolicy(command_line, process_type);
 
   BrokerProcess* broker_process = NULL;
   // Warm up resources needed by the policy we're about to enable and
   // eventually start a broker process.
   WarmupPolicy(syscall_policy, &broker_process);
 
-  StartSandboxWithPolicy(syscall_policy, broker_process);
+  Sandbox sandbox;

[jln (very slow on Chromium), 2013/02/20 01:35:49]

This looks very suspicious to have a sandbox object that is this short lived.

The main comment belongs in sandbox_bpf.h, but another comment here would be
helpful.

+  StartSandboxWithPolicy(&sandbox, syscall_policy, broker_process);
 
   return true;
 }
 
 }  // namespace
 
 #endif  // SECCOMP_BPF_SANDBOX
 
 namespace content {
 
@@ skipping 48 matching lines @@
     // should enable it, enable it or die.
     bool started_sandbox = StartBpfSandbox(command_line, process_type);
     CHECK(started_sandbox);
     return true;
   }
 #endif
   return false;
 }
 
 }  // namespace content