Linux sandbox: start adding syscall wrappers

Add some syscall wrappers instead of calling syscall() inline.
This adds some type safety and will make it easier to switch the
underlying syscall implementation.

This fixes a few issues with SYS_CLONE being called with the wrong number of
parameters.

BUG=435194

Review URL: https://codereview.chromium.org/684993005

Cr-Commit-Position: refs/heads/master@{#305051}

15 files changed, 114 insertions, 30 deletions

diff --git a/sandbox/linux/BUILD.gn b/sandbox/linux/BUILD.gn
index ce699c4..d17e25b 100644
--- a/sandbox/linux/BUILD.gn
+++ b/sandbox/linux/BUILD.gn
@@ -222,6 +222,8 @@ component("sandbox_services") {
     "services/init_process_reaper.h",
     "services/scoped_process.cc",
     "services/scoped_process.h",
+    "services/syscall_wrappers.cc",
+    "services/syscall_wrappers.h",
     "services/thread_helpers.cc",
     "services/thread_helpers.h",
     "services/yama.h",
diff --git a/sandbox/linux/bpf_dsl/bpf_dsl_more_unittest.cc b/sandbox/linux/bpf_dsl/bpf_dsl_more_unittest.cc
index b3d9126..66669e7 100644
--- a/sandbox/linux/bpf_dsl/bpf_dsl_more_unittest.cc
+++ b/sandbox/linux/bpf_dsl/bpf_dsl_more_unittest.cc
@@ -41,6 +41,7 @@
 #include "sandbox/linux/seccomp-bpf/syscall.h"
 #include "sandbox/linux/seccomp-bpf/trap.h"
 #include "sandbox/linux/services/linux_syscalls.h"
+#include "sandbox/linux/services/syscall_wrappers.h"
 #include "sandbox/linux/syscall_broker/broker_process.h"
 #include "sandbox/linux/tests/scoped_temporary_file.h"
 #include "sandbox/linux/tests/unit_tests.h"
@@ -203,7 +204,7 @@ class WhitelistGetpidPolicy : public Policy {
 BPF_TEST_C(SandboxBPF, ApplyBasicWhitelistPolicy, WhitelistGetpidPolicy) {
   // getpid() should be allowed
   errno = 0;
-  BPF_ASSERT(syscall(__NR_getpid) > 0);
+  BPF_ASSERT(sys_getpid() > 0);
   BPF_ASSERT(errno == 0);
 
   // getpgid() should be denied
@@ -246,7 +247,7 @@ BPF_TEST(SandboxBPF,
          int /* (*BPF_AUX) */) {
   // getpid() should work properly
   errno = 0;
-  BPF_ASSERT(syscall(__NR_getpid) > 0);
+  BPF_ASSERT(sys_getpid() > 0);
   BPF_ASSERT(errno == 0);
 
   // Our Auxiliary Data, should be reset by the signal handler
@@ -512,7 +513,7 @@ intptr_t CountSyscalls(const struct arch_seccomp_data& args, void* aux) {
 
   // Verify that within the callback function all filtering is temporarily
   // disabled.
-  BPF_ASSERT(syscall(__NR_getpid) > 1);
+  BPF_ASSERT(sys_getpid() > 1);
 
   // Verify that we can now call the underlying system call without causing
   // infinite recursion.
@@ -549,7 +550,7 @@ class GreyListedPolicy : public Policy {
 };
 
 BPF_TEST(SandboxBPF, GreyListedPolicy, GreyListedPolicy, int /* (*BPF_AUX) */) {
-  BPF_ASSERT(syscall(__NR_getpid) == -1);
+  BPF_ASSERT(sys_getpid() == -1);
   BPF_ASSERT(errno == EPERM);
   BPF_ASSERT(*BPF_AUX == 0);
   BPF_ASSERT(syscall(__NR_geteuid) == syscall(__NR_getuid));
@@ -2098,7 +2099,7 @@ SANDBOX_TEST(SandboxBPF, DISABLE_ON_TSAN(SeccompRetTrace)) {
     BPF_ASSERT(sandbox.StartSandbox(SandboxBPF::PROCESS_SINGLE_THREADED));
 
     // getpid is allowed.
-    BPF_ASSERT_EQ(my_pid, syscall(__NR_getpid));
+    BPF_ASSERT_EQ(my_pid, sys_getpid());
 
     // write to stdout is skipped and returns a fake value.
     BPF_ASSERT_EQ(kExpectedReturnValue,
diff --git a/sandbox/linux/sandbox_linux.gypi b/sandbox/linux/sandbox_linux.gypi
index 7f054a2..08a5c43 100644
--- a/sandbox/linux/sandbox_linux.gypi
+++ b/sandbox/linux/sandbox_linux.gypi
@@ -150,6 +150,7 @@
       ],
       'dependencies': [
         '../base/base.gyp:base',
+        'sandbox_services',
         'sandbox_services_headers',
       ],
       'defines': [
@@ -179,6 +180,7 @@
       ],
       'dependencies': [
         '../base/base.gyp:base',
+        'sandbox_services',
         'seccomp_bpf',
       ],
       'defines': [
@@ -221,6 +223,8 @@
         'services/init_process_reaper.h',
         'services/scoped_process.cc',
         'services/scoped_process.h',
+        'services/syscall_wrappers.cc',
+        'services/syscall_wrappers.h',
         'services/thread_helpers.cc',
         'services/thread_helpers.h',
         'services/yama.cc',
diff --git a/sandbox/linux/sandbox_linux_test_sources.gypi b/sandbox/linux/sandbox_linux_test_sources.gypi
index f246ebb..219b4e5 100644
--- a/sandbox/linux/sandbox_linux_test_sources.gypi
+++ b/sandbox/linux/sandbox_linux_test_sources.gypi
@@ -8,6 +8,7 @@
   'dependencies': [
     'sandbox',
     'sandbox_linux_test_utils',
+    'sandbox_services',
     '../base/base.gyp:base',
     '../base/base.gyp:test_support_base',
     '../testing/gtest.gyp:gtest',
diff --git a/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc b/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc
index 8f11bbb..69d32bc 100644
--- a/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc
+++ b/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc
@@ -19,6 +19,7 @@
 #include "sandbox/linux/seccomp-bpf-helpers/syscall_sets.h"
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
 #include "sandbox/linux/services/linux_syscalls.h"
+#include "sandbox/linux/services/syscall_wrappers.h"
 
 // Changing this implementation will have an effect on *all* policies.
 // Currently this means: Renderer/Worker, GPU, Flash and NaCl.
@@ -237,12 +238,13 @@ ResultExpr EvaluateSyscallImpl(int fs_denied_errno,
 BaselinePolicy::BaselinePolicy() : BaselinePolicy(EPERM) {}
 
 BaselinePolicy::BaselinePolicy(int fs_denied_errno)
-    : fs_denied_errno_(fs_denied_errno), policy_pid_(syscall(__NR_getpid)) {}
+    : fs_denied_errno_(fs_denied_errno), policy_pid_(sys_getpid()) {
+}
 
 BaselinePolicy::~BaselinePolicy() {
   // Make sure that this policy is created, used and destroyed by a single
   // process.
-  DCHECK_EQ(syscall(__NR_getpid), policy_pid_);
+  DCHECK_EQ(sys_getpid(), policy_pid_);
 }
 
 ResultExpr BaselinePolicy::EvaluateSyscall(int sysno) const {
@@ -250,7 +252,7 @@ ResultExpr BaselinePolicy::EvaluateSyscall(int sysno) const {
   DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
   // Make sure that this policy is used in the creating process.
   if (1 == sysno) {
-    DCHECK_EQ(syscall(__NR_getpid), policy_pid_);
+    DCHECK_EQ(sys_getpid(), policy_pid_);
   }
   return EvaluateSyscallImpl(fs_denied_errno_, policy_pid_, sysno);
 }
diff --git a/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc b/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc
index a71975b..c472a1b 100644
--- a/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc
+++ b/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc
@@ -32,6 +32,7 @@
 #include "sandbox/linux/seccomp-bpf/syscall.h"
 #include "sandbox/linux/services/android_futex.h"
 #include "sandbox/linux/services/linux_syscalls.h"
+#include "sandbox/linux/services/syscall_wrappers.h"
 #include "sandbox/linux/services/thread_helpers.h"
 #include "sandbox/linux/tests/unit_tests.h"
 
@@ -113,7 +114,7 @@ BPF_TEST_C(BaselinePolicy, ForkErrno, BaselinePolicy) {
 }
 
 pid_t ForkX86Glibc() {
-  return syscall(__NR_clone, CLONE_PARENT_SETTID | SIGCHLD);
+  return sys_clone(CLONE_PARENT_SETTID | SIGCHLD, 0, 0, 0, 0);
 }
 
 BPF_TEST_C(BaselinePolicy, ForkX86Eperm, BaselinePolicy) {
@@ -127,8 +128,8 @@ BPF_TEST_C(BaselinePolicy, ForkX86Eperm, BaselinePolicy) {
 }
 
 pid_t ForkARMGlibc() {
-  return syscall(__NR_clone,
-                 CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD);
+  return sys_clone(CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0, 0, 0,
+                   0);
 }
 
 BPF_TEST_C(BaselinePolicy, ForkArmEperm, BaselinePolicy) {
@@ -150,7 +151,7 @@ BPF_DEATH_TEST_C(BaselinePolicy,
                  DisallowedCloneFlagCrashes,
                  DEATH_SEGV_MESSAGE(GetCloneErrorMessageContentForTests()),
                  BaselinePolicy) {
-  pid_t pid = syscall(__NR_clone, CLONE_THREAD | SIGCHLD);
+  pid_t pid = sys_clone(CLONE_THREAD | SIGCHLD, 0, 0, 0, 0);
   HandlePostForkReturn(pid);
 }
 
diff --git a/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc b/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc
index 162b371..42be00b 100644
--- a/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc
+++ b/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc
@@ -17,6 +17,7 @@
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
 #include "sandbox/linux/seccomp-bpf/syscall.h"
 #include "sandbox/linux/services/linux_syscalls.h"
+#include "sandbox/linux/services/syscall_wrappers.h"
 
 #if defined(__mips__)
 // __NR_Linux, is defined in <asm/unistd.h>.
@@ -223,7 +224,7 @@ intptr_t SIGSYSSchedHandler(const struct arch_seccomp_data& args,
     case __NR_sched_setattr:
     case __NR_sched_setparam:
     case __NR_sched_setscheduler:
-      const pid_t tid = syscall(__NR_gettid);
+      const pid_t tid = sys_gettid();
       // The first argument is the pid.  If is our thread id, then replace it
       // with 0, which is equivalent and allowed by the policy.
       if (args.args[0] == static_cast<uint64_t>(tid)) {
diff --git a/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions_unittests.cc b/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions_unittests.cc
index 83e8489..c97d95e 100644
--- a/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions_unittests.cc
+++ b/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions_unittests.cc
@@ -23,6 +23,7 @@
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
 #include "sandbox/linux/seccomp-bpf/syscall.h"
 #include "sandbox/linux/services/linux_syscalls.h"
+#include "sandbox/linux/services/syscall_wrappers.h"
 #include "sandbox/linux/tests/unit_tests.h"
 
 #if !defined(OS_ANDROID)
@@ -163,7 +164,7 @@ void CheckSchedGetParam(pid_t pid, struct sched_param* param) {
 
 void SchedGetParamThread(base::WaitableEvent* thread_run) {
   const pid_t pid = getpid();
-  const pid_t tid = syscall(__NR_gettid);
+  const pid_t tid = sys_gettid();
   BPF_ASSERT_NE(pid, tid);
 
   struct sched_param current_pid_param;
diff --git a/sandbox/linux/seccomp-bpf/bpf_tests_unittest.cc b/sandbox/linux/seccomp-bpf/bpf_tests_unittest.cc
index 9d2bb79..192e68d 100644
--- a/sandbox/linux/seccomp-bpf/bpf_tests_unittest.cc
+++ b/sandbox/linux/seccomp-bpf/bpf_tests_unittest.cc
@@ -17,6 +17,7 @@
 #include "sandbox/linux/bpf_dsl/policy.h"
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
 #include "sandbox/linux/services/linux_syscalls.h"
+#include "sandbox/linux/services/syscall_wrappers.h"
 #include "sandbox/linux/tests/unit_tests.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -82,13 +83,11 @@ TEST(BPFTest, BPFTesterCompatibilityDelegateLeakTest) {
 
 class EnosysPtracePolicy : public bpf_dsl::Policy {
  public:
-  EnosysPtracePolicy() {
-    my_pid_ = syscall(__NR_getpid);
-  }
+  EnosysPtracePolicy() { my_pid_ = sys_getpid(); }
   virtual ~EnosysPtracePolicy() {
     // Policies should be able to bind with the process on which they are
     // created. They should never be created in a parent process.
-    BPF_ASSERT_EQ(my_pid_, syscall(__NR_getpid));
+    BPF_ASSERT_EQ(my_pid_, sys_getpid());
   }
 
   virtual ResultExpr EvaluateSyscall(int system_call_number) const override {
@@ -96,7 +95,7 @@ class EnosysPtracePolicy : public bpf_dsl::Policy {
     if (system_call_number == __NR_ptrace) {
       // The EvaluateSyscall function should run in the process that created
       // the current object.
-      BPF_ASSERT_EQ(my_pid_, syscall(__NR_getpid));
+      BPF_ASSERT_EQ(my_pid_, sys_getpid());
       return Error(ENOSYS);
     } else {
       return Allow();
diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
index d5a5d4d..43c9af6 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
@@ -41,6 +41,7 @@
 #include "sandbox/linux/seccomp-bpf/trap.h"
 #include "sandbox/linux/seccomp-bpf/verifier.h"
 #include "sandbox/linux/services/linux_syscalls.h"
+#include "sandbox/linux/services/syscall_wrappers.h"
 
 using sandbox::bpf_dsl::Allow;
 using sandbox::bpf_dsl::Error;
@@ -92,8 +93,8 @@ class ProbePolicy : public bpf_dsl::Policy {
 };
 
 void ProbeProcess(void) {
-  if (syscall(__NR_getpid) < 0 && errno == EPERM) {
-    syscall(__NR_exit_group, static_cast<intptr_t>(kExpectedExitCode));
+  if (sys_getpid() < 0 && errno == EPERM) {
+    sys_exit_group(kExpectedExitCode);
   }
 }
 
@@ -117,7 +118,7 @@ void TryVsyscallProcess(void) {
   // vsyscall=emulate and some versions of the seccomp BPF patch
   // we may get SIGKILL-ed. Detect this!
   if (time(&current_time) != static_cast<time_t>(-1)) {
-    syscall(__NR_exit_group, static_cast<intptr_t>(kExpectedExitCode));
+    sys_exit_group(kExpectedExitCode);
   }
 }
 
diff --git a/sandbox/linux/services/credentials.cc b/sandbox/linux/services/credentials.cc
index 96702b1..a745220 100644
--- a/sandbox/linux/services/credentials.cc
+++ b/sandbox/linux/services/credentials.cc
@@ -24,6 +24,7 @@
 #include "base/template_util.h"
 #include "base/third_party/valgrind/valgrind.h"
 #include "base/threading/thread.h"
+#include "sandbox/linux/services/syscall_wrappers.h"
 
 namespace {
 
@@ -286,7 +287,7 @@ bool Credentials::SupportsNewUserNS() {
   }
 
   // This is roughly a fork().
-  const pid_t pid = syscall(__NR_clone, CLONE_NEWUSER | SIGCHLD, 0, 0, 0);
+  const pid_t pid = sys_clone(CLONE_NEWUSER | SIGCHLD, 0, 0, 0, 0);
 
   if (pid == -1) {
     CheckCloneNewUserErrno(errno);
diff --git a/sandbox/linux/services/scoped_process.cc b/sandbox/linux/services/scoped_process.cc
index fd42a2a..bdeaee9 100644
--- a/sandbox/linux/services/scoped_process.cc
+++ b/sandbox/linux/services/scoped_process.cc
@@ -17,6 +17,7 @@
 #include "base/logging.h"
 #include "base/posix/eintr_wrapper.h"
 #include "build/build_config.h"
+#include "sandbox/linux/services/syscall_wrappers.h"
 #include "sandbox/linux/services/thread_helpers.h"
 
 namespace sandbox {
@@ -112,7 +113,7 @@ bool ScopedProcess::WaitForClosureToRun() {
 // This method allows to assert it is not happening.
 bool ScopedProcess::IsOriginalProcess() {
   // Make a direct syscall to bypass glibc caching of PIDs.
-  int pid = syscall(__NR_getpid);
+  pid_t pid = sys_getpid();
   return pid == process_id_;
 }
 
diff --git a/sandbox/linux/services/syscall_wrappers.cc b/sandbox/linux/services/syscall_wrappers.cc
new file mode 100644
index 0000000..46fe3e5
--- /dev/null
+++ b/sandbox/linux/services/syscall_wrappers.cc
@@ -0,0 +1,35 @@
+// Copyright 2014 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 "sandbox/linux/services/syscall_wrappers.h"
+
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "sandbox/linux/services/linux_syscalls.h"
+
+namespace sandbox {
+
+pid_t sys_getpid(void) {
+  return syscall(__NR_getpid);
+}
+
+pid_t sys_gettid(void) {
+  return syscall(__NR_gettid);
+}
+
+long sys_clone(unsigned long flags,
+               void* child_stack,
+               void* ptid,
+               void* ctid,
+               struct pt_regs* regs) {
+  return syscall(__NR_clone, flags, child_stack, ptid, ctid, regs);
+}
+
+void sys_exit_group(int status) {
+  syscall(__NR_exit_group, status);
+}
+
+}  // namespace sandbox
diff --git a/sandbox/linux/services/syscall_wrappers.h b/sandbox/linux/services/syscall_wrappers.h
new file mode 100644
index 0000000..eb2bd78
--- /dev/null
+++ b/sandbox/linux/services/syscall_wrappers.h
@@ -0,0 +1,33 @@
+// Copyright 2014 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.
+
+#ifndef SANDBOX_LINUX_SERVICES_SYSCALL_WRAPPERS_H_
+#define SANDBOX_LINUX_SERVICES_SYSCALL_WRAPPERS_H_
+
+#include <sys/types.h>
+
+#include "sandbox/sandbox_export.h"
+
+namespace sandbox {
+
+// Provide direct system call wrappers for a few common system calls.
+// These are guaranteed to perform a system call and do not rely on things such
+// as caching the current pid (c.f. getpid()).
+
+SANDBOX_EXPORT pid_t sys_getpid(void);
+
+SANDBOX_EXPORT pid_t sys_gettid(void);
+
+struct pt_regs;
+SANDBOX_EXPORT long sys_clone(unsigned long flags,
+                              void* child_stack,
+                              void* ptid,
+                              void* ctid,
+                              struct pt_regs* regs);
+
+SANDBOX_EXPORT void sys_exit_group(int status);
+
+}  // namespace sandbox
+
+#endif  // SANDBOX_LINUX_SERVICES_SYSCALL_WRAPPERS_H_
diff --git a/sandbox/linux/services/unix_domain_socket_unittest.cc b/sandbox/linux/services/unix_domain_socket_unittest.cc
index 17208a8..4d57c0d 100644
--- a/sandbox/linux/services/unix_domain_socket_unittest.cc
+++ b/sandbox/linux/services/unix_domain_socket_unittest.cc
@@ -18,6 +18,7 @@
 #include "base/posix/eintr_wrapper.h"
 #include "base/posix/unix_domain_socket_linux.h"
 #include "base/process/process_handle.h"
+#include "sandbox/linux/services/syscall_wrappers.h"
 #include "sandbox/linux/tests/unit_tests.h"
 
 // Additional tests for base's UnixDomainSocket to make sure it behaves
@@ -144,14 +145,14 @@ SANDBOX_TEST(UnixDomainSocketTest, Namespace) {
 
   CHECK(UnixDomainSocket::EnableReceiveProcessId(recv_sock.get()));
 
-  const pid_t pid = syscall(__NR_clone, CLONE_NEWPID | SIGCHLD, 0, 0, 0);
+  const pid_t pid = sys_clone(CLONE_NEWPID | SIGCHLD, 0, 0, 0, 0);
   CHECK_NE(-1, pid);
   if (pid == 0) {
     // Child process.
     recv_sock.reset();
 
     // Check that we think we're pid 1 in our new namespace.
-    CHECK_EQ(1, syscall(__NR_getpid));
+    CHECK_EQ(1, sys_getpid());
 
     SendHello(send_sock.get());
     _exit(0);
@@ -178,13 +179,13 @@ SANDBOX_TEST(UnixDomainSocketTest, DoubleNamespace) {
 
   CHECK(UnixDomainSocket::EnableReceiveProcessId(recv_sock.get()));
 
-  const pid_t pid = syscall(__NR_clone, CLONE_NEWPID | SIGCHLD, 0, 0, 0);
+  const pid_t pid = sys_clone(CLONE_NEWPID | SIGCHLD, 0, 0, 0, 0);
   CHECK_NE(-1, pid);
   if (pid == 0) {
     // Child process.
     recv_sock.reset();
 
-    const pid_t pid2 = syscall(__NR_clone, CLONE_NEWPID | SIGCHLD, 0, 0, 0);
+    const pid_t pid2 = sys_clone(CLONE_NEWPID | SIGCHLD, 0, 0, 0, 0);
     CHECK_NE(-1, pid2);
 
     if (pid2 != 0) {
@@ -195,7 +196,7 @@ SANDBOX_TEST(UnixDomainSocketTest, DoubleNamespace) {
     }
 
     // Check that we think we're pid 1.
-    CHECK_EQ(1, syscall(__NR_getpid));
+    CHECK_EQ(1, sys_getpid());
 
     SendHello(send_sock.get());
     _exit(0);
@@ -244,7 +245,7 @@ SANDBOX_TEST(UnixDomainSocketTest, ImpossiblePid) {
 
   CHECK(UnixDomainSocket::EnableReceiveProcessId(recv_sock.get()));
 
-  const pid_t pid = syscall(__NR_clone, CLONE_NEWPID | SIGCHLD, 0, 0, 0);
+  const pid_t pid = sys_clone(CLONE_NEWPID | SIGCHLD, 0, 0, 0, 0);
   CHECK_NE(-1, pid);
   if (pid == 0) {
     // Child process.