Linux sandbox: add interface for the brokered syscalls

Currently, BrokerProcess:SIGSYS_Handler dispatches syscall numbers to
specific syscall handler, along with a couple sanity checks.

When USER_NOTIF support lands, the broker will also need to read the
syscall number and dispatch to a syscall handler. So, create an
interface that lists the syscall handlers, and deduplicate the
dispatching code.

The current implementations of the syscall dispatch functions will
be specific to BrokerClient, so move them from broker_process.cc to
broker_client.cc as they won't be used by USER_NOTIF.
This means a lot of changing open_broker.Syscall(...) to
open_broker.GetBrokerClientSignalBased()->Syscall(...) in
broker_process_unittest.cc.

Bug: 1117351
Change-Id: I830fa6d10f973cf90697c7e19b80b596ba0428b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379551
Commit-Queue: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809136}

sandbox/linux/BUILD.gn

@@ -351,6 +351,8 @@ component("sandbox_services") {
     "syscall_broker/broker_process.h",
     "syscall_broker/broker_simple_message.cc",
     "syscall_broker/broker_simple_message.h",
+    "syscall_broker/syscall_dispatcher.cc",
+    "syscall_broker/syscall_dispatcher.h",
   ]
 
   defines = [ "SANDBOX_IMPLEMENTATION" ]
