Minor cleanup/formatting changes

PiperOrigin-RevId: 374164136
Change-Id: I505cbc3ac9f899ed965cde66aaae1aba55a90c64

sandboxed_api/sandbox2/executor.cc

@@ -84,7 +84,7 @@ pid_t Executor::StartSubProcess(int32_t clone_flags, const Namespace* ns,
   }
 
   // Add LD_ORIGIN_PATH to envs, as it'll make the amount of syscalls invoked by
-  // ld.so smaller. See http://b/7626303 for more details on this behavior.
+  // ld.so smaller.
   if (!path_.empty()) {
     request.add_envs(absl::StrCat("LD_ORIGIN_PATH=",
                                   file_util::fileops::StripBasename(path_)));

sandboxed_api/sandbox2/forkserver.cc

@@ -237,6 +237,7 @@ void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd,
   if (!sanitizer::GetListOfFDs(&open_fds)) {
     SAPI_RAW_LOG(WARNING, "Could not get list of current open FDs");
   }
+
   InitializeNamespaces(request, uid, gid, avoid_pivot_root);
 
   auto caps = cap_init();
@@ -320,9 +321,8 @@ pid_t ForkServer::ServeRequest() {
     if (comms_->IsTerminated()) {
       SAPI_RAW_VLOG(1, "ForkServer Comms closed. Exiting");
       exit(0);
-    } else {
-      SAPI_RAW_LOG(FATAL, "Failed to receive ForkServer request");
     }
+    SAPI_RAW_LOG(FATAL, "Failed to receive ForkServer request");
   }
   int comms_fd;
   SAPI_RAW_CHECK(comms_->RecvFD(&comms_fd), "Failed to receive Comms FD");
@@ -575,6 +575,7 @@ void ForkServer::InitializeNamespaces(const ForkRequest& request, uid_t uid,
     SAPI_RAW_PCHECK(!unshare(clone_flags),
                     "Could not create new namespaces for libunwind");
   }
+
   Namespace::InitializeNamespaces(
       uid, gid, clone_flags, Mounts(request.mount_tree()),
       request.mode() != FORKSERVER_FORK_JOIN_SANDBOX_UNWIND, request.hostname(),

sandboxed_api/sandbox2/mounts.cc

@@ -328,7 +328,7 @@ absl::Status Mounts::AddMappingsForBinary(const std::string& path,
   SAPI_RETURN_IF_ERROR(ValidateInterpreter(interpreter));
 
   std::vector<std::string> search_paths;
-  // 1. LD_LIBRARY_PRELOAD
+  // 1. LD_LIBRARY_PATH
   if (!ld_library_path.empty()) {
     std::vector<std::string> ld_library_paths =
         absl::StrSplit(ld_library_path, absl::ByAnyChar(":;"));
@@ -476,65 +476,52 @@ uint64_t GetMountFlagsFor(const std::string& path) {
     return 0;
   }
 
-  static constexpr struct {
-    const uint64_t mount_flag;
-    const uint64_t vfs_flag;
-  } mount_pairs[] = {
-      {MS_NOSUID, ST_NOSUID},         {MS_NODEV, ST_NODEV},
-      {MS_NOEXEC, ST_NOEXEC},         {MS_SYNCHRONOUS, ST_SYNCHRONOUS},
-      {MS_MANDLOCK, ST_MANDLOCK},     {MS_NOATIME, ST_NOATIME},
-      {MS_NODIRATIME, ST_NODIRATIME}, {MS_RELATIME, ST_RELATIME},
-  };
-
   uint64_t flags = 0;
-  for (const auto& i : mount_pairs) {
+  using MountPair = std::pair<uint64_t, uint64_t>;
-    if (vfs.f_flag & i.vfs_flag) {
+  for (const auto& [mount_flag, vfs_flag] : {
-      flags |= i.mount_flag;
+           MountPair(MS_NOSUID, ST_NOSUID),
+           MountPair(MS_NODEV, ST_NODEV),
+           MountPair(MS_NOEXEC, ST_NOEXEC),
+           MountPair(MS_SYNCHRONOUS, ST_SYNCHRONOUS),
+           MountPair(MS_MANDLOCK, ST_MANDLOCK),
+           MountPair(MS_NOATIME, ST_NOATIME),
+           MountPair(MS_NODIRATIME, ST_NODIRATIME),
+           MountPair(MS_RELATIME, ST_RELATIME),
+       }) {
+    if (vfs.f_flag & vfs_flag) {
+      flags |= mount_flag;
     }
   }
-
   return flags;
 }
 
 std::string MountFlagsToString(uint64_t flags) {
 #define SAPI_MAP(x) \
   { x, #x }
-  static constexpr std::pair<uint64_t, absl::string_view> map[] = {
+  static constexpr std::pair<uint64_t, absl::string_view> kMap[] = {
-      SAPI_MAP(MS_RDONLY),
+      SAPI_MAP(MS_RDONLY),      SAPI_MAP(MS_NOSUID),
-      SAPI_MAP(MS_NOSUID),
+      SAPI_MAP(MS_NODEV),       SAPI_MAP(MS_NOEXEC),
-      SAPI_MAP(MS_NODEV),
+      SAPI_MAP(MS_SYNCHRONOUS), SAPI_MAP(MS_REMOUNT),
-      SAPI_MAP(MS_NOEXEC),
+      SAPI_MAP(MS_MANDLOCK),    SAPI_MAP(MS_DIRSYNC),
-      SAPI_MAP(MS_SYNCHRONOUS),
+      SAPI_MAP(MS_NOATIME),     SAPI_MAP(MS_NODIRATIME),
-      SAPI_MAP(MS_REMOUNT),
+      SAPI_MAP(MS_BIND),        SAPI_MAP(MS_MOVE),
-      SAPI_MAP(MS_MANDLOCK),
-      SAPI_MAP(MS_DIRSYNC),
-      SAPI_MAP(MS_NOATIME),
-      SAPI_MAP(MS_NODIRATIME),
-      SAPI_MAP(MS_BIND),
-      SAPI_MAP(MS_MOVE),
       SAPI_MAP(MS_REC),
 #ifdef MS_VERBOSE
-      // MS_VERBOSE is deprecated
+      SAPI_MAP(MS_VERBOSE),  // Deprecated
-      SAPI_MAP(MS_VERBOSE),
 #endif
-      SAPI_MAP(MS_SILENT),
+      SAPI_MAP(MS_SILENT),      SAPI_MAP(MS_POSIXACL),
-      SAPI_MAP(MS_POSIXACL),
+      SAPI_MAP(MS_UNBINDABLE),  SAPI_MAP(MS_PRIVATE),
-      SAPI_MAP(MS_UNBINDABLE),
-      SAPI_MAP(MS_PRIVATE),
       SAPI_MAP(MS_SLAVE),  // Inclusive language: system constant
-      SAPI_MAP(MS_SHARED),
+      SAPI_MAP(MS_SHARED),      SAPI_MAP(MS_RELATIME),
-      SAPI_MAP(MS_RELATIME),
+      SAPI_MAP(MS_KERNMOUNT),   SAPI_MAP(MS_I_VERSION),
-      SAPI_MAP(MS_KERNMOUNT),
-      SAPI_MAP(MS_I_VERSION),
       SAPI_MAP(MS_STRICTATIME),
 #ifdef MS_LAZYTIME
-      // MS_LAZYTIME was added in Linux 4.0
+      SAPI_MAP(MS_LAZYTIME),  // Added in Linux 4.0
-      SAPI_MAP(MS_LAZYTIME),
 #endif
   };
 #undef SAPI_MAP
   std::vector<absl::string_view> flags_list;
-  for (auto [val, str] : map) {
+  for (const auto& [val, str] : kMap) {
     if ((flags & val) == val) {
       flags &= ~val;
       flags_list.push_back(str);

sandboxed_api/sandbox2/policy.h

@@ -58,6 +58,11 @@ class Policy final {
   void GetPolicyDescription(PolicyDescription* policy) const;
 
  private:
+  friend class Monitor;
+  friend class PolicyBuilder;
+  friend class PolicyBuilderPeer;  // For testing
+  friend class StackTracePeer;
+
   // Private constructor only called by the PolicyBuilder.
   Policy() = default;
 
@@ -105,11 +110,6 @@ class Policy final {
 
   // Contains a list of hosts the sandboxee is allowed to connect to.
   absl::optional<AllowedHosts> allowed_hosts_;
-
-  friend class Monitor;
-  friend class PolicyBuilder;
-  friend class PolicyBuilderPeer;  // For testing
-  friend class StackTracePeer;
 };
 
 }  // namespace sandbox2

sandboxed_api/util/BUILD.bazel

@@ -249,6 +249,7 @@ cc_library(
     copts = sapi_platform_copts(),
     deps = [
         ":fileops",
+        ":status",
         ":strerror",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",

sandboxed_api/util/CMakeLists.txt

@@ -151,6 +151,7 @@ add_library(sapi::temp_file ALIAS sapi_util_temp_file)
 target_link_libraries(sapi_util_temp_file
   PRIVATE absl::strings
           sapi::fileops
+          sapi::status
           sapi::strerror
           sapi::base
   PUBLIC absl::status

sandboxed_api/util/temp_file.cc

@@ -25,6 +25,7 @@
 #include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/util/fileops.h"
+#include "sandboxed_api/util/status_macros.h"
 #include "sandboxed_api/util/strerror.h"
 
 namespace sapi {
@@ -45,15 +46,9 @@ absl::StatusOr<std::pair<std::string, int>> CreateNamedTempFile(
 
 absl::StatusOr<std::string> CreateNamedTempFileAndClose(
     absl::string_view prefix) {
-  auto result_or = CreateNamedTempFile(prefix);
+  SAPI_ASSIGN_OR_RETURN(auto result, CreateNamedTempFile(prefix));
-  if (result_or.ok()) {
+  close(result.second);
-    std::string path;
+  return std::move(result.first);
-    int fd;
-    std::tie(path, fd) = std::move(result_or).value();
-    close(fd);
-    return path;
-  }
-  return result_or.status();
 }
 
 absl::StatusOr<std::string> CreateTempDir(absl::string_view prefix) {