Use sapi::file::GetContents() and light Mini-ELF refactoring

Plus some style fixes.

PiperOrigin-RevId: 370901533
Change-Id: If4f9d7c3157fdfc2ca4302b06cd95e96e7a8ebdd

sandboxed_api/sandbox2/unwind/BUILD.bazel

@@ -57,6 +57,7 @@ cc_library(
         "//sandboxed_api/sandbox2:comms",
         "//sandboxed_api/sandbox2/util:maps_parser",
         "//sandboxed_api/sandbox2/util:minielf",
+        "//sandboxed_api/util:file_helpers",
         "//sandboxed_api/util:raw_logging",
         "//sandboxed_api/util:strerror",
         "@com_google_absl//absl/strings",

sandboxed_api/sandbox2/unwind/CMakeLists.txt

@@ -30,6 +30,7 @@ add_library(sandbox2_unwind STATIC
 )
 add_library(sandbox2::unwind ALIAS sandbox2_unwind)
 target_link_libraries(sandbox2_unwind PRIVATE
+  absl::strings
   sandbox2::comms
   sandbox2::maps_parser
   sandbox2::minielf
@@ -37,6 +38,7 @@ target_link_libraries(sandbox2_unwind PRIVATE
   sapi::strerror
   sandbox2::unwind_proto
   sapi::base
+  sapi::file_helpers
   sapi::raw_logging
   unwind::unwind_ptrace_wrapped
 )

sandboxed_api/sandbox2/unwind/unwind.cc

@@ -32,6 +32,7 @@
 #include "sandboxed_api/sandbox2/unwind/unwind.pb.h"
 #include "sandboxed_api/sandbox2/util/maps_parser.h"
 #include "sandboxed_api/sandbox2/util/minielf.h"
+#include "sandboxed_api/util/file_helpers.h"
 #include "sandboxed_api/util/raw_logging.h"
 #include "sandboxed_api/util/strerror.h"
 
@@ -40,11 +41,13 @@ namespace {
 
 std::string DemangleSymbol(const std::string& maybe_mangled) {
   int status;
-  std::unique_ptr<char, std::function<void(char*)>> symbol = {
+  size_t length;
-      abi::__cxa_demangle(maybe_mangled.c_str(), nullptr, nullptr, &status),
+  std::unique_ptr<char, decltype(&std::free)> symbol(
-      free};
+      abi::__cxa_demangle(maybe_mangled.c_str(), /*output_buffer=*/nullptr,
+                          &length, &status),
+      std::free);
   if (symbol && status == 0) {
-    return symbol.get();
+    return std::string(symbol.get(), length);
   }
   return maybe_mangled;
 }
@@ -96,7 +99,7 @@ std::vector<uintptr_t> GetIPList(pid_t pid, int max_frames) {
   }
 
   std::vector<uintptr_t> ips;
-  for (int i = 0; i < max_frames; i++) {
+  for (int i = 0; i < max_frames; ++i) {
     unw_word_t ip;
     rc = unw_get_reg(&cursor, UNW_REG_IP, &ip);
     if (rc < 0) {
@@ -138,36 +141,20 @@ bool RunLibUnwindAndSymbolizer(Comms* comms) {
 std::vector<std::string> RunLibUnwindAndSymbolizer(pid_t pid,
                                                    std::vector<uintptr_t>* ips,
                                                    int max_frames) {
-  // Run libunwind.
+  *ips = GetIPList(pid, max_frames);  // Uses libunwind
-  *ips = GetIPList(pid, max_frames);
 
-  // Open /proc/pid/maps.
+  const std::string maps_filename = absl::StrCat("/proc/", pid, "/maps");
-  std::string path_maps = absl::StrCat("/proc/", pid, "/maps");
+  std::string maps_content;
-  std::unique_ptr<FILE, void (*)(FILE*)> f(fopen(path_maps.c_str(), "r"),
+  if (auto status = sapi::file::GetContents(maps_filename, &maps_content,
-                                           [](FILE* s) {
+                                            sapi::file::Defaults());
-                                             if (s) {
+      !status.ok()) {
-                                               fclose(s);
+    SAPI_RAW_LOG(ERROR, "%s", status.ToString().c_str());
-                                             }
-                                           });
-  if (!f) {
-    // Could not open maps file.
-    SAPI_RAW_LOG(ERROR, "Could not open %s", path_maps.c_str());
     return {};
   }
 
-  constexpr static size_t kBufferSize = 10 * 1024 * 1024;
-  std::string maps_content(kBufferSize, '\0');
-  size_t bytes_read = fread(&maps_content[0], 1, kBufferSize, f.get());
-  if (bytes_read == 0) {
-    // Could not read the whole maps file.
-    SAPI_RAW_PLOG(ERROR, "Could not read maps file");
-    return {};
-  }
-  maps_content.resize(bytes_read);
-
   auto maps = ParseProcMaps(maps_content);
   if (!maps.ok()) {
-    SAPI_RAW_LOG(ERROR, "Could not parse /proc/%d/maps", pid);
+    SAPI_RAW_LOG(ERROR, "Could not parse file: %s", maps_filename.c_str());
     return {};
   }
 

sandboxed_api/sandbox2/util/minielf.cc

@@ -69,7 +69,7 @@ absl::Status CheckedFSeek(FILE* f, long offset, int whence) {
 }
 
 absl::Status CheckedFRead(void* dst, size_t size, size_t nmemb, FILE* f) {
-  if (fread(dst, size, nmemb, f) == nmemb) {
+  if (std::fread(dst, size, nmemb, f) == nmemb) {
     return absl::OkStatus();
   }
   return absl::FailedPreconditionError(
@@ -104,10 +104,18 @@ class ElfParser {
   static constexpr int kMaxDynamicEntries = 10000;
   static constexpr size_t kMaxInterpreterSize = 1000;
 
-  ElfParser() = default;
+  static absl::StatusOr<ElfFile> Parse(const std::string& filename,
-  absl::StatusOr<ElfFile> Parse(FILE* elf, uint32_t features);
+                                       uint32_t features);
+
+  ~ElfParser() {
+    if (elf_) {
+      std::fclose(elf_);
+    }
+  }
 
  private:
+  ElfParser() = default;
+
   // Endianess support functions
   uint16_t Load16(const void* src) {
     return elf_little_ ? absl::little_endian::Load16(src)
@@ -121,6 +129,7 @@ class ElfParser {
     return elf_little_ ? absl::little_endian::Load64(src)
                        : absl::big_endian::Load64(src);
   }
+
   template <size_t N>
   void Load(unsigned char (*dst)[N], const void* src) {
     memcpy(dst, src, N);
@@ -138,19 +147,19 @@ class ElfParser {
   void Load(int32_t* dst, const void* src) { *dst = Load32(src); }
   void Load(int64_t* dst, const void* src) { *dst = Load64(src); }
 
-  // Reads elf file size.
+  // Reads ELF file size.
   absl::Status ReadFileSize();
-  // Reads elf header.
+  // Reads ELF header.
   absl::Status ReadFileHeader();
-  // Reads a single elf program header.
+  // Reads a single ELF program header.
   absl::StatusOr<ElfPhdr> ReadProgramHeader(absl::string_view src);
-  // Reads all elf program headers.
+  // Reads all ELF program headers.
   absl::Status ReadProgramHeaders();
-  // Reads a single elf section header.
+  // Reads a single ELF section header.
   absl::StatusOr<ElfShdr> ReadSectionHeader(absl::string_view src);
-  // Reads all elf section headers.
+  // Reads all ELF section headers.
   absl::Status ReadSectionHeaders();
-  // Reads contents of an elf section.
+  // Reads contents of an ELF section.
   absl::StatusOr<std::string> ReadSectionContents(int idx);
   absl::StatusOr<std::string> ReadSectionContents(
       const ElfShdr& section_header);
@@ -171,18 +180,9 @@ class ElfParser {
   int dynamic_entries_read = 0;
 };
 
-constexpr int ElfParser::kMaxProgramHeaderEntries;
-constexpr int ElfParser::kMaxSectionHeaderEntries;
-constexpr size_t ElfParser::kMaxSectionSize;
-constexpr size_t ElfParser::kMaxStrtabSize;
-constexpr size_t ElfParser::kMaxLibPathSize;
-constexpr int ElfParser::kMaxSymbolEntries;
-constexpr int ElfParser::kMaxDynamicEntries;
-constexpr size_t ElfParser::kMaxInterpreterSize;
-
 absl::Status ElfParser::ReadFileSize() {
-  fseek(elf_, 0, SEEK_END);
+  std::fseek(elf_, 0, SEEK_END);
-  file_size_ = ftell(elf_);
+  file_size_ = std::ftell(elf_);
   if (file_size_ < kElfHeaderSize) {
     return absl::FailedPreconditionError(
         absl::StrCat("file too small: ", file_size_, " bytes, at least ",
@@ -459,80 +459,79 @@ absl::Status ElfParser::ReadImportedLibrariesFromDynamic(
     auto offset = strtab_section.sh_offset + dyn.d_un.d_val;
     SAPI_RETURN_IF_ERROR(CheckedFSeek(elf_, offset, SEEK_SET));
     std::string path(std::min(kMaxLibPathSize, strtab_end - offset), '\0');
-    size_t size = fread(&path[0], 1, path.size(), elf_);
+    size_t size = std::fread(&path[0], 1, path.size(), elf_);
     path.resize(size);
     result_.imported_libraries_.push_back(path.substr(0, path.find('\0')));
   }
   return absl::OkStatus();
 }
 
-absl::StatusOr<ElfFile> ElfParser::Parse(FILE* elf, uint32_t features) {
+absl::StatusOr<ElfFile> ElfParser::Parse(const std::string& filename,
-  elf_ = elf;
+                                         uint32_t features) {
+  ElfParser parser;
+  if (parser.elf_ = std::fopen(filename.c_str(), "r"); !parser.elf_) {
+    return absl::UnknownError(
+        absl::StrCat("cannot open file: ", filename, ": ", StrError(errno)));
+  }
+
   // Basic sanity check.
   if (features & ~(ElfFile::kAll)) {
     return absl::InvalidArgumentError("Unknown feature flags specified");
   }
-  SAPI_RETURN_IF_ERROR(ReadFileSize());
+  SAPI_RETURN_IF_ERROR(parser.ReadFileSize());
-  SAPI_RETURN_IF_ERROR(ReadFileHeader());
+  SAPI_RETURN_IF_ERROR(parser.ReadFileHeader());
-  switch (file_header_.e_type) {
+  switch (parser.file_header_.e_type) {
     case ET_EXEC:
-      result_.position_independent_ = false;
+      parser.result_.position_independent_ = false;
       break;
     case ET_DYN:
-      result_.position_independent_ = true;
+      parser.result_.position_independent_ = true;
       break;
     default:
       return absl::FailedPreconditionError("not an executable: ");
   }
   if (features & ElfFile::kGetInterpreter) {
-    SAPI_RETURN_IF_ERROR(ReadProgramHeaders());
+    SAPI_RETURN_IF_ERROR(parser.ReadProgramHeaders());
     std::string interpreter;
     auto it = std::find_if(
-        program_headers_.begin(), program_headers_.end(),
+        parser.program_headers_.begin(), parser.program_headers_.end(),
         [](const ElfPhdr& hdr) { return hdr.p_type == PT_INTERP; });
     // No interpreter usually means that the executable was statically linked.
-    if (it != program_headers_.end()) {
+    if (it != parser.program_headers_.end()) {
       if (it->p_filesz > kMaxInterpreterSize) {
         return absl::FailedPreconditionError(
             absl::StrCat("program interpeter path too long: ", it->p_filesz));
       }
-      SAPI_RETURN_IF_ERROR(CheckedFSeek(elf, it->p_offset, SEEK_SET));
+      SAPI_RETURN_IF_ERROR(CheckedFSeek(parser.elf_, it->p_offset, SEEK_SET));
       interpreter.resize(it->p_filesz, '\0');
-      SAPI_RETURN_IF_ERROR(CheckedRead(&interpreter, elf));
+      SAPI_RETURN_IF_ERROR(CheckedRead(&interpreter, parser.elf_));
       auto first_nul = interpreter.find_first_of('\0');
       if (first_nul != std::string::npos) {
         interpreter.erase(first_nul);
       }
     }
-    result_.interpreter_ = std::move(interpreter);
+    parser.result_.interpreter_ = std::move(interpreter);
   }
 
   if (features & (ElfFile::kLoadSymbols | ElfFile::kLoadImportedLibraries)) {
-    SAPI_RETURN_IF_ERROR(ReadSectionHeaders());
+    SAPI_RETURN_IF_ERROR(parser.ReadSectionHeaders());
-    for (const auto& hdr : section_headers_) {
+    for (const auto& hdr : parser.section_headers_) {
       if (hdr.sh_type == SHT_SYMTAB && features & ElfFile::kLoadSymbols) {
-        SAPI_RETURN_IF_ERROR(ReadSymbolsFromSymtab(hdr));
+        SAPI_RETURN_IF_ERROR(parser.ReadSymbolsFromSymtab(hdr));
       }
       if (hdr.sh_type == SHT_DYNAMIC &&
           features & ElfFile::kLoadImportedLibraries) {
-        SAPI_RETURN_IF_ERROR(ReadImportedLibrariesFromDynamic(hdr));
+        SAPI_RETURN_IF_ERROR(parser.ReadImportedLibrariesFromDynamic(hdr));
       }
     }
   }
 
-  return std::move(result_);
+  return std::move(parser.result_);
 }
 
 absl::StatusOr<ElfFile> ElfFile::ParseFromFile(const std::string& filename,
                                                uint32_t features) {
-  std::unique_ptr<FILE, void (*)(FILE*)> elf{fopen(filename.c_str(), "r"),
+  return ElfParser::Parse(filename, features);
-                                             [](FILE* f) { fclose(f); }};
-  if (!elf) {
-    return absl::UnknownError(
-        absl::StrCat("cannot open file: ", filename, ": ", StrError(errno)));
-  }
-
-  return ElfParser().Parse(elf.get(), features);
 }
 
 }  // namespace sandbox2

sandboxed_api/sandbox2/util/minielf.h

@@ -33,6 +33,12 @@ class ElfFile {
     std::string name;
   };
 
+  static constexpr uint32_t kGetInterpreter = 1 << 0;
+  static constexpr uint32_t kLoadSymbols = 1 << 1;
+  static constexpr uint32_t kLoadImportedLibraries = 1 << 2;
+  static constexpr uint32_t kAll =
+      kGetInterpreter | kLoadSymbols | kLoadImportedLibraries;
+
   static absl::StatusOr<ElfFile> ParseFromFile(const std::string& filename,
                                                uint32_t features);
 
@@ -44,12 +50,6 @@ class ElfFile {
   }
   bool position_independent() const { return position_independent_; }
 
-  static constexpr uint32_t kGetInterpreter = 1 << 0;
-  static constexpr uint32_t kLoadSymbols = 1 << 1;
-  static constexpr uint32_t kLoadImportedLibraries = 1 << 2;
-  static constexpr uint32_t kAll =
-      kGetInterpreter | kLoadSymbols | kLoadImportedLibraries;
-
  private:
   friend class ElfParser;