diff options
author | ivanpe@google.com <ivanpe@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-22 00:34:52 +0000 |
---|---|---|
committer | ivanpe@google.com <ivanpe@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-22 00:34:52 +0000 |
commit | ad9a01213fe3630d7031f7b5b0fcb430fc46cf03 (patch) | |
tree | 4a1c52895c5128ee660be5f09b09b20f29301a67 /base | |
parent | f3357cbc0b2001640b93a32d927e30e3a713f69e (diff) | |
download | chromium_src-ad9a01213fe3630d7031f7b5b0fcb430fc46cf03.zip chromium_src-ad9a01213fe3630d7031f7b5b0fcb430fc46cf03.tar.gz chromium_src-ad9a01213fe3630d7031f7b5b0fcb430fc46cf03.tar.bz2 |
Stack trace symbolization for Chrome sandbox processes.
Patching the POSIX implementation of base::debug::StackTrace to properly
symbolize stack traces when used in Chrome sandbox processes. The main
reason symbolization doesn't work today is because sandboxed processes
cannot open proc/self/maps and the object files that contain the symbols.
The proposed solution is to open all necessary files before enabling the
sandbox and then keep their file descriptors around for the duration of
the sandboxed process. This cannot be done in shipping/retail builds
because it has security implications so for the time being this code will
run only in DEBUG builds of Chrome.
Originally, the code review was started under a different account. For
more information take a look at: https://codereview.chromium.org/156043003/
BUG=341966
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258548
Review URL: https://codereview.chromium.org/184273012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258719 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base')
-rw-r--r-- | base/debug/proc_maps_linux.cc | 6 | ||||
-rw-r--r-- | base/debug/proc_maps_linux_unittest.cc | 32 | ||||
-rw-r--r-- | base/debug/stack_trace.h | 9 | ||||
-rw-r--r-- | base/debug/stack_trace_posix.cc | 271 |
4 files changed, 313 insertions, 5 deletions
diff --git a/base/debug/proc_maps_linux.cc b/base/debug/proc_maps_linux.cc index d9ff8c5..1e0209e 100644 --- a/base/debug/proc_maps_linux.cc +++ b/base/debug/proc_maps_linux.cc @@ -91,6 +91,7 @@ bool ReadProcMaps(std::string* proc_maps) { bool ParseProcMaps(const std::string& input, std::vector<MappedMemoryRegion>* regions_out) { + CHECK(regions_out); std::vector<MappedMemoryRegion> regions; // This isn't async safe nor terribly efficient, but it doesn't need to be at @@ -101,8 +102,10 @@ bool ParseProcMaps(const std::string& input, for (size_t i = 0; i < lines.size(); ++i) { // Due to splitting on '\n' the last line should be empty. if (i == lines.size() - 1) { - if (!lines[i].empty()) + if (!lines[i].empty()) { + DLOG(WARNING) << "Last line not empty"; return false; + } break; } @@ -125,6 +128,7 @@ bool ParseProcMaps(const std::string& input, if (sscanf(line, "%" SCNxPTR "-%" SCNxPTR " %4c %llx %hhx:%hhx %ld %n", ®ion.start, ®ion.end, permissions, ®ion.offset, &dev_major, &dev_minor, &inode, &path_index) < 7) { + DPLOG(WARNING) << "sscanf failed for line: " << line; return false; } diff --git a/base/debug/proc_maps_linux_unittest.cc b/base/debug/proc_maps_linux_unittest.cc index 7c2929f..fc8ced6 100644 --- a/base/debug/proc_maps_linux_unittest.cc +++ b/base/debug/proc_maps_linux_unittest.cc @@ -277,5 +277,37 @@ TEST(ProcMapsTest, InvalidInput) { } } +TEST(ProcMapsTest, ParseProcMapsEmptyString) { + std::vector<MappedMemoryRegion> regions; + EXPECT_TRUE(ParseProcMaps("", ®ions)); + EXPECT_EQ(0ULL, regions.size()); +} + +// Testing a couple of remotely possible weird things in the input: +// - Line ending with \r\n or \n\r. +// - File name contains quotes. +// - File name has whitespaces. +TEST(ProcMapsTest, ParseProcMapsWeirdCorrectInput) { + std::vector<MappedMemoryRegion> regions; + const std::string kContents = + "00400000-0040b000 r-xp 00000000 fc:00 2106562 " + " /bin/cat\r\n" + "7f53b7dad000-7f53b7f62000 r-xp 00000000 fc:00 263011 " + " /lib/x86_64-linux-gnu/libc-2.15.so\n\r" + "7f53b816d000-7f53b818f000 r-xp 00000000 fc:00 264284 " + " /lib/x86_64-linux-gnu/ld-2.15.so\n" + "7fff9c7ff000-7fff9c800000 r-xp 00000000 00:00 0 " + " \"vd so\"\n" + "ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 " + " [vsys call]\n"; + EXPECT_TRUE(ParseProcMaps(kContents, ®ions)); + EXPECT_EQ(5ULL, regions.size()); + EXPECT_EQ("/bin/cat", regions[0].path); + EXPECT_EQ("/lib/x86_64-linux-gnu/libc-2.15.so", regions[1].path); + EXPECT_EQ("/lib/x86_64-linux-gnu/ld-2.15.so", regions[2].path); + EXPECT_EQ("\"vd so\"", regions[3].path); + EXPECT_EQ("[vsys call]", regions[4].path); +} + } // namespace debug } // namespace base diff --git a/base/debug/stack_trace.h b/base/debug/stack_trace.h index b0883c1..78f886a 100644 --- a/base/debug/stack_trace.h +++ b/base/debug/stack_trace.h @@ -27,6 +27,15 @@ namespace debug { // unit_tests only! This is not thread-safe: only call from main thread. BASE_EXPORT bool EnableInProcessStackDumping(); +// A different version of EnableInProcessStackDumping that also works for +// sandboxed processes. For more details take a look at the description +// of EnableInProcessStackDumping. +// Calling this function on Linux opens /proc/self/maps and caches its +// contents. In DEBUG builds, this function also opens the object files that +// are loaded in memory and caches their file descriptors (this cannot be +// done in official builds because it has security implications). +BASE_EXPORT bool EnableInProcessStackDumpingForSandbox(); + // A stacktrace can be helpful in debugging. For example, you can include a // stacktrace member in a object (probably around #ifndef NDEBUG) so that you // can later see where the given object was created from. diff --git a/base/debug/stack_trace_posix.cc b/base/debug/stack_trace_posix.cc index 3f0e95f..a7c1c6f 100644 --- a/base/debug/stack_trace_posix.cc +++ b/base/debug/stack_trace_posix.cc @@ -15,7 +15,10 @@ #include <sys/types.h> #include <unistd.h> +#include <map> #include <ostream> +#include <string> +#include <vector> #if defined(__GLIBCXX__) #include <cxxabi.h> @@ -27,8 +30,10 @@ #include "base/basictypes.h" #include "base/debug/debugger.h" +#include "base/debug/proc_maps_linux.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/singleton.h" #include "base/numerics/safe_conversions.h" #include "base/posix/eintr_wrapper.h" #include "base/strings/string_number_conversions.h" @@ -115,14 +120,27 @@ class BacktraceOutputHandler { }; void OutputPointer(void* pointer, BacktraceOutputHandler* handler) { - char buf[1024] = { '\0' }; - handler->HandleOutput(" [0x"); + // This should be more than enough to store a 64-bit number in hex: + // 16 hex digits + 1 for null-terminator. + char buf[17] = { '\0' }; + handler->HandleOutput("0x"); internal::itoa_r(reinterpret_cast<intptr_t>(pointer), buf, sizeof(buf), 16, 12); handler->HandleOutput(buf); - handler->HandleOutput("]"); } +#if defined(USE_SYMBOLIZE) +void OutputFrameId(intptr_t frame_id, BacktraceOutputHandler* handler) { + // Max unsigned 64-bit number in decimal has 20 digits (18446744073709551615). + // Hence, 30 digits should be more than enough to represent it in decimal + // (including the null-terminator). + char buf[30] = { '\0' }; + handler->HandleOutput("#"); + internal::itoa_r(frame_id, buf, sizeof(buf), 10, 1); + handler->HandleOutput(buf); +} +#endif // defined(USE_SYMBOLIZE) + void ProcessBacktrace(void *const *trace, size_t size, BacktraceOutputHandler* handler) { @@ -131,6 +149,8 @@ void ProcessBacktrace(void *const *trace, #if defined(USE_SYMBOLIZE) for (size_t i = 0; i < size; ++i) { + OutputFrameId(i, handler); + handler->HandleOutput(" "); OutputPointer(trace[i], handler); handler->HandleOutput(" "); @@ -168,8 +188,9 @@ void ProcessBacktrace(void *const *trace, if (!printed) { for (size_t i = 0; i < size; ++i) { + handler->HandleOutput(" ["); OutputPointer(trace[i], handler); - handler->HandleOutput("\n"); + handler->HandleOutput("]\n"); } } #endif // defined(USE_SYMBOLIZE) @@ -438,6 +459,248 @@ void WarmUpBacktrace() { } // namespace +#if defined(USE_SYMBOLIZE) + +// class SandboxSymbolizeHelper. +// +// The purpose of this class is to prepare and install a "file open" callback +// needed by the stack trace symbolization code +// (base/third_party/symbolize/symbolize.h) so that it can function properly +// in a sandboxed process. The caveat is that this class must be instantiated +// before the sandboxing is enabled so that it can get the chance to open all +// the object files that are loaded in the virtual address space of the current +// process. +class SandboxSymbolizeHelper { + public: + // Returns the singleton instance. + static SandboxSymbolizeHelper* GetInstance() { + return Singleton<SandboxSymbolizeHelper>::get(); + } + + private: + friend struct DefaultSingletonTraits<SandboxSymbolizeHelper>; + + SandboxSymbolizeHelper() + : is_initialized_(false) { + Init(); + } + + ~SandboxSymbolizeHelper() { + UnregisterCallback(); + CloseObjectFiles(); + } + + // Returns a O_RDONLY file descriptor for |file_path| if it was opened + // sucessfully during the initialization. The file is repositioned at + // offset 0. + // IMPORTANT: This function must be async-signal-safe because it can be + // called from a signal handler (symbolizing stack frames for a crash). + int GetFileDescriptor(const char* file_path) { + int fd = -1; + +#if !defined(NDEBUG) + if (file_path) { + // The assumption here is that iterating over std::map<std::string, int> + // using a const_iterator does not allocate dynamic memory, hense it is + // async-signal-safe. + std::map<std::string, int>::const_iterator it; + for (it = modules_.begin(); it != modules_.end(); ++it) { + if (strcmp((it->first).c_str(), file_path) == 0) { + // POSIX.1-2004 requires an implementation to guarantee that dup() + // is async-signal-safe. + fd = dup(it->second); + break; + } + } + // POSIX.1-2004 requires an implementation to guarantee that lseek() + // is async-signal-safe. + if (fd >= 0 && lseek(fd, 0, SEEK_SET) < 0) { + // Failed to seek. + fd = -1; + } + } +#endif // !defined(NDEBUG) + + return fd; + } + + // Searches for the object file (from /proc/self/maps) that contains + // the specified pc. If found, sets |start_address| to the start address + // of where this object file is mapped in memory, sets the module base + // address into |base_address|, copies the object file name into + // |out_file_name|, and attempts to open the object file. If the object + // file is opened successfully, returns the file descriptor. Otherwise, + // returns -1. |out_file_name_size| is the size of the file name buffer + // (including the null terminator). + // IMPORTANT: This function must be async-signal-safe because it can be + // called from a signal handler (symbolizing stack frames for a crash). + static int OpenObjectFileContainingPc(uint64_t pc, uint64_t& start_address, + uint64_t& base_address, char* file_path, + int file_path_size) { + // This method can only be called after the singleton is instantiated. + // This is ensured by the following facts: + // * This is the only static method in this class, it is private, and + // the class has no friends (except for the DefaultSingletonTraits). + // The compiler guarantees that it can only be called after the + // singleton is instantiated. + // * This method is used as a callback for the stack tracing code and + // the callback registration is done in the constructor, so logically + // it cannot be called before the singleton is created. + SandboxSymbolizeHelper* instance = GetInstance(); + + // The assumption here is that iterating over + // std::vector<MappedMemoryRegion> using a const_iterator does not allocate + // dynamic memory, hence it is async-signal-safe. + std::vector<MappedMemoryRegion>::const_iterator it; + bool is_first = true; + for (it = instance->regions_.begin(); it != instance->regions_.end(); + ++it, is_first = false) { + const MappedMemoryRegion& region = *it; + if (region.start <= pc && pc < region.end) { + start_address = region.start; + // Don't subtract 'start_address' from the first entry: + // * If a binary is compiled w/o -pie, then the first entry in + // process maps is likely the binary itself (all dynamic libs + // are mapped higher in address space). For such a binary, + // instruction offset in binary coincides with the actual + // instruction address in virtual memory (as code section + // is mapped to a fixed memory range). + // * If a binary is compiled with -pie, all the modules are + // mapped high at address space (in particular, higher than + // shadow memory of the tool), so the module can't be the + // first entry. + base_address = (is_first ? 0U : start_address) - region.offset; + if (file_path && file_path_size > 0) { + strncpy(file_path, region.path.c_str(), file_path_size); + // Ensure null termination. + file_path[file_path_size - 1] = '\0'; + } + return instance->GetFileDescriptor(region.path.c_str()); + } + } + return -1; + } + + // Parses /proc/self/maps in order to compile a list of all object file names + // for the modules that are loaded in the current process. + // Returns true on success. + bool CacheMemoryRegions() { + // Reads /proc/self/maps. + std::string contents; + if (!ReadProcMaps(&contents)) { + LOG(ERROR) << "Failed to read /proc/self/maps"; + return false; + } + + // Parses /proc/self/maps. + if (!ParseProcMaps(contents, ®ions_)) { + LOG(ERROR) << "Failed to parse the contents of /proc/self/maps"; + return false; + } + + is_initialized_ = true; + return true; + } + + // Opens all object files and caches their file descriptors. + void OpenSymbolFiles() { + // Pre-opening and caching the file descriptors of all loaded modules is + // not considered safe for retail builds. Hence it is only done in debug + // builds. For more details, take a look at: http://crbug.com/341966 + // Enabling this to release mode would require approval from the security + // team. +#if !defined(NDEBUG) + // Open the object files for all read-only executable regions and cache + // their file descriptors. + std::vector<MappedMemoryRegion>::const_iterator it; + for (it = regions_.begin(); it != regions_.end(); ++it) { + const MappedMemoryRegion& region = *it; + // Only interesed in read-only executable regions. + if ((region.permissions & MappedMemoryRegion::READ) == + MappedMemoryRegion::READ && + (region.permissions & MappedMemoryRegion::WRITE) == 0 && + (region.permissions & MappedMemoryRegion::EXECUTE) == + MappedMemoryRegion::EXECUTE) { + if (region.path.empty()) { + // Skip regions with empty file names. + continue; + } + if (region.path[0] == '[') { + // Skip pseudo-paths, like [stack], [vdso], [heap], etc ... + continue; + } + // Avoid duplicates. + if (modules_.find(region.path) == modules_.end()) { + int fd = open(region.path.c_str(), O_RDONLY | O_CLOEXEC); + if (fd >= 0) { + modules_.insert(std::make_pair(region.path, fd)); + } else { + LOG(WARNING) << "Failed to open file: " << region.path + << "\n Error: " << strerror(errno); + } + } + } + } +#endif // !defined(NDEBUG) + } + + // Initializes and installs the symbolization callback. + void Init() { + if (CacheMemoryRegions()) { + OpenSymbolFiles(); + google::InstallSymbolizeOpenObjectFileCallback( + &OpenObjectFileContainingPc); + } + } + + // Unregister symbolization callback. + void UnregisterCallback() { + if (is_initialized_) { + google::InstallSymbolizeOpenObjectFileCallback(NULL); + is_initialized_ = false; + } + } + + // Closes all file descriptors owned by this instance. + void CloseObjectFiles() { +#if !defined(NDEBUG) + std::map<std::string, int>::iterator it; + for (it = modules_.begin(); it != modules_.end(); ++it) { + int ret = IGNORE_EINTR(close(it->second)); + DCHECK(!ret); + it->second = -1; + } + modules_.clear(); +#endif // !defined(NDEBUG) + } + + // Set to true upon successful initialization. + bool is_initialized_; + +#if !defined(NDEBUG) + // Mapping from file name to file descriptor. Includes file descriptors + // for all successfully opened object files and the file descriptor for + // /proc/self/maps. This code is not safe for release builds so + // this is only done for DEBUG builds. + std::map<std::string, int> modules_; +#endif // !defined(NDEBUG) + + // Cache for the process memory regions. Produced by parsing the contents + // of /proc/self/maps cache. + std::vector<MappedMemoryRegion> regions_; + + DISALLOW_COPY_AND_ASSIGN(SandboxSymbolizeHelper); +}; +#endif // USE_SYMBOLIZE + +bool EnableInProcessStackDumpingForSandbox() { +#if defined(USE_SYMBOLIZE) + SandboxSymbolizeHelper::GetInstance(); +#endif // USE_SYMBOLIZE + + return EnableInProcessStackDumping(); +} + bool EnableInProcessStackDumping() { // When running in an application, our code typically expects SIGPIPE // to be ignored. Therefore, when testing that same code, it should run |