From ba00a0d45569463799f67ed37e66c77f0cd6f015 Mon Sep 17 00:00:00 2001 From: Tudor Bosman Date: Wed, 23 Jul 2014 15:16:32 -0700 Subject: [PATCH] Make ElfFile not crash on invalid ELF files Summary: @rkroll wants to run this on more than just fbcode binaries; there are x86 (not x86_64) binaries in there and probably a lot of other junk. So, if you call openNoThrow explicitly, you get a pretty error message in this case. Test Plan: folly/experimental/symbolizer/test Reviewed By: lucian@fb.com Subscribers: rkroll, jhj, lesha, kma FB internal diff: D1453758 --- folly/experimental/symbolizer/Elf.cpp | 87 +++++++++++++++------- folly/experimental/symbolizer/Elf.h | 15 +++- folly/experimental/symbolizer/ElfCache.cpp | 11 ++- 3 files changed, 79 insertions(+), 34 deletions(-) diff --git a/folly/experimental/symbolizer/Elf.cpp b/folly/experimental/symbolizer/Elf.cpp index 6f525f3f..b4b76e13 100644 --- a/folly/experimental/symbolizer/Elf.cpp +++ b/folly/experimental/symbolizer/Elf.cpp @@ -50,7 +50,11 @@ ElfFile::ElfFile(const char* name, bool readOnly) void ElfFile::open(const char* name, bool readOnly) { const char* msg = ""; int r = openNoThrow(name, readOnly, &msg); - folly::checkUnixError(r, msg); + if (r == kSystemError) { + throwSystemError(msg); + } else { + CHECK_EQ(r, kSuccess) << msg; + } } int ElfFile::openNoThrow(const char* name, bool readOnly, const char** msg) @@ -59,14 +63,14 @@ int ElfFile::openNoThrow(const char* name, bool readOnly, const char** msg) fd_ = ::open(name, readOnly ? O_RDONLY : O_RDWR); if (fd_ == -1) { if (msg) *msg = "open"; - return -1; + return kSystemError; } struct stat st; int r = fstat(fd_, &st); if (r == -1) { if (msg) *msg = "fstat"; - return -1; + return kSystemError; } length_ = st.st_size; @@ -77,17 +81,21 @@ int ElfFile::openNoThrow(const char* name, bool readOnly, const char** msg) file_ = static_cast(mmap(nullptr, length_, prot, MAP_SHARED, fd_, 0)); if (file_ == MAP_FAILED) { if (msg) *msg = "mmap"; - return -1; + return kSystemError; + } + if (!init(msg)) { + errno = EINVAL; + return kInvalidElfFile; } - init(); - return 0; + + return kSuccess; } ElfFile::~ElfFile() { destroy(); } -ElfFile::ElfFile(ElfFile&& other) +ElfFile::ElfFile(ElfFile&& other) noexcept : fd_(other.fd_), file_(other.file_), length_(other.length_), @@ -125,22 +133,26 @@ void ElfFile::destroy() { } } -void ElfFile::init() { +bool ElfFile::init(const char** msg) { auto& elfHeader = this->elfHeader(); // Validate ELF magic numbers - FOLLY_SAFE_CHECK(elfHeader.e_ident[EI_MAG0] == ELFMAG0 && - elfHeader.e_ident[EI_MAG1] == ELFMAG1 && - elfHeader.e_ident[EI_MAG2] == ELFMAG2 && - elfHeader.e_ident[EI_MAG3] == ELFMAG3, - "invalid ELF magic"); + if (!(elfHeader.e_ident[EI_MAG0] == ELFMAG0 && + elfHeader.e_ident[EI_MAG1] == ELFMAG1 && + elfHeader.e_ident[EI_MAG2] == ELFMAG2 && + elfHeader.e_ident[EI_MAG3] == ELFMAG3)) { + if (msg) *msg = "invalid ELF magic"; + return false; + } // Validate ELF class (32/64 bits) #define EXPECTED_CLASS P1(ELFCLASS, __ELF_NATIVE_CLASS) #define P1(a, b) P2(a, b) #define P2(a, b) a ## b - FOLLY_SAFE_CHECK(elfHeader.e_ident[EI_CLASS] == EXPECTED_CLASS, - "invalid ELF class"); + if (elfHeader.e_ident[EI_CLASS] != EXPECTED_CLASS) { + if (msg) *msg = "invalid ELF class"; + return false; + } #undef P1 #undef P2 #undef EXPECTED_CLASS @@ -153,24 +165,38 @@ void ElfFile::init() { #else # error Unsupported byte order #endif - FOLLY_SAFE_CHECK(elfHeader.e_ident[EI_DATA] == EXPECTED_ENCODING, - "invalid ELF encoding"); + if (elfHeader.e_ident[EI_DATA] != EXPECTED_ENCODING) { + if (msg) *msg = "invalid ELF encoding"; + return false; + } #undef EXPECTED_ENCODING // Validate ELF version (1) - FOLLY_SAFE_CHECK(elfHeader.e_ident[EI_VERSION] == EV_CURRENT && - elfHeader.e_version == EV_CURRENT, - "invalid ELF version"); + if (elfHeader.e_ident[EI_VERSION] != EV_CURRENT || + elfHeader.e_version != EV_CURRENT) { + if (msg) *msg = "invalid ELF version"; + return false; + } // We only support executable and shared object files - FOLLY_SAFE_CHECK(elfHeader.e_type == ET_EXEC || elfHeader.e_type == ET_DYN, - "invalid ELF file type"); + if (elfHeader.e_type != ET_EXEC && elfHeader.e_type != ET_DYN) { + if (msg) *msg = "invalid ELF file type"; + return false; + } - FOLLY_SAFE_CHECK(elfHeader.e_phnum != 0, "no program header!"); - FOLLY_SAFE_CHECK(elfHeader.e_phentsize == sizeof(ElfW(Phdr)), - "invalid program header entry size"); - FOLLY_SAFE_CHECK(elfHeader.e_shentsize == sizeof(ElfW(Shdr)), - "invalid section header entry size"); + if (elfHeader.e_phnum == 0) { + if (msg) *msg = "no program header!"; + return false; + } + + if (elfHeader.e_phentsize != sizeof(ElfW(Phdr))) { + if (msg) *msg = "invalid program header entry size"; + return false; + } + + if (elfHeader.e_shentsize != sizeof(ElfW(Shdr))) { + if (msg) *msg = "invalid section header entry size"; + } const ElfW(Phdr)* programHeader = &at(elfHeader.e_phoff); bool foundBase = false; @@ -184,7 +210,12 @@ void ElfFile::init() { } } - FOLLY_SAFE_CHECK(foundBase, "could not find base address"); + if (!foundBase) { + if (msg) *msg = "could not find base address"; + return false; + } + + return true; } const ElfW(Shdr)* ElfFile::getSectionByIndex(size_t idx) const { diff --git a/folly/experimental/symbolizer/Elf.h b/folly/experimental/symbolizer/Elf.h index f1c49334..6214a4d5 100644 --- a/folly/experimental/symbolizer/Elf.h +++ b/folly/experimental/symbolizer/Elf.h @@ -49,8 +49,15 @@ class ElfFile { explicit ElfFile(const char* name, bool readOnly=true); // Open the ELF file. - // Returns 0 on success, -1 (and sets errno) on failure and (if msg is not - // NULL) sets *msg to a static string indicating what failed. + // Returns 0 on success, kSystemError (guaranteed to be -1) (and sets errno) + // on IO error, kInvalidElfFile (and sets errno to EINVAL) for an invalid + // Elf file. On error, if msg is not NULL, sets *msg to a static string + // indicating what failed. + enum { + kSuccess = 0, + kSystemError = -1, + kInvalidElfFile = -2, + }; int openNoThrow(const char* name, bool readOnly=true, const char** msg=nullptr) noexcept; @@ -59,7 +66,7 @@ class ElfFile { ~ElfFile(); - ElfFile(ElfFile&& other); + ElfFile(ElfFile&& other) noexcept; ElfFile& operator=(ElfFile&& other); /** Retrieve the ELF header */ @@ -185,7 +192,7 @@ class ElfFile { const ElfW(Shdr)* getSectionContainingAddress(ElfW(Addr) addr) const; private: - void init(); + bool init(const char** msg); void destroy(); ElfFile(const ElfFile&) = delete; ElfFile& operator=(const ElfFile&) = delete; diff --git a/folly/experimental/symbolizer/ElfCache.cpp b/folly/experimental/symbolizer/ElfCache.cpp index 03fc36d0..7aa067bd 100644 --- a/folly/experimental/symbolizer/ElfCache.cpp +++ b/folly/experimental/symbolizer/ElfCache.cpp @@ -46,9 +46,13 @@ std::shared_ptr SignalSafeElfCache::getFile(StringPiece p) { } auto& f = slots_[n]; - if (f->openNoThrow(path.data()) == -1) { + + const char* msg = ""; + int r = f->openNoThrow(path.data(), true, &msg); + if (r == ElfFile::kSystemError) { return nullptr; } + FOLLY_SAFE_CHECK(r == ElfFile::kSuccess, msg); map_[path] = n; return f; @@ -73,9 +77,12 @@ std::shared_ptr ElfCache::getFile(StringPiece p) { auto& path = entry->path; // No negative caching - if (entry->file.openNoThrow(path.c_str()) == -1) { + const char* msg = ""; + int r = entry->file.openNoThrow(path.c_str(), true, &msg); + if (r == ElfFile::kSystemError) { return nullptr; } + FOLLY_SAFE_CHECK(r == ElfFile::kSuccess, msg); if (files_.size() == capacity_) { auto& e = lruList_.front(); -- 2.34.1