Linux sandbox: Add WriteRemoteData() and ReadFilePathFromRemoteProcess()

These wrap the process_vm_writev() and process_vm_readv() syscalls
which write to another process's memory (as long the caller has ptrace
privileges for the target process). This is necessary for the
USER_NOTIF broker to read syscall params and write to syscall outparams.

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

sandbox/linux/BUILD.gn

@@ -103,6 +103,7 @@ source_set("sandbox_linux_unittests_sources") {
     "syscall_broker/broker_file_permission_unittest.cc",
     "syscall_broker/broker_process_unittest.cc",
     "syscall_broker/broker_simple_message_unittest.cc",
+    "syscall_broker/remote_syscall_arg_handler_unittest.cc",
     "tests/main.cc",
     "tests/scoped_temporary_file.cc",
     "tests/scoped_temporary_file.h",
@@ -358,6 +359,8 @@ component("sandbox_services") {
     "syscall_broker/broker_process.h",
     "syscall_broker/broker_simple_message.cc",
     "syscall_broker/broker_simple_message.h",
+    "syscall_broker/remote_syscall_arg_handler.cc",
+    "syscall_broker/remote_syscall_arg_handler.h",
     "syscall_broker/syscall_dispatcher.cc",
     "syscall_broker/syscall_dispatcher.h",
   ]
@@ -409,6 +412,8 @@ component("sandbox_services") {
       "syscall_broker/broker_process.h",
       "syscall_broker/broker_simple_message.cc",
       "syscall_broker/broker_simple_message.h",
+      "syscall_broker/remote_syscall_arg_handler.cc",
+      "syscall_broker/remote_syscall_arg_handler.h",
       "syscall_broker/syscall_dispatcher.cc",
       "syscall_broker/syscall_dispatcher.h",
     ]

sandbox/linux/syscall_broker/remote_syscall_arg_handler.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/remote_syscall_arg_handler.h"
+
+#include <sys/ioctl.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/uio.h>
+#include <unistd.h>
+
+#include "base/bits.h"
+#include "base/check_op.h"
+#include "base/containers/span.h"
+#include "base/logging.h"
+#include "base/process/process_metrics.h"
+#include "sandbox/linux/system_headers/linux_seccomp.h"
+#include "sandbox/linux/system_headers/linux_syscalls.h"
+
+namespace sandbox {
+namespace syscall_broker {
+
+RemoteProcessIOResult WriteRemoteData(pid_t pid,
+                                      uintptr_t remote_addr,
+                                      size_t remote_size,
+                                      base::span<char> data) {
+  CHECK_GE(remote_size, data.size());
+
+  base::span<char> remote_span(reinterpret_cast<char*>(remote_addr),
+                               remote_size);
+  struct iovec local_iov = {};
+  struct iovec remote_iov = {};
+
+  while (!data.empty()) {
+    local_iov.iov_base = data.data();
+    local_iov.iov_len = data.size();
+    remote_iov.iov_base = remote_span.data();
+    remote_iov.iov_len = data.size();
+
+    ssize_t bytes_written = syscall(__NR_process_vm_writev, pid, &local_iov,
+                                    1ul, &remote_iov, 1ul, 0ul);
+    if (bytes_written < 0) {
+      if (errno == EFAULT)
+        return RemoteProcessIOResult::kRemoteMemoryInvalid;
+      if (errno == ESRCH)
+        return RemoteProcessIOResult::kRemoteExited;
+      PLOG(ERROR)
+          << "process_vm_writev() failed with unknown error code! Write to pid "
+          << pid << " at remote address " << remote_iov.iov_base
+          << " of length " << data.size() << ". ";
+      return RemoteProcessIOResult::kUnknownError;
+    }
+
+    remote_span = remote_span.subspan(bytes_written);
+    data = data.subspan(bytes_written);
+  }
+
+  return RemoteProcessIOResult::kSuccess;
+}
+
+RemoteProcessIOResult ReadFilePathFromRemoteProcess(pid_t pid,
+                                                    const void* remote_addr,
+                                                    std::string* out_str) {
+  // Most pathnames will be small so avoid copying PATH_MAX bytes every time,
+  // by reading in chunks and checking if the the string ends within the
+  // chunk.
+  char buffer[PATH_MAX];
+  base::span<char> buffer_span(buffer);
+
+  struct iovec local_iov = {};
+  struct iovec remote_iov = {};
+
+  uintptr_t remote_ptr = reinterpret_cast<uintptr_t>(remote_addr);
+
+  for (;;) {
+    uintptr_t bytes_left_in_page = internal::NumBytesLeftInPage(remote_ptr);
+
+    // Read the minimum of the chunk size, remaining local buffer size, and
+    // the number of bytes left in the remote page.
+    size_t bytes_to_read = std::min(
+        {internal::kNumBytesPerChunk, buffer_span.size(), bytes_left_in_page});
+
+    // Set up the iovecs.
+    local_iov.iov_base = buffer_span.data();
+    local_iov.iov_len = bytes_to_read;
+
+    remote_iov.iov_base = reinterpret_cast<void*>(remote_ptr);
+    remote_iov.iov_len = bytes_to_read;
+
+    // The arguments below must include the ul suffix since they need to be
+    // 64-bit values, but syscall() takes varargs and doesn't know to promote
+    // them from 32-bit to 64-bit.
+    ssize_t bytes_read = syscall(__NR_process_vm_readv, pid, &local_iov, 1ul,
+                                 &remote_iov, 1ul, 0ul);
+    if (bytes_read < 0) {
+      if (errno == EFAULT)
+        return RemoteProcessIOResult::kRemoteMemoryInvalid;
+      if (errno == ESRCH)
+        return RemoteProcessIOResult::kRemoteExited;
+      PLOG(ERROR)
+          << "process_vm_readv() failed with unknown error code! Read from pid "
+          << pid << " at remote address " << remote_iov.iov_base
+          << " of length " << bytes_to_read << ". ";
+      return RemoteProcessIOResult::kUnknownError;
+    }
+
+    // We successfully performed a read.
+    remote_ptr += bytes_read;
+    buffer_span = buffer_span.subspan(bytes_read);
+
+    // Check for null byte.
+    char* null_byte_ptr =
+        static_cast<char*>(memchr(local_iov.iov_base, '\0', bytes_read));
+    if (null_byte_ptr) {
+      *out_str = std::string(buffer, null_byte_ptr);
+      return RemoteProcessIOResult::kSuccess;
+    }
+
+    if (buffer_span.empty()) {
+      // If we haven't found a null byte yet and our available buffer space is
+      // empty, stop.
+      LOG(ERROR) << "Read PATH_MAX bytes in sandboxed process and did not find "
+                    "expected null byte.";
+      return RemoteProcessIOResult::kExceededPathMax;
+    }
+  }
+}
+
+namespace internal {
+uintptr_t NumBytesLeftInPage(uintptr_t addr) {
+  const uintptr_t page_end = base::bits::Align(addr + 1, base::GetPageSize());
+  return page_end - addr;
+}
+}  // namespace internal
+}  // namespace syscall_broker
+}  // namespace sandbox

sandbox/linux/syscall_broker/remote_syscall_arg_handler.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_REMOTE_SYSCALL_ARG_HANDLER_H_
+#define SANDBOX_LINUX_SYSCALL_BROKER_REMOTE_SYSCALL_ARG_HANDLER_H_
+
+#include <unistd.h>
+
+#include "base/containers/span.h"
+#include "sandbox/sandbox_export.h"
+
+namespace sandbox {
+namespace syscall_broker {
+
+enum class RemoteProcessIOResult {
+  kSuccess,
+  kRemoteExited,
+  kExceededPathMax,
+  kRemoteMemoryInvalid,
+  kUnknownError
+};
+
+// Writes |data| at |remote_addr| in |pid|'s address space. Returns the
+// appropriate result.
+SANDBOX_EXPORT RemoteProcessIOResult WriteRemoteData(pid_t pid,
+                                                     uintptr_t remote_addr,
+                                                     size_t remote_size,
+                                                     base::span<char> data);
+
+// Reads a filepath from |remote_addr| (which points into process |pid|'s memory
+// space) into |*out_str|. Returns the appropriate result.
+// Safety checks should occur before usage of any system call arguments read
+// from a remote address space, so callers should use RemoteSyscallFilepathArgs
+// instead of calling this directly.
+SANDBOX_EXPORT RemoteProcessIOResult
+ReadFilePathFromRemoteProcess(pid_t pid,
+                              const void* remote_addr,
+                              std::string* out_str);
+
+namespace internal {
+// The number of bytes we read from a remote process at a time when reading a
+// remote filepath, to avoid reading PATH_MAX bytes every time.
+const size_t kNumBytesPerChunk = 256;
+
+// Calculates the number of bytes left in a page for a particular address.
+uintptr_t NumBytesLeftInPage(uintptr_t addr);
+}  // namespace internal
+}  // namespace syscall_broker
+}  // namespace sandbox
+
+#endif  // SANDBOX_LINUX_SYSCALL_BROKER_REMOTE_SYSCALL_ARG_HANDLER_H_

sandbox/linux/syscall_broker/remote_syscall_arg_handler_unittest.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/remote_syscall_arg_handler.h"
+
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <algorithm>
+#include <cstring>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/callback_forward.h"
+#include "base/files/scoped_file.h"
+#include "base/macros.h"
+#include "base/posix/unix_domain_socket.h"
+#include "base/process/process_metrics.h"
+#include "base/test/bind_test_util.h"
+#include "sandbox/linux/tests/unit_tests.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace sandbox {
+namespace syscall_broker {
+
+namespace {
+const char kPathPart[] = "/i/am/path";
+
+void FillBufferWithPath(char* buf, size_t size, bool null_terminate) {
+  SANDBOX_ASSERT_LE(size, static_cast<size_t>(PATH_MAX));
+  size_t str_len = strlen(kPathPart);
+  size_t len_left_to_write = size;
+  char* curr_buf_pos = buf;
+  while (len_left_to_write > 0) {
+    size_t bytes_to_write = std::min(str_len, len_left_to_write);
+    memcpy(curr_buf_pos, kPathPart, bytes_to_write);
+    curr_buf_pos += bytes_to_write;
+    len_left_to_write -= bytes_to_write;
+  }
+
+  if (null_terminate) {
+    buf[size - 1] = '\0';
+  }
+}
+
+void VerifyCorrectString(std::string str, size_t size) {
+  SANDBOX_ASSERT_EQ(str.size(), size);
+  size_t curr_path_part_pos = 0;
+  for (char ch : str) {
+    SANDBOX_ASSERT(ch == kPathPart[curr_path_part_pos]);
+    curr_path_part_pos++;
+    curr_path_part_pos %= strlen(kPathPart);
+  }
+}
+
+void* MapPagesOrDie(size_t num_pages) {
+  void* addr = mmap(nullptr, num_pages * base::GetPageSize(),
+                    PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  PCHECK(addr);
+  return addr;
+}
+
+void MprotectLastPageOrDie(char* addr, size_t num_pages) {
+  size_t last_page_offset = (num_pages - 1) * base::GetPageSize();
+  PCHECK(mprotect(addr + last_page_offset, base::GetPageSize(), PROT_NONE) >=
+         0);
+}
+
+pid_t ForkWaitingChild(base::OnceCallback<void(int)>
+                           after_parent_signals_callback = base::DoNothing(),
+                       base::ScopedFD* parent_sync_fd = nullptr) {
+  base::ScopedFD parent_sync, child_sync;
+  base::CreateSocketPair(&parent_sync, &child_sync);
+
+  pid_t pid = fork();
+  if (!pid) {
+    parent_sync.reset();
+    char dummy_buf[1];
+    std::vector<base::ScopedFD> empty_fd_vec;
+    // Wait for parent to exit before exiting ourselves.
+    base::UnixDomainSocket::RecvMsg(child_sync.get(), dummy_buf,
+                                    sizeof(dummy_buf), &empty_fd_vec);
+    std::move(after_parent_signals_callback).Run(child_sync.get());
+    _exit(1);
+  }
+
+  child_sync.reset();
+
+  if (parent_sync_fd)
+    *parent_sync_fd = std::move(parent_sync);
+  else
+    ignore_result(parent_sync.release());  // Closes when parent dies.
+  return pid;
+}
+
+struct ReadTestConfig {
+  size_t start_at = 0;
+  size_t total_size = strlen(kPathPart) + 1;
+  bool include_null_byte = true;
+  bool last_page_inaccessible = false;
+  RemoteProcessIOResult result = RemoteProcessIOResult::kSuccess;
+};
+
+void ReadTest(const ReadTestConfig& test_config) {
+  // Map exactly the right number of pages for the config parameters.
+  size_t total_pages = (test_config.start_at + test_config.total_size +
+                        base::GetPageSize() - 1) /
+                       base::GetPageSize();
+  char* mmap_addr = static_cast<char*>(MapPagesOrDie(total_pages));
+  char* addr = mmap_addr + test_config.start_at;
+  FillBufferWithPath(addr, test_config.total_size,
+                     test_config.include_null_byte);
+
+  if (test_config.last_page_inaccessible)
+    MprotectLastPageOrDie(mmap_addr, total_pages);
+
+  pid_t pid = ForkWaitingChild();
+  munmap(mmap_addr, base::GetPageSize() * total_pages);
+
+  std::string out_str;
+  SANDBOX_ASSERT_EQ(ReadFilePathFromRemoteProcess(pid, addr, &out_str),
+                    test_config.result);
+  if (test_config.result == RemoteProcessIOResult::kSuccess) {
+    VerifyCorrectString(std::move(out_str), test_config.total_size - 1);
+  }
+}
+}  // namespace
+
+// | path + null_byte |
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, BasicRead) {
+  ReadTest(ReadTestConfig());
+}
+
+// | zero + path... | ...path + null_byte + zero |
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, MultipageRead) {
+  ReadTestConfig config;
+  CHECK(PATH_MAX / 2 <= base::GetPageSize());
+  config.start_at = base::GetPageSize() - (PATH_MAX / 2);
+  config.total_size = PATH_MAX;
+
+  ReadTest(config);
+}
+
+// | path... | ...path |
+SANDBOX_TEST_ALLOW_NOISE(BrokerRemoteSyscallArgHandler, ReadExceededPathMax) {
+  ReadTestConfig config;
+  config.total_size = PATH_MAX * 2;
+  config.result = RemoteProcessIOResult::kExceededPathMax;
+}
+// | path... | null_byte + zero |
+SANDBOX_TEST_ALLOW_NOISE(BrokerRemoteSyscallArgHandler,
+                         ReadBarelyExceededPathMax) {
+  ReadTestConfig config;
+  config.total_size = PATH_MAX + 1;
+  config.result = RemoteProcessIOResult::kExceededPathMax;
+}
+
+// | zero + path... | INACCESSIBLE |
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, ReadUnreadablePage) {
+  ReadTestConfig config;
+  config.start_at = base::GetPageSize() - (PATH_MAX / 2);
+  config.total_size = PATH_MAX / 2;
+  config.last_page_inaccessible = true;
+  config.include_null_byte = false;
+  config.result = RemoteProcessIOResult::kRemoteMemoryInvalid;
+
+  ReadTest(config);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, ReadChunkMinus1) {
+  ReadTestConfig config;
+  config.total_size = internal::kNumBytesPerChunk - 1;
+
+  ReadTest(config);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, ReadChunk) {
+  ReadTestConfig config;
+  config.total_size = internal::kNumBytesPerChunk;
+
+  ReadTest(config);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, ReadChunkPlus1) {
+  ReadTestConfig config;
+  config.total_size = internal::kNumBytesPerChunk + 1;
+
+  ReadTest(config);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, ReadChunkEndingAtPage) {
+  ReadTestConfig config;
+  config.start_at = base::GetPageSize() - internal::kNumBytesPerChunk;
+  config.total_size = internal::kNumBytesPerChunk;
+
+  ReadTest(config);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, ReadChunkEndingOnePastPage) {
+  ReadTestConfig config;
+  config.start_at = base::GetPageSize() - internal::kNumBytesPerChunk + 1;
+  config.total_size = internal::kNumBytesPerChunk;
+
+  ReadTest(config);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, ReadChunkPlus1EndingOnePastPage) {
+  ReadTestConfig config;
+  config.start_at = base::GetPageSize() - internal::kNumBytesPerChunk;
+  config.total_size = internal::kNumBytesPerChunk + 1;
+
+  ReadTest(config);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, ReadChildExited) {
+  void* addr = MapPagesOrDie(1);
+  FillBufferWithPath(static_cast<char*>(addr), strlen(kPathPart) + 1, true);
+
+  base::ScopedFD parent_sync, child_sync;
+  base::CreateSocketPair(&parent_sync, &child_sync);
+
+  pid_t pid = fork();
+  if (!pid) {
+    parent_sync.reset();
+    _exit(1);
+  }
+
+  child_sync.reset();
+
+  // Wait for child to exit before reading memory.
+  char dummy_buf[1];
+  std::vector<base::ScopedFD> empty_fd_vec;
+  base::UnixDomainSocket::RecvMsg(parent_sync.get(), dummy_buf,
+                                  sizeof(dummy_buf), &empty_fd_vec);
+
+  munmap(addr, base::GetPageSize());
+
+  std::string out_str;
+  SANDBOX_ASSERT_EQ(ReadFilePathFromRemoteProcess(pid, addr, &out_str),
+                    RemoteProcessIOResult::kRemoteExited);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, BasicWrite) {
+  void* read_from = MapPagesOrDie(1);
+  const size_t write_size = base::GetPageSize();
+  FillBufferWithPath(static_cast<char*>(read_from), write_size, false);
+  char* write_to = static_cast<char*>(MapPagesOrDie(1));
+  base::ScopedFD parent_signal_fd;
+  const std::vector<int> empty_fd_vec;
+
+  pid_t pid =
+      ForkWaitingChild(base::BindLambdaForTesting([=](int child_sync_fd) {
+                         // Check correct result received and tell parent about
+                         // success.
+                         int res = memcmp(read_from, write_to, write_size);
+
+                         base::UnixDomainSocket::SendMsg(
+                             child_sync_fd, &res, sizeof(res), empty_fd_vec);
+                         _exit(1);
+                       }),
+                       &parent_signal_fd);
+
+  RemoteProcessIOResult result = WriteRemoteData(
+      pid, reinterpret_cast<uintptr_t>(write_to), write_size,
+      base::span<char>(static_cast<char*>(read_from), write_size));
+  SANDBOX_ASSERT_EQ(result, RemoteProcessIOResult::kSuccess);
+
+  // Release child.
+  char dummy_buf[1];
+  base::UnixDomainSocket::SendMsg(parent_signal_fd.get(), dummy_buf,
+                                  sizeof(dummy_buf), empty_fd_vec);
+
+  // Read result of memcmp and assert.
+  int memcmp_res;
+  std::vector<base::ScopedFD> dummy_fd_vec;
+  base::UnixDomainSocket::RecvMsg(parent_signal_fd.get(), &memcmp_res,
+                                  sizeof(memcmp_res), &dummy_fd_vec);
+  SANDBOX_ASSERT_EQ(memcmp_res, 0);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, WriteToInvalidAddress) {
+  char* write_to = static_cast<char*>(MapPagesOrDie(1));
+  MprotectLastPageOrDie(write_to, 1);
+  base::ScopedFD parent_signal_fd;
+  const std::vector<int> empty_fd_vec;
+
+  pid_t pid = ForkWaitingChild();
+  munmap(write_to, base::GetPageSize());
+
+  char buf[5];
+  memset(buf, 'a', sizeof(buf));
+  RemoteProcessIOResult result =
+      WriteRemoteData(pid, reinterpret_cast<uintptr_t>(write_to), sizeof(buf),
+                      base::span<char>(buf, sizeof(buf)));
+  SANDBOX_ASSERT_EQ(result, RemoteProcessIOResult::kRemoteMemoryInvalid);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, WritePartiallyToInvalidAddress) {
+  char* read_from = static_cast<char*>(MapPagesOrDie(2));
+  const size_t write_size = base::GetPageSize();
+  FillBufferWithPath(static_cast<char*>(read_from), write_size, false);
+  char* write_to = static_cast<char*>(MapPagesOrDie(2));
+  MprotectLastPageOrDie(write_to, 2);
+  write_to += base::GetPageSize() / 2;
+  base::ScopedFD parent_signal_fd;
+  const std::vector<int> empty_fd_vec;
+
+  pid_t pid = ForkWaitingChild();
+  munmap(write_to, base::GetPageSize());
+
+  RemoteProcessIOResult result =
+      WriteRemoteData(pid, reinterpret_cast<uintptr_t>(write_to), write_size,
+                      base::span<char>(read_from, write_size));
+  SANDBOX_ASSERT_EQ(result, RemoteProcessIOResult::kRemoteMemoryInvalid);
+}
+
+SANDBOX_TEST(BrokerRemoteSyscallArgHandler, WriteChildExited) {
+  char* addr = static_cast<char*>(MapPagesOrDie(1));
+  FillBufferWithPath(static_cast<char*>(addr), strlen(kPathPart) + 1, true);
+
+  base::ScopedFD parent_sync, child_sync;
+  base::CreateSocketPair(&parent_sync, &child_sync);
+
+  pid_t pid = fork();
+  if (!pid) {
+    parent_sync.reset();
+    _exit(1);
+  }
+
+  child_sync.reset();
+
+  // Wait for child to exit before writing memory.
+  char dummy_buf[1];
+  std::vector<base::ScopedFD> empty_fd_vec;
+  base::UnixDomainSocket::RecvMsg(parent_sync.get(), dummy_buf,
+                                  sizeof(dummy_buf), &empty_fd_vec);
+
+  std::string out_str;
+  SANDBOX_ASSERT_EQ(
+      WriteRemoteData(pid, reinterpret_cast<uintptr_t>(addr), strlen(kPathPart),
+                      base::span<char>(addr, strlen(kPathPart))),
+      RemoteProcessIOResult::kRemoteExited);
+}
+
+}  // namespace syscall_broker
+}  // namespace sandbox