Reland using shared memory in s2

PiperOrigin-RevId: 867535377
Change-Id: I131223a942549ba7f433cb88670356086c730b3b

Author: Sandboxed API Team

sandboxed_api/sandbox2/BUILD

@@ -185,7 +185,6 @@ cc_library(
         ":logserver",
         ":logsink",
         "//sandboxed_api/util:thread",
-        "@abseil-cpp//absl/base:core_headers",
         "@abseil-cpp//absl/log",
         "@abseil-cpp//absl/strings",
     ],
@@ -349,6 +348,7 @@ cc_library(
     copts = sapi_platform_copts(),
     visibility = ["//visibility:public"],
     deps = [
+        ":buffer",
         ":client",
         ":comms",
         ":executor",
@@ -374,6 +374,7 @@ cc_library(
         "//sandboxed_api/sandbox2/network_proxy:client",
         "//sandboxed_api/sandbox2/network_proxy:filtering",
         "//sandboxed_api/util:fileops",
+        "//sandboxed_api/util:status",
         "@abseil-cpp//absl/base",
         "@abseil-cpp//absl/base:core_headers",
         "@abseil-cpp//absl/container:flat_hash_map",
@@ -594,6 +595,7 @@ cc_library(
     copts = sapi_platform_copts(),
     visibility = ["//visibility:public"],
     deps = [
+        ":buffer",
         ":comms",
         ":logsink",
         ":policy",
@@ -602,10 +604,13 @@ cc_library(
         "//sandboxed_api:config",
         "//sandboxed_api/sandbox2/network_proxy:client",
         "//sandboxed_api/sandbox2/util:bpf_helper",
+        "//sandboxed_api/util:fileops",
         "//sandboxed_api/util:raw_logging",
+        "//sandboxed_api/util:status",
         "@abseil-cpp//absl/base:core_headers",
         "@abseil-cpp//absl/container:flat_hash_map",
         "@abseil-cpp//absl/status",
+        "@abseil-cpp//absl/status:statusor",
         "@abseil-cpp//absl/strings",
     ],
 )
@@ -946,7 +951,6 @@ cc_test(
     deps = [
         ":limits",
         ":sandbox2",
-        "//sandboxed_api:config",
         "//sandboxed_api:testing",
         "@googletest//:gtest_main",
     ],
@@ -989,6 +993,7 @@ cc_test(
         "//sandboxed_api/sandbox2/testcases:policy",
         "//sandboxed_api/sandbox2/testcases:posix_timers",
         "//sandboxed_api/sandbox2/testcases:sandbox_detection",
+        "//sandboxed_api/sandbox2/testcases:shared_memory",
     ],
     tags = ["no_qemu_user_mode"],
     deps = [
@@ -1015,6 +1020,7 @@ cc_test(
         "//sandboxed_api/sandbox2/testcases:abort",
         "//sandboxed_api/sandbox2/testcases:custom_fork",
         "//sandboxed_api/sandbox2/testcases:minimal",
+        "//sandboxed_api/sandbox2/testcases:shared_memory",
         "//sandboxed_api/sandbox2/testcases:sleep",
         "//sandboxed_api/sandbox2/testcases:starve",
         "//sandboxed_api/sandbox2/testcases:terminate_process_group",
@@ -1030,7 +1036,6 @@ cc_test(
         ":util",
         "//sandboxed_api:config",
         "//sandboxed_api:testing",
-        "//sandboxed_api/sandbox2/allowlists:testonly_map_exec",
         "//sandboxed_api/util:thread",
         "@abseil-cpp//absl/log",
         "@abseil-cpp//absl/status",

sandboxed_api/sandbox2/CMakeLists.txt

@@ -247,7 +247,6 @@ target_link_libraries(sandbox2_global_forkserver
           absl::cleanup
           absl::strings
           absl::status
-          absl::statusor
           absl::log
           sandbox2::client
           sandbox2::flags
@@ -336,6 +335,7 @@ target_link_libraries(sandbox2_sandbox2
          sapi::config
          sapi::fileops
          sapi::temp_file
+         sandbox2::buffer
          sandbox2::client
          sandbox2::comms
          sandbox2::executor
@@ -567,13 +567,17 @@ add_library(sandbox2::client ALIAS sandbox2_client)
 target_link_libraries(sandbox2_client
   PRIVATE absl::core_headers
           absl::strings
+          absl::statusor
           sandbox2::bpf_helper
+          sandbox2::buffer
           sandbox2::policy
           sandbox2::sanitizer
           sandbox2::syscall
           sapi::base
           sapi::config
+          sapi::fileops
           sapi::raw_logging
+          sapi::status
   PUBLIC absl::flat_hash_map
          absl::status
          sandbox2::comms

sandboxed_api/sandbox2/buffer.cc

@@ -93,8 +93,9 @@ absl::StatusOr<std::unique_ptr<Buffer>> Buffer::Expand(
 
 // Creates a new Buffer of the specified size, backed by a temporary file that
 // will be immediately deleted.
-absl::StatusOr<std::unique_ptr<Buffer>> Buffer::CreateWithSize(size_t size) {
-  absl::StatusOr<FDCloser> fd = util::CreateMemFd();
+absl::StatusOr<std::unique_ptr<Buffer>> Buffer::CreateWithSize(
+    size_t size, const char* name) {
+  absl::StatusOr<FDCloser> fd = util::CreateMemFd(name);
   if (!fd.ok()) {
     return fd.status();
   }

sandboxed_api/sandbox2/buffer.h

@@ -51,9 +51,10 @@ class Buffer final {
     return CreateFromFd(sapi::file_util::fileops::FDCloser(fd));
   }
 
-  // Creates a new Buffer of the specified size, backed by a temporary file that
-  // will be immediately deleted.
-  static absl::StatusOr<std::unique_ptr<Buffer>> CreateWithSize(size_t size);
+  // Creates a new Buffer of the specified size, backed by a temporary file
+  // (using memfd_create) that will be immediately deleted.
+  static absl::StatusOr<std::unique_ptr<Buffer>> CreateWithSize(
+      size_t size, const char* name = "buffer_file");
 
   // Expands the input buffer to the specified size.
   // Unlike CreateWithSize, this function will pre-allocate the memory.

sandboxed_api/sandbox2/client.cc

@@ -20,7 +20,9 @@
 #include <linux/bpf_common.h>
 #include <linux/filter.h>
 #include <linux/seccomp.h>
+#include <sys/mman.h>
 #include <sys/prctl.h>
+#include <sys/stat.h>
 #include <syscall.h>
 #include <unistd.h>
 
@@ -40,20 +42,24 @@
 #include "absl/base/macros.h"
 #include "absl/container/flat_hash_map.h"
 #include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_join.h"
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
 #include "sandboxed_api/config.h"
+#include "sandboxed_api/sandbox2/buffer.h"
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/sandbox2/logsink.h"
 #include "sandboxed_api/sandbox2/network_proxy/client.h"
 #include "sandboxed_api/sandbox2/policy.h"
 #include "sandboxed_api/sandbox2/sanitizer.h"
 #include "sandboxed_api/sandbox2/syscall.h"
 #include "sandboxed_api/sandbox2/util/bpf_helper.h"
+#include "sandboxed_api/util/fileops.h"
 #include "sandboxed_api/util/raw_logging.h"
+#include "sandboxed_api/util/status_macros.h"
 
 #ifndef SECCOMP_FILTER_FLAG_NEW_LISTENER
 #define SECCOMP_FILTER_FLAG_NEW_LISTENER (1UL << 3)
@@ -384,7 +390,7 @@ int Client::GetMappedFD(const std::string& name) {
   return fd;
 }
 
-bool Client::HasMappedFD(const std::string& name) {
+bool Client::HasMappedFD(const std::string& name) const {
   return fd_map_.find(name) != fd_map_.end();
 }
 
@@ -413,4 +419,24 @@ absl::Status Client::InstallNetworkProxyHandler() {
       GetNetworkProxyClient());
 }
 
+absl::StatusOr<const Buffer*> Client::GetSharedMemoryMapping() {
+  if (shared_memory_mapping_ != nullptr) {
+    return shared_memory_mapping_.get();
+  }
+
+  if (!IsSharedMemoryEnabled()) {
+    return absl::FailedPreconditionError("Shared memory is not enabled");
+  }
+
+  int shared_memory_fd = GetMappedFD("s2_shared_memory");
+  SAPI_ASSIGN_OR_RETURN(shared_memory_mapping_,
+                        Buffer::CreateFromFd(sapi::file_util::fileops::FDCloser(
+                            shared_memory_fd)));
+  return shared_memory_mapping_.get();
+}
+
+bool Client::IsSharedMemoryEnabled() const {
+  return shared_memory_mapping_ != nullptr || HasMappedFD("s2_shared_memory");
+}
+
 }  // namespace sandbox2

sandboxed_api/sandbox2/client.h

@@ -25,6 +25,8 @@
 
 #include "absl/container/flat_hash_map.h"
 #include "absl/status/status.h"
+#include "absl/status/statusor.h"
+#include "sandboxed_api/sandbox2/buffer.h"
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/sandbox2/logsink.h"
 #include "sandboxed_api/sandbox2/network_proxy/client.h"
@@ -63,7 +65,7 @@ class Client {
   // Returns the file descriptor that was mapped to the sandboxee using
   // IPC::ReceiveFd(name).
   int GetMappedFD(const std::string& name);
-  bool HasMappedFD(const std::string& name);
+  bool HasMappedFD(const std::string& name) const;
 
   // Registers a LogSink that forwards all logs to the supervisor.
   void SendLogsToSupervisor();
@@ -76,6 +78,12 @@ class Client {
   // the NetworkProxyClient class.
   absl::Status InstallNetworkProxyHandler();
 
+  // Gets the shared memory buffer if it is enabled.
+  absl::StatusOr<const Buffer*> GetSharedMemoryMapping();
+
+  // Returns true if shared memory was enabled.
+  bool IsSharedMemoryEnabled() const;
+
  protected:
   // Comms used for synchronization with the monitor, not owned by the object.
   Comms* comms_;
@@ -100,6 +108,10 @@ class Client {
   // this case that is parsed in the Client constructor if present.
   absl::flat_hash_map<std::string, int> fd_map_;
 
+  // The mapping of the shared memory region in the sandboxee's address space.
+  // This is only set if shared memory is enabled.
+  std::unique_ptr<Buffer> shared_memory_mapping_;
+
   std::string GetFdMapEnvVar() const;
 
   // Sets up communication channels with the sandbox.

sandboxed_api/sandbox2/ipc.cc

@@ -35,19 +35,36 @@ namespace sandbox2 {
 
 void IPC::SetUpServerSideComms(int fd) { comms_ = std::make_unique<Comms>(fd); }
 
-void IPC::MapFd(int local_fd, int remote_fd) {
+void IPC::MapFd(int local_fd, int remote_fd, absl::string_view name) {
   VLOG(3) << "Will send: " << local_fd << ", to overwrite: " << remote_fd;
-  fd_map_.push_back(std::make_tuple(local_fd, remote_fd, ""));
+  fd_map_.push_back(std::make_tuple(local_fd, remote_fd, std::string(name)));
 }
 
-void IPC::MapDupedFd(int local_fd, int remote_fd) {
+void IPC::MapFd(int local_fd, absl::string_view name) {
+  return MapFd(local_fd, -1, name);
+}
+
+void IPC::MapFd(int local_fd, int remote_fd) {
+  return MapFd(local_fd, remote_fd, "");
+}
+
+void IPC::MapDupedFd(int local_fd, int remote_fd, absl::string_view name) {
   const int dup_local_fd = dup(local_fd);
   if (dup_local_fd == -1) {
     PLOG(FATAL) << "dup(" << local_fd << ")";
   }
   VLOG(3) << "Will send: " << dup_local_fd << " (dup of " << local_fd
           << "), to overwrite: " << remote_fd;
-  fd_map_.push_back(std::make_tuple(dup_local_fd, remote_fd, ""));
+  fd_map_.push_back(
+      std::make_tuple(dup_local_fd, remote_fd, std::string(name)));
+}
+
+void IPC::MapDupedFd(int local_fd, absl::string_view name) {
+  return MapDupedFd(local_fd, -1, name);
+}
+
+void IPC::MapDupedFd(int local_fd, int remote_fd) {
+  return MapDupedFd(local_fd, remote_fd, "");
 }
 
 int IPC::ReceiveFd(int remote_fd) { return ReceiveFd(remote_fd, ""); }

sandboxed_api/sandbox2/ipc.h

@@ -47,12 +47,30 @@ class IPC final {
   // being sent (in SendFdsOverComms() which is called by the Monitor class when
   // Sandbox2::RunAsync() is called), so local_fd should not be used from that
   // point on. The application must not close local_fd after calling MapFd().
+  // If a name is specified, users can use Client::GetMappedFD api to retrieve
+  // the fd in the sandboxee.
+  void MapFd(int local_fd, int remote_fd, absl::string_view name);
+
+  // Similar to MapFd() above, except that we do not name the remote_fd.
   void MapFd(int local_fd, int remote_fd);
 
+  // Similar to MapFd() above, except that we do not pin the remote_fd in the
+  // sandboxee.
+  void MapFd(int local_fd, absl::string_view name);
+
   // Similar to MapFd(), except local_fd remains available for use in the
   // application even after Sandbox2::RunAsync() is called; the application
   // retains responsibility for closing local_fd and may do so at any time after
   // calling MapDupedFd().
+  // If a name is specified, users can use Client::GetMappedFD api to retrieve
+  // the fd in the sandboxee.
+  void MapDupedFd(int local_fd, int remote_fd, absl::string_view name);
+
+  // Similar to MapDupedFd() above, except that we do not pin the remote_fd in
+  // the sandboxee.
+  void MapDupedFd(int local_fd, absl::string_view name);
+
+  // Similar to MapDupedFd() above, except that we do not name the remote_fd.
   void MapDupedFd(int local_fd, int remote_fd);
 
   // Creates and returns a socketpair endpoint. The other endpoint of the

sandboxed_api/sandbox2/policy_test.cc

@@ -582,6 +582,46 @@ TEST_P(PolicyTest, OverridablePolicyOnSyscallsWorks) {
   EXPECT_THAT(result.reason_code(), Eq(0));
 }
 
+TEST_P(PolicyTest, InstallSharedMemoryWithWrongPolicy) {
+  SKIP_SANITIZERS;
+  // The policy doesn't allow installing shared memory, so the sandboxee won't
+  // be able to map the shared memory region.
+  sandbox2::PolicyBuilder builder;
+  builder.AllowStaticStartup();
+  builder.AllowMmapWithoutExec();
+  builder.AllowTcMalloc();
+  builder.AllowExit();
+  auto s2 = CreateTestSandbox({"shared_memory", "2"}, builder);
+  ASSERT_THAT(s2->CreateSharedMemoryMapping(100ULL << 20), IsOk());
+  auto result = s2->Run();
+
+  ASSERT_EQ(result.final_status(), sandbox2::Result::VIOLATION);
+  EXPECT_EQ(result.reason_code(), sandbox2::Result::VIOLATION_SYSCALL);
+  ASSERT_TRUE(result.GetSyscall() != nullptr);
+  EXPECT_THAT(result.GetSyscall()->nr(), Eq(__NR_fstat));
+}
+
+TEST_P(PolicyTest, InstallSharedMemoryWithMinimalPolicy) {
+  SKIP_SANITIZERS;
+  // The policy allows installing shared memory, so the sandboxee will be able
+  // to map the shared memory region, even though we enabled sandbox before
+  // execve.
+  sandbox2::PolicyBuilder builder;
+  builder.AllowStaticStartup();
+  builder.AllowSharedMemory();
+  builder.AllowTcMalloc();
+  builder.AllowExit();
+  auto s2 = CreateTestSandbox({"shared_memory", "2"}, builder);
+  SAPI_ASSERT_OK_AND_ASSIGN(auto res,
+                            s2->CreateSharedMemoryMapping(100ULL << 20));
+  res->data()[0] = 'Z';
+  auto result = s2->Run();
+
+  ASSERT_EQ(result.final_status(), sandbox2::Result::OK);
+  EXPECT_EQ(result.reason_code(), 0);
+  EXPECT_EQ(res->data()[0], 'A');
+}
+
 INSTANTIATE_TEST_SUITE_P(Sandbox2, PolicyTest, ::testing::Values(false, true),
                          [](const ::testing::TestParamInfo<bool>& info) {
                            return info.param ? "UnotifyMonitor"

sandboxed_api/sandbox2/policybuilder.cc

@@ -1461,6 +1461,27 @@ PolicyBuilder& PolicyBuilder::AllowDynamicStartup(MapExec) {
   });
 }
 
+PolicyBuilder& PolicyBuilder::AllowSharedMemory() {
+  if (allowed_complex_.shared_memory) {
+    return *this;
+  }
+  allowed_complex_.shared_memory = true;
+  AddPolicyOnMmap([](bpf_labels& labels) -> std::vector<sock_filter> {
+    return {
+        ARG_32(2),  // prot
+        JNE32(PROT_READ | PROT_WRITE, JUMP(&labels, mmap_end)),
+
+        ARG_32(3),  // flags
+        JEQ32(MAP_SHARED, ALLOW),
+
+        LABEL(&labels, mmap_end),
+    };
+  });
+  AllowSyscalls({__NR_munmap, __NR_close});
+  AllowStat();
+  return *this;
+}
+
 PolicyBuilder& PolicyBuilder::AddPolicyOnSyscall(
     uint32_t num, absl::Span<const sock_filter> policy) {
   return AddPolicyOnSyscalls({num}, policy);