Replace GetNode with ResolvePath in Mounts

Now unwinding will properly handle binaries inside bind-mounted directories.

Drive-by:
 - Get rid of n^2 path handling
 - Get rid of namespace alias
PiperOrigin-RevId: 358353666
Change-Id: Ieec7690ec6a1ae6d358de375220566b69e8cb094

cmake/abseil-cpp.cmake

@@ -16,7 +16,7 @@ set(workdir "${CMAKE_BINARY_DIR}/_deps/absl-populate")
 
 set(SAPI_ABSL_GIT_REPOSITORY https://github.com/abseil/abseil-cpp.git
                              CACHE STRING "")
-set(SAPI_ABSL_GIT_TAG 4fd9a1ec5077daac14eeee05df931d658ec0b7b8
+set(SAPI_ABSL_GIT_TAG b315753c0b8b4aa4e3e1479375eddb518393bab6
                       CACHE STRING "") # 2020-11-19
 set(SAPI_ABSL_SOURCE_DIR "${CMAKE_BINARY_DIR}/_deps/absl-src" CACHE STRING "")
 set(SAPI_ABSL_BINARY_DIR "${CMAKE_BINARY_DIR}/_deps/absl-build" CACHE STRING "")

cmake/protobuf.cmake

@@ -17,7 +17,7 @@ set(workdir "${CMAKE_BINARY_DIR}/_deps/protobuf-populate")
 set(SAPI_PROTOBUF_GIT_REPOSITORY
   https://github.com/protocolbuffers/protobuf.git
   CACHE STRING "")
-set(SAPI_PROTOBUF_GIT_TAG v3.11.4 CACHE STRING "") # 2020-02-14
+set(SAPI_PROTOBUF_GIT_TAG v3.14.0 CACHE STRING "") # 2020-11-14
 set(SAPI_PROTOBUF_SOURCE_DIR "${CMAKE_BINARY_DIR}/_deps/protobuf-src"
                              CACHE STRING "")
 set(SAPI_PROTOBUF_BINARY_DIR "${CMAKE_BINARY_DIR}/_deps/protobuf-build"

sandboxed_api/bazel/sapi_deps.bzl

@@ -34,9 +34,9 @@ def sapi_deps():
     maybe(
         http_archive,
         name = "com_google_absl",
-        sha256 = "59d1ada90ee43dd514c659246d24f7edc003eede0f3243f030f0daa03371e458",  # 2020-11-19
+        sha256 = "e140988c4d3c22f829a3095f0d34a0783aa2f8829556283f10b8eb63a9428b19",  # 2021-02-19
-        strip_prefix = "abseil-cpp-4fd9a1ec5077daac14eeee05df931d658ec0b7b8",
+        strip_prefix = "abseil-cpp-b315753c0b8b4aa4e3e1479375eddb518393bab6",
-        urls = ["https://github.com/abseil/abseil-cpp/archive/4fd9a1ec5077daac14eeee05df931d658ec0b7b8.zip"],
+        urls = ["https://github.com/abseil/abseil-cpp/archive/b315753c0b8b4aa4e3e1479375eddb518393bab6.zip"],
     )
     maybe(
         http_archive,
@@ -83,9 +83,9 @@ def sapi_deps():
     maybe(
         http_archive,
         name = "com_google_protobuf",
-        sha256 = "9748c0d90e54ea09e5e75fb7fac16edce15d2028d4356f32211cfa3c0e956564",  # 2020-02-14
+        sha256 = "bf0e5070b4b99240183b29df78155eee335885e53a8af8683964579c214ad301",  # 2020-11-14
-        strip_prefix = "protobuf-3.11.4",
+        strip_prefix = "protobuf-3.14.0",
-        urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.11.4.zip"],
+        urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.14.0.zip"],
     )
 
     # libcap

sandboxed_api/sandbox2/CMakeLists.txt

@@ -423,12 +423,8 @@ add_library(sandbox2_mounts ${SAPI_LIB_TYPE}
 )
 add_library(sandbox2::mounts ALIAS sandbox2_mounts)
 target_link_libraries(sandbox2_mounts
-  PRIVATE absl::core_headers
+  PRIVATE absl::flat_hash_set
-          absl::flat_hash_set
-          absl::status
-          absl::statusor
           absl::str_format
-          absl::strings
           protobuf::libprotobuf
           sapi::config
           sapi::file_base
@@ -438,7 +434,11 @@ target_link_libraries(sandbox2_mounts
           sapi::base
           sapi::raw_logging
           sapi::status
-  PUBLIC sandbox2::mounttree_proto
+  PUBLIC absl::core_headers
+         absl::status
+         absl::statusor
+         absl::strings
+         sandbox2::mounttree_proto
 )
 
 # sandboxed_api/sandbox2:namespace

sandboxed_api/sandbox2/mounts.cc

@@ -34,6 +34,7 @@
 #include "absl/strings/str_join.h"
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
+#include "absl/strings/strip.h"
 #include "sandboxed_api/config.h"
 #include "sandboxed_api/sandbox2/util/minielf.h"
 #include "sandboxed_api/util/fileops.h"
@@ -46,12 +47,11 @@ namespace sandbox2 {
 namespace {
 
 namespace cpu = ::sapi::cpu;
-namespace file = ::sapi::file;
 namespace file_util = ::sapi::file_util;
 namespace host_cpu = ::sapi::host_cpu;
 
 bool PathContainsNullByte(absl::string_view path) {
-  return path.find('\x00') != absl::string_view::npos;
+  return absl::StrContains(path, '\x00');
 }
 
 bool IsSameFile(const std::string& path1, const std::string& path2) {
@@ -105,7 +105,8 @@ absl::string_view GetOutsidePath(const MountTree::Node& node) {
 
 absl::StatusOr<std::string> ExistingPathInsideDir(
     absl::string_view dir_path, absl::string_view relative_path) {
-  auto path = file::CleanPath(file::JoinPath(dir_path, relative_path));
+  auto path =
+      sapi::file::CleanPath(sapi::file::JoinPath(dir_path, relative_path));
   if (file_util::fileops::StripBasename(path) != dir_path) {
     return absl::InvalidArgumentError("Relative path goes above the base dir");
   }
@@ -192,9 +193,9 @@ absl::Status Mounts::Insert(absl::string_view path,
       break;
   }
 
-  std::string fixed_path = file::CleanPath(path);
+  std::string fixed_path = sapi::file::CleanPath(path);
 
-  if (!file::IsAbsolutePath(fixed_path)) {
+  if (!sapi::file::IsAbsolutePath(fixed_path)) {
     return absl::InvalidArgumentError("Only absolute paths are supported");
   }
 
@@ -202,22 +203,15 @@ absl::Status Mounts::Insert(absl::string_view path,
     return absl::InvalidArgumentError("The root already exists");
   }
 
-  std::vector<absl::string_view> parts;
+  std::vector<absl::string_view> parts =
-
+      absl::StrSplit(absl::StripPrefix(fixed_path, "/"), '/');
-  auto split = file::SplitPath(fixed_path);
+  std::string final_part(parts.back());
-  absl::string_view cur = split.first;
+  parts.pop_back();
-  auto final_part = std::string(split.second);
-
-  while (cur != "/") {
-    auto split = file::SplitPath(cur);
-    cur = split.first;
-    parts.push_back(split.second);
-  }
 
   MountTree* curtree = &mount_tree_;
-  for (auto part = parts.rbegin(); part != parts.rend(); ++part) {
+  for (absl::string_view part : parts) {
     curtree = &(curtree->mutable_entries()
-                    ->insert({std::string(*part), MountTree()})
+                    ->insert({std::string(part), MountTree()})
                     .first->second);
     if (curtree->has_node() && curtree->node().has_file_node()) {
       return absl::FailedPreconditionError(
@@ -272,26 +266,38 @@ absl::Status Mounts::AddDirectoryAt(absl::string_view outside,
   return Insert(inside, node);
 }
 
-const MountTree::Node* Mounts::GetNode(const std::string& path) const {
+absl::StatusOr<std::string> Mounts::ResolvePath(absl::string_view path) const {
-  std::string fixed_path = file::CleanPath(path);
+  if (!sapi::file::IsAbsolutePath(path)) {
-  absl::string_view cur = fixed_path;
+    return absl::InvalidArgumentError("Path has to be absolute");
-  std::vector<std::string> parts;
-
-  while (cur != "/") {
-    auto split = file::SplitPath(cur);
-    cur = split.first;
-    parts.push_back(std::string(split.second));
   }
+  std::string fixed_path = sapi::file::CleanPath(path);
+  absl::string_view tail = absl::StripPrefix(fixed_path, "/");
 
   const MountTree* curtree = &mount_tree_;
-  for (auto part = parts.rbegin(); part != parts.rend(); ++part) {
+  while (!tail.empty()) {
-    const auto& p = curtree->entries().find(*part);
+    std::pair<absl::string_view, absl::string_view> parts =
-    if (p == curtree->entries().end()) {
+        absl::StrSplit(tail, absl::MaxSplits('/', 1));
-      return nullptr;
+    absl::string_view cur = parts.first;
+    tail = parts.second;
+    const auto it = curtree->entries().find(cur);
+    if (it == curtree->entries().end()) {
+      if (curtree->node().has_dir_node()) {
+        return sapi::file::JoinPath(curtree->node().dir_node().outside(), tail);
+      }
+      return absl::NotFoundError("Path could not be resolved in the mounts");
     }
-    curtree = &p->second;
+    curtree = &it->second;
   }
-  return &curtree->node();
+  switch (curtree->node().node_case()) {
+    case MountTree::Node::kFileNode:
+    case MountTree::Node::kDirNode:
+      return std::string(GetOutsidePath(curtree->node()));
+    case MountTree::Node::kRootNode:
+    case MountTree::Node::kTmpfsNode:
+    case MountTree::Node::NODE_NOT_SET:
+      break;
+  }
+  return absl::NotFoundError("Path could not be resolved in the mounts");
 }
 
 namespace {
@@ -347,7 +353,7 @@ absl::Status Mounts::AddMappingsForBinary(const std::string& path,
       std::string path = search_path;
       for (int hw_cap = 0; hw_cap < hw_cap_paths.size(); ++hw_cap) {
         if ((hw_caps_set & (1 << hw_cap)) != 0) {
-          path = file::JoinPath(path, hw_cap_paths[hw_cap]);
+          path = sapi::file::JoinPath(path, hw_cap_paths[hw_cap]);
         }
       }
       if (file_util::fileops::Exists(path, /*fully_resolve=*/false)) {
@@ -647,7 +653,7 @@ void CreateMounts(const MountTree& tree, const std::string& path,
 
   // Traverse the subtrees.
   for (const auto& kv : tree.entries()) {
-    std::string new_path = file::JoinPath(path, kv.first);
+    std::string new_path = sapi::file::JoinPath(path, kv.first);
     CreateMounts(kv.second, new_path, create_backing_files);
   }
 }

sandboxed_api/sandbox2/mounts.h

@@ -20,6 +20,7 @@
 
 #include "absl/base/attributes.h"
 #include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "sandboxed_api/sandbox2/mounttree.pb.h"
 
@@ -76,7 +77,7 @@ class Mounts {
   void RecursivelyListMounts(std::vector<std::string>* outside_entries,
                              std::vector<std::string>* inside_entries);
 
-  const MountTree::Node* GetNode(const std::string& path) const;
+  absl::StatusOr<std::string> ResolvePath(absl::string_view path) const;
 
  private:
   friend class MountTreeTest;

sandboxed_api/sandbox2/stack_trace.cc

@@ -217,13 +217,8 @@ bool StackTracePeer::LaunchLibunwindSandbox(const Regs* regs,
 
   // The exe_path will have a mountable path of the application, even if it was
   // removed.
-  std::string exe_path;
-
   // Resolve app_path backing file.
-  const auto* app_node = mounts.GetNode(app_path);
+  std::string exe_path = mounts.ResolvePath(app_path).value_or("");
-  if (app_node) {
-    exe_path = app_node->file_node().outside();
-  }
 
   if (exe_path.empty()) {
     // File was probably removed.