diff --git a/sandboxed_api/sandbox2/BUILD.bazel b/sandboxed_api/sandbox2/BUILD.bazel index 4acfbab..d371561 100644 --- a/sandboxed_api/sandbox2/BUILD.bazel +++ b/sandboxed_api/sandbox2/BUILD.bazel @@ -384,8 +384,11 @@ cc_library( "//sandboxed_api/util:file_helpers", "//sandboxed_api/util:fileops", "//sandboxed_api/util:raw_logging", + "//sandboxed_api/util:status", "//sandboxed_api/util:strerror", "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_glog//:glog", ], @@ -415,6 +418,7 @@ cc_library( "//sandboxed_api/util:status", "//sandboxed_api/util:strerror", "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/memory", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", diff --git a/sandboxed_api/sandbox2/CMakeLists.txt b/sandboxed_api/sandbox2/CMakeLists.txt index dfee90e..448faaa 100644 --- a/sandboxed_api/sandbox2/CMakeLists.txt +++ b/sandboxed_api/sandbox2/CMakeLists.txt @@ -368,6 +368,7 @@ add_library(sandbox2_sanitizer ${SAPI_LIB_TYPE} add_library(sandbox2::sanitizer ALIAS sandbox2_sanitizer) target_link_libraries(sandbox2_sanitizer PRIVATE absl::core_headers + absl::flat_hash_set absl::strings sapi::file_helpers sapi::fileops @@ -384,6 +385,7 @@ add_library(sandbox2_forkserver ${SAPI_LIB_TYPE} add_library(sandbox2::forkserver ALIAS sandbox2_forkserver) target_link_libraries(sandbox2_forkserver PRIVATE absl::flat_hash_map + absl::flat_hash_set absl::memory absl::status absl::statusor diff --git a/sandboxed_api/sandbox2/forkserver.cc b/sandboxed_api/sandbox2/forkserver.cc index 633da28..05c81e7 100644 --- a/sandboxed_api/sandbox2/forkserver.cc +++ b/sandboxed_api/sandbox2/forkserver.cc @@ -35,6 +35,7 @@ #include #include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" #include "absl/memory/memory.h" #include "absl/status/status.h" #include "absl/status/statusor.h" @@ -107,7 +108,7 @@ void MoveFDs(std::initializer_list> move_fds, } } -void RunInitProcess(std::set open_fds) { +void RunInitProcess(const absl::flat_hash_set& open_fds) { if (prctl(PR_SET_NAME, "S2-INIT-PROC", 0, 0, 0) != 0) { SAPI_RAW_PLOG(WARNING, "prctl(PR_SET_NAME, 'S2-INIT-PROC')"); } @@ -266,9 +267,11 @@ void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd, SanitizeEnvironment(); - std::set open_fds; - if (!sanitizer::GetListOfFDs(&open_fds)) { - SAPI_RAW_LOG(WARNING, "Could not get list of current open FDs"); + absl::StatusOr> open_fds = sanitizer::GetListOfFDs(); + if (!open_fds.ok()) { + SAPI_RAW_LOG(WARNING, "Could not get list of current open FDs: %s", + std::string(open_fds.status().message()).c_str()); + open_fds = absl::flat_hash_set(); } InitializeNamespaces(request, uid, gid, avoid_pivot_root); @@ -297,7 +300,7 @@ void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd, SAPI_RAW_PLOG(FATAL, "Could not spawn init process"); } if (child != 0) { - RunInitProcess(open_fds); + RunInitProcess(*open_fds); } // Send sandboxee pid auto status = SendPid(signaling_fd); diff --git a/sandboxed_api/sandbox2/sanitizer.cc b/sandboxed_api/sandbox2/sanitizer.cc index 1321897..0dc6b42 100644 --- a/sandboxed_api/sandbox2/sanitizer.cc +++ b/sandboxed_api/sandbox2/sanitizer.cc @@ -35,6 +35,7 @@ #include #include +#include "absl/container/flat_hash_set.h" #include "absl/strings/numbers.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_split.h" @@ -42,6 +43,7 @@ #include "sandboxed_api/util/file_helpers.h" #include "sandboxed_api/util/fileops.h" #include "sandboxed_api/util/raw_logging.h" +#include "sandboxed_api/util/status_macros.h" #include "sandboxed_api/util/strerror.h" namespace sandbox2::sanitizer { @@ -52,55 +54,62 @@ namespace file_util = ::sapi::file_util; constexpr char kProcSelfFd[] = "/proc/self/fd"; // Reads filenames inside the directory and converts them to numerical values. -bool ListNumericalDirectoryEntries(const std::string& directory, - std::set* nums) { +absl::StatusOr> ListNumericalDirectoryEntries( + const std::string& directory) { + absl::flat_hash_set result; std::vector entries; std::string error; if (!file_util::fileops::ListDirectoryEntries(directory, &entries, &error)) { - SAPI_RAW_LOG(WARNING, "List directory entries for '%s' failed: %s", - kProcSelfFd, error.c_str()); - return false; + return absl::InternalError(absl::StrCat("List directory entries for '", + directory, "' failed: ", error)); } + result.reserve(entries.size()); for (const auto& entry : entries) { int num; if (!absl::SimpleAtoi(entry, &num)) { - SAPI_RAW_LOG(WARNING, "Cannot convert %s to a number", entry.c_str()); - return false; + return absl::InternalError( + absl::StrCat("Cannot convert ", entry, " to a number")); } - nums->insert(num); + result.insert(num); } - return true; + return result; } } // namespace -bool GetListOfFDs(std::set* fds) { - if (!ListNumericalDirectoryEntries(kProcSelfFd, fds)) { - return false; - } - // Exclude the dirfd which was opened in ListDirectoryEntries. - // Most probably will be the highest fd, hence the reverse order. - for (auto it = fds->rbegin(), end = fds->rend(); it != end; ++it) { +absl::StatusOr> GetListOfFDs() { + SAPI_ASSIGN_OR_RETURN(absl::flat_hash_set fds, + ListNumericalDirectoryEntries(kProcSelfFd)); + + // Exclude the dirfd which was opened in ListDirectoryEntries. + for (auto it = fds.begin(), end = fds.end(); it != end; ++it) { if (access(absl::StrCat(kProcSelfFd, "/", *it).c_str(), F_OK) != 0) { - fds->erase(*it); + fds.erase(it); break; } } - return true; + return fds; } bool GetListOfTasks(int pid, std::set* tasks) { const std::string task_dir = absl::StrCat("/proc/", pid, "/task"); - return ListNumericalDirectoryEntries(task_dir, tasks); -} - -bool CloseAllFDsExcept(const std::set& fd_exceptions) { - std::set fds; - if (!GetListOfFDs(&fds)) { + auto task_entries = ListNumericalDirectoryEntries(task_dir); + if (!task_entries.ok()) { return false; } - for (auto fd : fds) { + tasks->clear(); + tasks->insert(task_entries->begin(), task_entries->end()); + return true; +} + +bool CloseAllFDsExcept(const std::set& fd_exceptions) { + absl::StatusOr> fds = GetListOfFDs(); + if (!fds.ok()) { + return false; + } + + for (auto fd : *fds) { if (fd_exceptions.find(fd) != fd_exceptions.end()) { continue; } @@ -111,12 +120,12 @@ bool CloseAllFDsExcept(const std::set& fd_exceptions) { } bool MarkAllFDsAsCOEExcept(const std::set& fd_exceptions) { - std::set fds; - if (!GetListOfFDs(&fds)) { + auto fds = GetListOfFDs(); + if (!fds.ok()) { return false; } - for (auto fd : fds) { + for (auto fd : *fds) { if (fd_exceptions.find(fd) != fd_exceptions.end()) { continue; } diff --git a/sandboxed_api/sandbox2/sanitizer.h b/sandboxed_api/sandbox2/sanitizer.h index 048f621..1737001 100644 --- a/sandboxed_api/sandbox2/sanitizer.h +++ b/sandboxed_api/sandbox2/sanitizer.h @@ -21,12 +21,14 @@ #include #include "absl/base/macros.h" +#include "absl/container/flat_hash_set.h" +#include "absl/status/statusor.h" namespace sandbox2 { namespace sanitizer { // Reads a list of open file descriptors in the current process. -bool GetListOfFDs(std::set* fds); +absl::StatusOr> GetListOfFDs(); // Closes all file descriptors in the current process except the ones in // fd_exceptions.