Simpleperf: check if elf file paths are valid before reading them.

Reading invalid file paths can cause troubles. For example,
reading /dev/zero can eat all memory.

Bug: 25194400
Change-Id: I4de17f4f9200a43b50a4efe4c42a6fd9eaec1ba6

diff --git a/simpleperf/read_elf.cpp b/simpleperf/read_elf.cpp
index 9a4f4b9..4774ea3 100644 (file)
--- a/simpleperf/read_elf.cpp
+++ b/simpleperf/read_elf.cpp
@@ -40,6 +40,22 @@
 #define ELF_NOTE_GNU "GNU"
 #define NT_GNU_BUILD_ID 3
 
+bool IsValidElfPath(const std::string& filename) {
+  static const char elf_magic[] = {0x7f, 'E', 'L', 'F'};
+
+  if (!IsRegularFile(filename)) {
+    return false;
+  }
+  FILE* fp = fopen(filename.c_str(), "rb");
+  char buf[4];
+  if (fread(buf, 4, 1, fp) != 1) {
+    fclose(fp);
+    return false;
+  }
+  fclose(fp);
+  return memcmp(buf, elf_magic, 4) == 0;
+}
+
 static bool GetBuildIdFromNoteSection(const char* section, size_t section_size, BuildId* build_id) {
   const char* p = section;
   const char* end = p + section_size;
@@ -112,6 +128,9 @@ static bool GetBuildIdFromObjectFile(llvm::object::ObjectFile* obj, BuildId* bui
 }
 
 bool GetBuildIdFromElfFile(const std::string& filename, BuildId* build_id) {
+  if (!IsValidElfPath(filename)) {
+    return false;
+  }
   auto owning_binary = llvm::object::createBinary(llvm::StringRef(filename));
   if (owning_binary.getError()) {
     PLOG(DEBUG) << "can't open file " << filename;
@@ -227,6 +246,9 @@ static llvm::object::ObjectFile* GetObjectFile(
 
 bool ParseSymbolsFromElfFile(const std::string& filename, const BuildId& expected_build_id,
                              std::function<void(const ElfFileSymbol&)> callback) {
+  if (!IsValidElfPath(filename)) {
+    return false;
+  }
   auto owning_binary = llvm::object::createBinary(llvm::StringRef(filename));
   llvm::object::ObjectFile* obj = GetObjectFile(owning_binary, filename, expected_build_id);
   if (obj == nullptr) {
@@ -265,6 +287,9 @@ bool ReadMinExecutableVirtualAddress(const llvm::object::ELFFile<ELFT>* elf, uin
 bool ReadMinExecutableVirtualAddressFromElfFile(const std::string& filename,
                                                 const BuildId& expected_build_id,
                                                 uint64_t* min_vaddr) {
+  if (!IsValidElfPath(filename)) {
+    return false;
+  }
   auto owning_binary = llvm::object::createBinary(llvm::StringRef(filename));
   llvm::object::ObjectFile* obj = GetObjectFile(owning_binary, filename, expected_build_id);
   if (obj == nullptr) {

diff --git a/simpleperf/read_elf.h b/simpleperf/read_elf.h
index cc33211..cfb13be 100644 (file)
--- a/simpleperf/read_elf.h
+++ b/simpleperf/read_elf.h
@@ -45,5 +45,6 @@ bool ReadMinExecutableVirtualAddressFromElfFile(const std::string& filename,
 
 // Expose the following functions for unit tests.
 bool IsArmMappingSymbol(const char* name);
+bool IsValidElfPath(const std::string& filename);
 
 #endif  // SIMPLE_PERF_READ_ELF_H_

diff --git a/simpleperf/read_elf_test.cpp b/simpleperf/read_elf_test.cpp
index 924af97..407e7b7 100644 (file)
--- a/simpleperf/read_elf_test.cpp
+++ b/simpleperf/read_elf_test.cpp
@@ -25,16 +25,10 @@ static void ParseSymbol(const ElfFileSymbol& symbol, bool* result) {
 }
 
 TEST(read_elf, parse_symbols_from_elf_file) {
-  char elf_file[PATH_MAX];
-  ssize_t elf_file_len = readlink("/proc/self/exe", elf_file, sizeof(elf_file));
-  ASSERT_GT(elf_file_len, 0L);
-  ASSERT_LT(static_cast<size_t>(elf_file_len), sizeof(elf_file));
-  elf_file[elf_file_len] = '\0';
-
   BuildId build_id;
-  GetBuildIdFromElfFile(elf_file, &build_id);
+  GetBuildIdFromElfFile("proc/self/exe", &build_id);
   bool result = false;
-  ASSERT_TRUE(ParseSymbolsFromElfFile(elf_file, build_id,
+  ASSERT_TRUE(ParseSymbolsFromElfFile("/proc/self/exe", build_id,
                                       std::bind(ParseSymbol, std::placeholders::_1, &result)));
   ASSERT_TRUE(result);
 }
@@ -45,3 +39,9 @@ TEST(read_elf, arm_mapping_symbol) {
   ASSERT_TRUE(IsArmMappingSymbol("$a.anything"));
   ASSERT_FALSE(IsArmMappingSymbol("$a_no_dot"));
 }
+
+TEST(read_elf, IsValidElfPath) {
+  ASSERT_FALSE(IsValidElfPath("/dev/zero"));
+  ASSERT_FALSE(IsValidElfPath("/sys/devices/system/cpu/online"));
+  ASSERT_TRUE(IsValidElfPath("/proc/self/exe"));
+}

diff --git a/simpleperf/utils.cpp b/simpleperf/utils.cpp
index 1c95c4e..ccb10f5 100644 (file)
--- a/simpleperf/utils.cpp
+++ b/simpleperf/utils.cpp
@@ -105,6 +105,16 @@ bool IsDir(const std::string& dirpath) {
   return false;
 }
 
+bool IsRegularFile(const std::string& filename) {
+  struct stat st;
+  if (stat(filename.c_str(), &st) == 0) {
+    if (S_ISREG(st.st_mode)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 bool RemovePossibleFile(const std::string& filename) {
   struct stat st;
   if (stat(filename.c_str(), &st) == 0) {

diff --git a/simpleperf/utils.h b/simpleperf/utils.h
index 4077c97..ceaaa44 100644 (file)
--- a/simpleperf/utils.h
+++ b/simpleperf/utils.h
@@ -108,6 +108,7 @@ bool IsPowerOfTwo(uint64_t value);
 void GetEntriesInDir(const std::string& dirpath, std::vector<std::string>* files,
                      std::vector<std::string>* subdirs);
 bool IsDir(const std::string& dirpath);
+bool IsRegularFile(const std::string& filename);
 bool RemovePossibleFile(const std::string& filename);
 bool StringToPid(const std::string& s, int* pid);