diff options
author | Yabin Cui <yabinc@google.com> | 2015-12-08 17:43:15 -0800 |
---|---|---|
committer | Yabin Cui <yabinc@google.com> | 2015-12-08 18:36:17 -0800 |
commit | 797116bfb9ad10d858086680c7465d65c399ef6e (patch) | |
tree | 94c8d884515452948e75d893c89cfc73f5d7c8f9 | |
parent | 113206fb6548cf95bdb91821c7b4dd384c926756 (diff) | |
download | extras-797116bfb9ad10d858086680c7465d65c399ef6e.tar.gz |
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
-rw-r--r-- | simpleperf/read_elf.cpp | 25 | ||||
-rw-r--r-- | simpleperf/read_elf.h | 1 | ||||
-rw-r--r-- | simpleperf/read_elf_test.cpp | 16 | ||||
-rw-r--r-- | simpleperf/utils.cpp | 10 | ||||
-rw-r--r-- | simpleperf/utils.h | 1 |
5 files changed, 45 insertions, 8 deletions
diff --git a/simpleperf/read_elf.cpp b/simpleperf/read_elf.cpp index 9a4f4b95..4774ea3f 100644 --- 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 cc33211f..cfb13be3 100644 --- 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 924af97a..407e7b78 100644 --- 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 1c95c4ea..ccb10f5e 100644 --- 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 4077c975..ceaaa448 100644 --- 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); |