@@ -400,6 +402,8 @@ component("sandbox_services") {
       "syscall_broker/broker_process.h",
       "syscall_broker/broker_simple_message.cc",
       "syscall_broker/broker_simple_message.h",
+      "syscall_broker/syscall_dispatcher.cc",
+      "syscall_broker/syscall_dispatcher.h",
     ]
   } else if (!is_android) {
     sources += [

sandbox/linux/bpf_dsl/trap_registry.h

@@ -8,18 +8,11 @@
 #include <stdint.h>
 
 #include "base/macros.h"
+#include "sandbox/linux/system_headers/linux_seccomp.h"
 #include "sandbox/sandbox_export.h"
 
 namespace sandbox {
 
-// This must match the kernel's seccomp_data structure.
-struct arch_seccomp_data {
-  int nr;
-  uint32_t arch;
-  uint64_t instruction_pointer;
-  uint64_t args[6];
-};
-
 namespace bpf_dsl {
 
 // TrapRegistry provides an interface for registering "trap handlers"

sandbox/linux/bpf_dsl/verifier.h

@@ -12,10 +12,10 @@
 #include "base/macros.h"
 #include "sandbox/sandbox_export.h"
 
+struct arch_seccomp_data;
 struct sock_filter;
 
 namespace sandbox {
-struct arch_seccomp_data;
 
 namespace bpf_dsl {
 

sandbox/linux/integration_tests/seccomp_broker_process_unittest.cc

@@ -464,8 +464,8 @@ class HandleFilesystemViaBrokerPolicy : public bpf_dsl::Policy {
     // Broker everything that we're supposed to broker.
     if (broker_process_->IsSyscallAllowed(sysno)) {
       return sandbox::bpf_dsl::Trap(
-          sandbox::syscall_broker::BrokerProcess::SIGSYS_Handler,
-          broker_process_);
+          sandbox::syscall_broker::BrokerClient::SIGSYS_Handler,
+          broker_process_->GetBrokerClientSignalBased());
     }
 
     // Otherwise, if this is a syscall that takes a pathname but isn't an

sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc

@@ -21,6 +21,7 @@
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
 #include "sandbox/linux/seccomp-bpf/syscall.h"
 #include "sandbox/linux/services/syscall_wrappers.h"
+#include "sandbox/linux/system_headers/linux_seccomp.h"
 #include "sandbox/linux/system_headers/linux_syscalls.h"
 
 #if defined(__mips__)
@@ -147,7 +148,7 @@ class NumberToHex {
 
 // Records the syscall number and first four arguments in a crash key, to help
 // debug the failure.
-void SetSeccompCrashKey(const struct sandbox::arch_seccomp_data& args) {
+void SetSeccompCrashKey(const struct arch_seccomp_data& args) {
 #if !defined(OS_NACL_NONSFI)
   NumberToHex<int> nr(args.nr);
   NumberToHex<uint64_t> arg1(args.args[0]);

sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h

@@ -15,43 +15,43 @@
 // guaranteed to be async-signal safe.
 // See sandbox/linux/seccomp-bpf/trap.h to see how they work.
 
-namespace sandbox {
-
 struct arch_seccomp_data;
 
+namespace sandbox {
+
 // This handler will crash the currently running process. The crashing address
 // will be the number of the current system call, extracted from |args|.
 // This handler will also print to stderr the number of the crashing syscall.
-SANDBOX_EXPORT intptr_t
-    CrashSIGSYS_Handler(const struct arch_seccomp_data& args, void* aux);
+SANDBOX_EXPORT intptr_t CrashSIGSYS_Handler(const arch_seccomp_data& args,
+                                            void* aux);
 
 // The following seven handlers are suitable to report failures for specific
 // system calls with additional information.
 
 // The crashing address will be (clone_flags & 0xFFFFFF), where clone_flags is
 // the clone(2) argument, extracted from |args|.
-SANDBOX_EXPORT intptr_t
-    SIGSYSCloneFailure(const struct arch_seccomp_data& args, void* aux);
+SANDBOX_EXPORT intptr_t SIGSYSCloneFailure(const arch_seccomp_data& args,
+                                           void* aux);
 // The crashing address will be (option & 0xFFF), where option is the prctl(2)
 // argument.
-SANDBOX_EXPORT intptr_t
-    SIGSYSPrctlFailure(const struct arch_seccomp_data& args, void* aux);
+SANDBOX_EXPORT intptr_t SIGSYSPrctlFailure(const arch_seccomp_data& args,
+                                           void* aux);
 // The crashing address will be request & 0xFFFF, where request is the ioctl(2)
 // argument.
-SANDBOX_EXPORT intptr_t
-    SIGSYSIoctlFailure(const struct arch_seccomp_data& args, void* aux);
+SANDBOX_EXPORT intptr_t SIGSYSIoctlFailure(const arch_seccomp_data& args,
+                                           void* aux);
 // The crashing address will be (pid & 0xFFF), where pid is the first
 // argument (and can be a tid).
-SANDBOX_EXPORT intptr_t
-    SIGSYSKillFailure(const struct arch_seccomp_data& args, void* aux);
+SANDBOX_EXPORT intptr_t SIGSYSKillFailure(const arch_seccomp_data& args,
+                                          void* aux);
 // The crashing address will be (op & 0xFFF), where op is the second
 // argument.
-SANDBOX_EXPORT intptr_t
-    SIGSYSFutexFailure(const struct arch_seccomp_data& args, void* aux);
+SANDBOX_EXPORT intptr_t SIGSYSFutexFailure(const arch_seccomp_data& args,
+                                           void* aux);
 // The crashing address will be (op & 0xFFF), where op is the second
 // argument.
-SANDBOX_EXPORT intptr_t
-SIGSYSPtraceFailure(const struct arch_seccomp_data& args, void* aux);
+SANDBOX_EXPORT intptr_t SIGSYSPtraceFailure(const arch_seccomp_data& args,
+                                            void* aux);
 // If the syscall is not being called on the current tid, crashes in the same
 // way as CrashSIGSYS_Handler.  Otherwise, returns the result of calling the
 // syscall with the pid argument set to 0 (which for these calls means the
@@ -60,8 +60,8 @@ SIGSYSPtraceFailure(const struct arch_seccomp_data& args, void* aux);
 // sched_getaffinity(), sched_getattr(), sched_getparam(), sched_getscheduler(),
 // sched_rr_get_interval(), sched_setaffinity(), sched_setattr(),
 // sched_setparam(), sched_setscheduler()
-SANDBOX_EXPORT intptr_t
-    SIGSYSSchedHandler(const struct arch_seccomp_data& args, void* aux);
+SANDBOX_EXPORT intptr_t SIGSYSSchedHandler(const arch_seccomp_data& args,
+                                           void* aux);
 
 // Variants of the above functions for use with bpf_dsl.
 SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYS();

sandbox/linux/seccomp-bpf/sandbox_bpf.h

@@ -15,9 +15,10 @@
 #include "sandbox/linux/bpf_dsl/policy.h"
 #include "sandbox/sandbox_export.h"
 
-namespace sandbox {
 struct arch_seccomp_data;
 
+namespace sandbox {
+
 // This class can be used to apply a syscall sandboxing policy expressed in a
 // bpf_dsl::Policy object to the current process.
 // Syscall sandboxing policies get inherited by subprocesses and, once applied,

sandbox/linux/syscall_broker/broker_client.cc

@@ -337,5 +337,13 @@ int BrokerClient::StatFamilySyscall(BrokerCommand syscall_type,
   return return_value;
 }
 
+// static
+intptr_t BrokerClient::SIGSYS_Handler(const arch_seccomp_data& args,
+                                      void* aux_broker_client) {
+  RAW_CHECK(aux_broker_client);
+  auto* broker_client = static_cast<BrokerClient*>(aux_broker_client);
+  return broker_client->DispatchSyscall(args);
+}
+
 }  // namespace syscall_broker
 }  // namespace sandbox

sandbox/linux/syscall_broker/broker_client.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "sandbox/linux/syscall_broker/broker_channel.h"
 #include "sandbox/linux/syscall_broker/broker_command.h"
+#include "sandbox/linux/syscall_broker/syscall_dispatcher.h"
 #include "sandbox/sandbox_export.h"
 
 namespace sandbox {
@@ -26,8 +27,13 @@ class BrokerPermissionList;
 // thread-safe and async-signal safe way. The goal is to be able to use it to
 // replace the open() or access() system calls happening anywhere in a process
 // (as allowed for instance by seccomp-bpf's SIGSYS mechanism).
-class SANDBOX_EXPORT BrokerClient {
+class SANDBOX_EXPORT BrokerClient : public SyscallDispatcher {
  public:
+  // Handler to be used with a bpf_dsl Trap() function to forward system calls
+  // to the methods below.
+  static intptr_t SIGSYS_Handler(const arch_seccomp_data& args,
+                                 void* aux_broker_process);
+
   // |policy| needs to match the policy used by BrokerHost. This
   // allows to predict some of the requests which will be denied
   // and save an IPC round trip.
@@ -37,43 +43,29 @@ class SANDBOX_EXPORT BrokerClient {
                BrokerChannel::EndPoint ipc_channel,
                const BrokerCommandSet& allowed_command_set,
                bool fast_check_in_client);
-  ~BrokerClient();
+  ~BrokerClient() override;
 
-  // Get the file descriptor used for IPC. This is used for tests.
-  int GetIPCDescriptor() const { return ipc_channel_.get(); }
+  // Get the file descriptor used for IPC.
+  int GetIPCDescriptorForTesting() const { return ipc_channel_.get(); }
 
   // The following public methods can be used in place of the equivalently
   // name system calls. They all return -errno on errors. They are all async
   // signal safe so they may be called from a SIGSYS trap handler.
 
-  // Can be used in place of access().
-  // X_OK will always return an error in practice since the broker process
-  // doesn't support execute permissions.
-  int Access(const char* pathname, int mode) const;
-
-  // Can be used in place of mkdir().
-  int Mkdir(const char* path, int mode) const;
-
-  // Can be used in place of open().
-  // The implementation only supports certain white listed flags and will
-  // return -EPERM on other flags.
-  int Open(const char* pathname, int flags) const;
-
-  // Can be used in place of Readlink().
-  int Readlink(const char* path, char* buf, size_t bufsize) const;
-
-  // Can be used in place of rename().
-  int Rename(const char* oldpath, const char* newpath) const;
-
-  // Can be used in place of rmdir().
-  int Rmdir(const char* path) const;
-
-  // Can be used in place of stat()/stat64()/lstat()/lstat64()
-  int Stat(const char* pathname, bool follow_links, struct stat* sb) const;
-  int Stat64(const char* pathname, bool folllow_links, struct stat64* sb) const;
-
-  // Can be used in place of unlink().
-  int Unlink(const char* unlink) const;
+  // SyscallDispatcher implementation:
+  int Access(const char* pathname, int mode) const override;
+  int Mkdir(const char* path, int mode) const override;
+  int Open(const char* pathname, int flags) const override;
+  int Readlink(const char* path, char* buf, size_t bufsize) const override;
+  int Rename(const char* oldpath, const char* newpath) const override;
+  int Rmdir(const char* path) const override;
+  int Stat(const char* pathname,
+           bool follow_links,
+           struct stat* sb) const override;
+  int Stat64(const char* pathname,
+             bool follow_links,
+             struct stat64* sb) const override;
+  int Unlink(const char* unlink) const override;
 
  private:
   int PathOnlySyscall(BrokerCommand syscall_type, const char* pathname) const;

sandbox/linux/syscall_broker/broker_process.cc

@@ -194,251 +194,5 @@ void BrokerProcess::CloseChannel() {
   broker_client_.reset();
 }
 
-int BrokerProcess::Access(const char* pathname, int mode) const {
-  RAW_CHECK(initialized_);
-  return broker_client_->Access(pathname, mode);
-}
-
-int BrokerProcess::Mkdir(const char* path, int mode) const {
-  RAW_CHECK(initialized_);
-  return broker_client_->Mkdir(path, mode);
-}
-
-int BrokerProcess::Open(const char* pathname, int flags) const {
-  RAW_CHECK(initialized_);
-  return broker_client_->Open(pathname, flags);
-}
-
-int BrokerProcess::Readlink(const char* path, char* buf, size_t bufsize) const {
-  RAW_CHECK(initialized_);
-  return broker_client_->Readlink(path, buf, bufsize);
-}
-
-int BrokerProcess::Rename(const char* oldpath, const char* newpath) const {
-  RAW_CHECK(initialized_);
-  return broker_client_->Rename(oldpath, newpath);
-}
-
-int BrokerProcess::Rmdir(const char* pathname) const {
-  RAW_CHECK(initialized_);
-  return broker_client_->Rmdir(pathname);
-}
-
-int BrokerProcess::Stat(const char* pathname,
-                        bool follow_links,
-                        struct stat* sb) const {
-  RAW_CHECK(initialized_);
-  return broker_client_->Stat(pathname, follow_links, sb);
-}
-
-int BrokerProcess::Stat64(const char* pathname,
-                          bool follow_links,
-                          struct stat64* sb) const {
-  RAW_CHECK(initialized_);
-  return broker_client_->Stat64(pathname, follow_links, sb);
-}
-
-int BrokerProcess::Unlink(const char* pathname) const {
-  RAW_CHECK(initialized_);
-  return broker_client_->Unlink(pathname);
-}
-
-#if defined(MEMORY_SANITIZER)
-#define BROKER_UNPOISON_STRING(x) __msan_unpoison_string(x)
-#else
-#define BROKER_UNPOISON_STRING(x)
-#endif
-
-namespace {
-// Validates the args passed to a *statat*() syscall and performs the syscall
-// using |broker_process|.
-int PerformStatat(const sandbox::arch_seccomp_data& args,
-                  BrokerProcess* broker_process,
-                  bool arch64) {
-  if (static_cast<int>(args.args[0]) != AT_FDCWD)
-    return -EPERM;
-  // Only allow the AT_SYMLINK_NOFOLLOW flag which is used by some libc
-  // implementations for lstat().
-  if ((static_cast<int>(args.args[3]) & ~AT_SYMLINK_NOFOLLOW) != 0)
-    return -EINVAL;
-
-  const bool follow_links =
-      !(static_cast<int>(args.args[3]) & AT_SYMLINK_NOFOLLOW);
-  if (arch64) {
-    return broker_process->Stat64(
-        reinterpret_cast<const char*>(args.args[1]), follow_links,
-        reinterpret_cast<struct stat64*>(args.args[2]));
-  }
-
-  return broker_process->Stat(reinterpret_cast<const char*>(args.args[1]),
-                              follow_links,
-                              reinterpret_cast<struct stat*>(args.args[2]));
-}
-}  // namespace
-
-// static
-intptr_t BrokerProcess::SIGSYS_Handler(const sandbox::arch_seccomp_data& args,
-                                       void* aux_broker_process) {
-  RAW_CHECK(aux_broker_process);
-  auto* broker_process = static_cast<BrokerProcess*>(aux_broker_process);
-  switch (args.nr) {
-#if defined(__NR_access)
-    case __NR_access:
-      return broker_process->Access(reinterpret_cast<const char*>(args.args[0]),
-                                    static_cast<int>(args.args[1]));
-#endif
-#if defined(__NR_faccessat)
-    case __NR_faccessat:
-      if (static_cast<int>(args.args[0]) != AT_FDCWD)
-        return -EPERM;
-      return broker_process->Access(reinterpret_cast<const char*>(args.args[1]),
-                                    static_cast<int>(args.args[2]));
-#endif
-#if defined(__NR_mkdir)
-    case __NR_mkdir:
-      return broker_process->Mkdir(reinterpret_cast<const char*>(args.args[0]),
-                                   static_cast<int>(args.args[1]));
-#endif
-#if defined(__NR_mkdirat)
-    case __NR_mkdirat:
-      if (static_cast<int>(args.args[0]) != AT_FDCWD)
-        return -EPERM;
-      return broker_process->Mkdir(reinterpret_cast<const char*>(args.args[1]),
-                                   static_cast<int>(args.args[2]));
-#endif
-#if defined(__NR_open)
-    case __NR_open:
-      // http://crbug.com/372840
-      BROKER_UNPOISON_STRING(reinterpret_cast<const char*>(args.args[0]));
-      return broker_process->Open(reinterpret_cast<const char*>(args.args[0]),
-                                  static_cast<int>(args.args[1]));
-#endif
-#if defined(__NR_openat)
-    case __NR_openat:
-      if (static_cast<int>(args.args[0]) != AT_FDCWD)
-        return -EPERM;
-      // http://crbug.com/372840
-      BROKER_UNPOISON_STRING(reinterpret_cast<const char*>(args.args[1]));
-      return broker_process->Open(reinterpret_cast<const char*>(args.args[1]),
-                                  static_cast<int>(args.args[2]));
-#endif
-#if defined(__NR_readlink)
-    case __NR_readlink:
-      return broker_process->Readlink(
-          reinterpret_cast<const char*>(args.args[0]),
-          reinterpret_cast<char*>(args.args[1]),
-          static_cast<size_t>(args.args[2]));
-#endif
-#if defined(__NR_readlinkat)
-    case __NR_readlinkat:
-      if (static_cast<int>(args.args[0]) != AT_FDCWD)
-        return -EPERM;
-      return broker_process->Readlink(
-          reinterpret_cast<const char*>(args.args[1]),
-          reinterpret_cast<char*>(args.args[2]),
-          static_cast<size_t>(args.args[3]));
-#endif
-#if defined(__NR_rename)
-    case __NR_rename:
-      return broker_process->Rename(
-          reinterpret_cast<const char*>(args.args[0]),
-          reinterpret_cast<const char*>(args.args[1]));
-#endif
-#if defined(__NR_renameat)
-    case __NR_renameat:
-      if (static_cast<int>(args.args[0]) != AT_FDCWD ||
-          static_cast<int>(args.args[2]) != AT_FDCWD) {
-        return -EPERM;
-      }
-      return broker_process->Rename(
-          reinterpret_cast<const char*>(args.args[1]),
-          reinterpret_cast<const char*>(args.args[3]));
-#endif
-#if defined(__NR_renameat2)
-    case __NR_renameat2:
-      if (static_cast<int>(args.args[0]) != AT_FDCWD ||
-          static_cast<int>(args.args[2]) != AT_FDCWD) {
-        return -EPERM;
-      }
-      if (static_cast<int>(args.args[4]) != 0)
-        return -EINVAL;
-      return broker_process->Rename(
-          reinterpret_cast<const char*>(args.args[1]),
-          reinterpret_cast<const char*>(args.args[3]));
-#endif
-#if defined(__NR_rmdir)
-    case __NR_rmdir:
-      return broker_process->Rmdir(reinterpret_cast<const char*>(args.args[0]));
-#endif
-#if defined(__NR_stat)
-    case __NR_stat:
-      return broker_process->Stat(reinterpret_cast<const char*>(args.args[0]),
-                                  true,
-                                  reinterpret_cast<struct stat*>(args.args[1]));
-#endif
-#if defined(__NR_stat64)
-    case __NR_stat64:
-      return broker_process->Stat64(
-          reinterpret_cast<const char*>(args.args[0]), true,
-          reinterpret_cast<struct stat64*>(args.args[1]));
-#endif
-#if defined(__NR_lstat)
-    case __NR_lstat:
-      // See https://crbug.com/847096
-      BROKER_UNPOISON_STRING(reinterpret_cast<const char*>(args.args[0]));
-      return broker_process->Stat(reinterpret_cast<const char*>(args.args[0]),
-                                  false,
-                                  reinterpret_cast<struct stat*>(args.args[1]));
-#endif
-#if defined(__NR_lstat64)
-    case __NR_lstat64:
-      // See https://crbug.com/847096
-      BROKER_UNPOISON_STRING(reinterpret_cast<const char*>(args.args[0]));
-      return broker_process->Stat64(
-          reinterpret_cast<const char*>(args.args[0]), false,
-          reinterpret_cast<struct stat64*>(args.args[1]));
-#endif
-#if defined(__NR_fstatat)
-    case __NR_fstatat:
-      return PerformStatat(args, broker_process, /*arch64=*/false);
-#endif
-#if defined(__NR_fstatat64)
-    case __NR_fstatat64:
-      return PerformStatat(args, broker_process, /*arch64=*/true);
-#endif
-#if defined(__NR_newfstatat)
-    case __NR_newfstatat:
-      return PerformStatat(args, broker_process, /*arch64=*/false);
-#endif
-#if defined(__NR_unlink)
-    case __NR_unlink:
-      return broker_process->Unlink(
-          reinterpret_cast<const char*>(args.args[0]));
-#endif
-#if defined(__NR_unlinkat)
-    case __NR_unlinkat: {
-      if (static_cast<int>(args.args[0]) != AT_FDCWD)
-        return -EPERM;
-
-      int flags = static_cast<int>(args.args[2]);
-
-      if (flags == AT_REMOVEDIR) {
-        return broker_process->Rmdir(
-            reinterpret_cast<const char*>(args.args[1]));
-      }
-
-      if (flags != 0)
-        return -EPERM;
-
-      return broker_process->Unlink(
-          reinterpret_cast<const char*>(args.args[1]));
-    }
-#endif  // defined(__NR_unlinkat)
-    default:
-      RAW_CHECK(false);
-      return -ENOSYS;
-  }
-}
-
 }  // namespace syscall_broker
 }  // namespace sandbox

sandbox/linux/syscall_broker/broker_process.h

@@ -39,11 +39,6 @@ class BrokerFilePermission;
 // 4. Use open_broker.Open() to open files.
 class SANDBOX_EXPORT BrokerProcess {
  public:
-  // Handler to be used with a bpf_dsl Trap() function to forward system calls
-  // to the methods below.
-  static intptr_t SIGSYS_Handler(const arch_seccomp_data& args,
-                                 void* aux_broker_process);
-
   // |denied_errno| is the error code returned when methods such as Open()
   // or Access() are invoked on a file which is not in the allowlist (EACCESS
   // would be a typical value).  |allowed_command_mask| is a bitwise-or of
@@ -89,40 +84,6 @@ class SANDBOX_EXPORT BrokerProcess {
     return broker_client_.get();
   }
 
-  // The following methods are used in place of the equivalently-named
-  // syscalls by the trap handler. They, in turn, forward the call onto
-  // |broker_client_| for further processing. They will all be async signal
-  // safe. They all return -errno on errors.
-
-  // Can be used in place of access().
-  // X_OK will always return an error in practice since the broker process
-  // doesn't support execute permissions.
-  int Access(const char* pathname, int mode) const;
-
-  // Can be used in place of mkdir().
-  int Mkdir(const char* path, int mode) const;
-
-  // Can be used in place of open()
-  // The implementation only supports certain white listed flags and will
-  // return -EPERM on other flags.
-  int Open(const char* pathname, int flags) const;
-
-  // Can be used in place of readlink().
-  int Readlink(const char* path, char* buf, size_t bufsize) const;
-
-  // Can be used in place of rename().
-  int Rename(const char* oldpath, const char* newpath) const;
-
-  // Can be used in place of rmdir().
-  int Rmdir(const char* path) const;
-
-  // Can be used in place of stat()/stat64()/lstat()/lstat64().
-  int Stat(const char* pathname, bool follow_links, struct stat* sb) const;
-  int Stat64(const char* pathname, bool follow_links, struct stat64* sb) const;
-
-  // Can be used in place of unlink().
-  int Unlink(const char* path) const;
-
  private:
   friend class BrokerProcessTestHelper;
   friend class HandleFilesystemViaBrokerPolicy;

sandbox/linux/syscall_broker/syscall_dispatcher.cc

+// Copyright 2020 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/syscall_broker/syscall_dispatcher.h"
+
+#include <fcntl.h>
+
+#include "base/check.h"
+#include "base/logging.h"
+#include "sandbox/linux/system_headers/linux_syscalls.h"
+
+namespace sandbox {
+namespace syscall_broker {
+
+#if defined(MEMORY_SANITIZER)
+#define BROKER_UNPOISON_STRING(x) __msan_unpoison_string(x)
+#else
+#define BROKER_UNPOISON_STRING(x)
+#endif
+
+int SyscallDispatcher::PerformStatat(const arch_seccomp_data& args,
+                                     bool arch64) {
+  if (static_cast<int>(args.args[0]) != AT_FDCWD)
+    return -EPERM;
+  // Only allow the AT_SYMLINK_NOFOLLOW flag which is used by some libc
+  // implementations for lstat().
+  if ((static_cast<int>(args.args[3]) & ~AT_SYMLINK_NOFOLLOW) != 0)
+    return -EINVAL;
+
+  const bool follow_links =
+      !(static_cast<int>(args.args[3]) & AT_SYMLINK_NOFOLLOW);
+  if (arch64) {
+    return Stat64(reinterpret_cast<const char*>(args.args[1]), follow_links,
+                  reinterpret_cast<struct stat64*>(args.args[2]));
+  }
+
+  return Stat(reinterpret_cast<const char*>(args.args[1]), follow_links,
+              reinterpret_cast<struct stat*>(args.args[2]));
+}
+
+int SyscallDispatcher::DispatchSyscall(const arch_seccomp_data& args) {
+  switch (args.nr) {
+#if defined(__NR_access)
+    case __NR_access:
+      return Access(reinterpret_cast<const char*>(args.args[0]),
+                    static_cast<int>(args.args[1]));
+#endif
+#if defined(__NR_faccessat)
+    case __NR_faccessat:
+      if (static_cast<int>(args.args[0]) != AT_FDCWD)
+        return -EPERM;
+      return Access(reinterpret_cast<const char*>(args.args[1]),
+                    static_cast<int>(args.args[2]));
+#endif
+#if defined(__NR_mkdir)
+    case __NR_mkdir:
+      return Mkdir(reinterpret_cast<const char*>(args.args[0]),
+                   static_cast<int>(args.args[1]));
+#endif
+#if defined(__NR_mkdirat)
+    case __NR_mkdirat:
+      if (static_cast<int>(args.args[0]) != AT_FDCWD)
+        return -EPERM;
+      return Mkdir(reinterpret_cast<const char*>(args.args[1]),
+                   static_cast<int>(args.args[2]));
+#endif
+#if defined(__NR_open)
+    case __NR_open:
+      // http://crbug.com/372840
+      BROKER_UNPOISON_STRING(reinterpret_cast<const char*>(args.args[0]));
+      return Open(reinterpret_cast<const char*>(args.args[0]),
+                  static_cast<int>(args.args[1]));
+#endif
+#if defined(__NR_openat)
+    case __NR_openat:
+      if (static_cast<int>(args.args[0]) != AT_FDCWD)
+        return -EPERM;
+      // http://crbug.com/372840
+      BROKER_UNPOISON_STRING(reinterpret_cast<const char*>(args.args[1]));
+      return Open(reinterpret_cast<const char*>(args.args[1]),
+                  static_cast<int>(args.args[2]));
+#endif
+#if defined(__NR_readlink)
+    case __NR_readlink:
+      return Readlink(reinterpret_cast<const char*>(args.args[0]),
+                      reinterpret_cast<char*>(args.args[1]),
+                      static_cast<size_t>(args.args[2]));
+#endif
+#if defined(__NR_readlinkat)
+    case __NR_readlinkat:
+      if (static_cast<int>(args.args[0]) != AT_FDCWD)
+        return -EPERM;
+      return Readlink(reinterpret_cast<const char*>(args.args[1]),
+                      reinterpret_cast<char*>(args.args[2]),
+                      static_cast<size_t>(args.args[3]));
+#endif
+#if defined(__NR_rename)
+    case __NR_rename:
+      return Rename(reinterpret_cast<const char*>(args.args[0]),
+                    reinterpret_cast<const char*>(args.args[1]));
+#endif
+#if defined(__NR_renameat)
+    case __NR_renameat:
+      if (static_cast<int>(args.args[0]) != AT_FDCWD ||
+          static_cast<int>(args.args[2]) != AT_FDCWD) {
+        return -EPERM;
+      }
+      return Rename(reinterpret_cast<const char*>(args.args[1]),
+                    reinterpret_cast<const char*>(args.args[3]));
+#endif
+#if defined(__NR_renameat2)
+    case __NR_renameat2:
+      if (static_cast<int>(args.args[0]) != AT_FDCWD ||
+          static_cast<int>(args.args[2]) != AT_FDCWD) {
+        return -EPERM;
+      }
+      if (static_cast<int>(args.args[4]) != 0)
+        return -EINVAL;
+      return Rename(reinterpret_cast<const char*>(args.args[1]),
+                    reinterpret_cast<const char*>(args.args[3]));
+#endif
+#if defined(__NR_rmdir)
+    case __NR_rmdir:
+      return Rmdir(reinterpret_cast<const char*>(args.args[0]));
+#endif
+#if defined(__NR_stat)
+    case __NR_stat:
+      return Stat(reinterpret_cast<const char*>(args.args[0]), true,
+                  reinterpret_cast<struct stat*>(args.args[1]));
+#endif
+#if defined(__NR_stat64)
+    case __NR_stat64:
+      return Stat64(reinterpret_cast<const char*>(args.args[0]), true,
+                    reinterpret_cast<struct stat64*>(args.args[1]));
+#endif
+#if defined(__NR_lstat)
+    case __NR_lstat:
+      // See https://crbug.com/847096
+      BROKER_UNPOISON_STRING(reinterpret_cast<const char*>(args.args[0]));
+      return Stat(reinterpret_cast<const char*>(args.args[0]), false,
+                  reinterpret_cast<struct stat*>(args.args[1]));
+#endif
+#if defined(__NR_lstat64)
+    case __NR_lstat64:
+      // See https://crbug.com/847096
+      BROKER_UNPOISON_STRING(reinterpret_cast<const char*>(args.args[0]));
+      return Stat64(reinterpret_cast<const char*>(args.args[0]), false,
+                    reinterpret_cast<struct stat64*>(args.args[1]));
+#endif
+#if defined(__NR_fstatat)
+    case __NR_fstatat:
+      return PerformStatat(args, /*arch64=*/false);
+#endif
+#if defined(__NR_fstatat64)
+    case __NR_fstatat64:
+      return PerformStatat(args, /*arch64=*/true);
+#endif
+#if defined(__NR_newfstatat)
+    case __NR_newfstatat:
+      return PerformStatat(args, /*arch64=*/false);
+#endif
+#if defined(__NR_unlink)
+    case __NR_unlink:
+      return Unlink(reinterpret_cast<const char*>(args.args[0]));
+#endif
+#if defined(__NR_unlinkat)
+    case __NR_unlinkat: {
+      if (static_cast<int>(args.args[0]) != AT_FDCWD)
+        return -EPERM;
+
+      int flags = static_cast<int>(args.args[2]);
+
+      if (flags == AT_REMOVEDIR) {
+        return Rmdir(reinterpret_cast<const char*>(args.args[1]));
+      }
+
+      if (flags != 0)
+        return -EPERM;
+
+      return Unlink(reinterpret_cast<const char*>(args.args[1]));
+    }
+#endif  // defined(__NR_unlinkat)
+    default:
+      RAW_CHECK(false);
+      return -ENOSYS;
+  }
+}
+
+}  // namespace syscall_broker
+}  // namespace sandbox

sandbox/linux/syscall_broker/syscall_dispatcher.h

+// Copyright 2020 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_SYSCALL_BROKER_SYSCALL_DISPATCHER_H_
+#define SANDBOX_LINUX_SYSCALL_BROKER_SYSCALL_DISPATCHER_H_
+
+#include <sys/stat.h>
+#include <cstddef>
+
+#include "sandbox/linux/system_headers/linux_seccomp.h"
+
+namespace sandbox {
+namespace syscall_broker {
+
+// An abstract class that defines all the system calls we perform for the
+// sandboxed process.
+class SyscallDispatcher {
+ public:
+  // Emulates access()/faccessat().
+  // X_OK will always return an error in practice since the broker process
+  // doesn't support execute permissions.
+  virtual int Access(const char* pathname, int mode) const = 0;
+
+  // Emulates mkdir()/mkdirat.
+  virtual int Mkdir(const char* path, int mode) const = 0;
+
+  // Emulates open()/openat().
+  // The implementation only supports certain white listed flags and will
+  // return -EPERM on other flags.
+  virtual int Open(const char* pathname, int flags) const = 0;
+
+  // Emulates readlink()/readlinkat().
+  virtual int Readlink(const char* path, char* buf, size_t bufsize) const = 0;
+
+  // Emulates rename()/renameat()/renameat2().
+  virtual int Rename(const char* oldpath, const char* newpath) const = 0;
+
+  // Emulates rmdir().
+  virtual int Rmdir(const char* path) const = 0;
+
+  // Emulates stat()/stat64()/lstat()/lstat64()/fstatat()/newfstatat().
+  virtual int Stat(const char* pathname,
+                   bool follow_links,
+                   struct stat* sb) const = 0;
+  virtual int Stat64(const char* pathname,
+                     bool follow_links,
+                     struct stat64* sb) const = 0;
+
+  // Emulates unlink()/unlinkat().
+  virtual int Unlink(const char* unlink) const = 0;
+
+  // Validates the args passed to a *statat*() syscall and performs the syscall
+  // using Stat() or Stat64().
+  int PerformStatat(const arch_seccomp_data& args, bool arch64);
+
+  // Reads the syscall number and arguments, imposes some policy (e.g. the *at()
+  // system calls must only allow AT_FDCWD as the first argument), and
+  // dispatches to the correct method from above.
+  // Async-signal-safe since this might be called in a signal handler.
+  int DispatchSyscall(const arch_seccomp_data& args);
+
+ protected:
+  virtual ~SyscallDispatcher() = default;
+};
+
+}  // namespace syscall_broker
+}  // namespace sandbox
+
+#endif  // SANDBOX_LINUX_SYSCALL_BROKER_SYSCALL_DISPATCHER_H_

sandbox/linux/system_headers/linux_seccomp.h

@@ -5,6 +5,8 @@
 #ifndef SANDBOX_LINUX_SYSTEM_HEADERS_LINUX_SECCOMP_H_
 #define SANDBOX_LINUX_SYSTEM_HEADERS_LINUX_SECCOMP_H_
 
+#include <stdint.h>
+
 // The Seccomp2 kernel ABI is not part of older versions of glibc.
 // As we can't break compilation with these versions of the library,
 // we explicitly define all missing symbols.
@@ -103,6 +105,17 @@
 #define SECCOMP_FILTER_FLAG_SPEC_ALLOW (1UL << 2)
 #endif
 
+// In the future, if we add fields to this struct and then access them, they
+// might be out-of-bounds on an older kernel. So before adding to this struct,
+// make sure to annotate them with a comment that it may be unsafe to access
+// those fields on older kernels.
+struct arch_seccomp_data {
+  int nr;
+  uint32_t arch;
+  uint64_t instruction_pointer;
+  uint64_t args[6];
+};
+
 #ifndef SECCOMP_RET_KILL
 // Return values supported for BPF filter programs. Please note that the
 // "illegal" SECCOMP_RET_INVALID is not supported by the kernel, should only

sandbox/policy/linux/sandbox_linux.cc

@@ -40,6 +40,7 @@
 #include "sandbox/linux/services/thread_helpers.h"
 #include "sandbox/linux/services/yama.h"
 #include "sandbox/linux/suid/client/setuid_sandbox_client.h"
+#include "sandbox/linux/syscall_broker/broker_client.h"
 #include "sandbox/linux/syscall_broker/broker_command.h"
 #include "sandbox/linux/syscall_broker/broker_process.h"
 #include "sandbox/policy/linux/bpf_broker_policy_linux.h"
@@ -506,7 +507,8 @@ bool SandboxLinux::ShouldBrokerHandleSyscall(int sysno) const {
 
 sandbox::bpf_dsl::ResultExpr SandboxLinux::HandleViaBroker() const {
   return sandbox::bpf_dsl::Trap(
-      sandbox::syscall_broker::BrokerProcess::SIGSYS_Handler, broker_process_);
+      sandbox::syscall_broker::BrokerClient::SIGSYS_Handler,
+      broker_process_->GetBrokerClientSignalBased());
 }
 
 bool SandboxLinux::HasOpenDirectories() const {