From 0a5fc5d663054eb836eafc258cc2f6792358e2c9 Mon Sep 17 00:00:00 2001 From: "ted.mielczarek" Date: Wed, 23 Dec 2009 17:09:27 +0000 Subject: Issue 357: New Linux file_id code doesn't persist across strip. r=agl,nealsid at http://breakpad.appspot.com/49008 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@461 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/client/linux/Makefile | 1 + src/client/linux/minidump_writer/linux_dumper.cc | 29 +++++++ src/client/linux/minidump_writer/linux_dumper.h | 5 ++ .../linux/minidump_writer/linux_dumper_unittest.cc | 65 +++++++++++++++ .../linux/minidump_writer/minidump_writer.cc | 25 ++---- src/common/linux/file_id.cc | 92 ++++++++++++++++++---- src/common/linux/file_id.h | 10 ++- src/common/linux/file_id_unittest.cc | 76 ++++++++++++++++++ src/tools/linux/dump_syms/Makefile | 16 ++++ 9 files changed, 283 insertions(+), 36 deletions(-) create mode 100644 src/common/linux/file_id_unittest.cc (limited to 'src') diff --git a/src/client/linux/Makefile b/src/client/linux/Makefile index 6f1c24f5..9116fe6f 100644 --- a/src/client/linux/Makefile +++ b/src/client/linux/Makefile @@ -39,6 +39,7 @@ TEST_CC_SRC=handler/exception_handler_unittest.cc \ minidump_writer/line_reader_unittest.cc \ minidump_writer/linux_dumper_unittest.cc \ minidump_writer/minidump_writer_unittest.cc \ + ../../common/linux/file_id.cc \ $(GOOGLETEST_CC_SRC) TEST_CC_OBJ=$(patsubst %.cc, $(OBJ_DIR)/%.o,$(TEST_CC_SRC)) diff --git a/src/client/linux/minidump_writer/linux_dumper.cc b/src/client/linux/minidump_writer/linux_dumper.cc index 28729b34..d752608d 100644 --- a/src/client/linux/minidump_writer/linux_dumper.cc +++ b/src/client/linux/minidump_writer/linux_dumper.cc @@ -42,15 +42,20 @@ #include #include +#include #include #include +#include #include #include #include +#include + #include "client/linux/minidump_writer/directory_reader.h" #include "client/linux/minidump_writer/line_reader.h" +#include "common/linux/file_id.h" #include "common/linux/linux_libc_support.h" #include "common/linux/linux_syscall_support.h" @@ -156,6 +161,30 @@ LinuxDumper::BuildProcPath(char* path, pid_t pid, const char* node) const { memcpy(path + total_length, "\0", 1); } +bool +LinuxDumper::ElfFileIdentifierForMapping(unsigned int mapping_id, + uint8_t identifier[sizeof(MDGUID)]) +{ + assert(mapping_id < mappings_.size()); + const MappingInfo* mapping = mappings_[mapping_id]; + int fd = sys_open(mapping->name, O_RDONLY, 0); + if (fd < 0) + return false; + struct kernel_stat st; + if (sys_fstat(fd, &st) != 0) { + sys_close(fd); + return false; + } + void* base = sys_mmap2(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0); + sys_close(fd); + if (base == MAP_FAILED) + return false; + + bool success = FileID::ElfFileIdentifierFromMappedFile(base, identifier); + sys_munmap(base, st.st_size); + return success; +} + void* LinuxDumper::FindBeginningOfLinuxGateSharedLibrary(const pid_t pid) const { char auxv_path[80]; diff --git a/src/client/linux/minidump_writer/linux_dumper.h b/src/client/linux/minidump_writer/linux_dumper.h index d8e5e783..3e8869b5 100644 --- a/src/client/linux/minidump_writer/linux_dumper.h +++ b/src/client/linux/minidump_writer/linux_dumper.h @@ -37,6 +37,7 @@ #include #include "common/linux/memory.h" +#include "google_breakpad/common/minidump_format.h" namespace google_breakpad { @@ -122,6 +123,10 @@ class LinuxDumper { // without any slashes. void BuildProcPath(char* path, pid_t pid, const char* node) const; + // Generate a File ID from the .text section of a mapped entry + bool ElfFileIdentifierForMapping(unsigned int mapping_id, + uint8_t identifier[sizeof(MDGUID)]); + // Utility method to find the location of where the kernel has // mapped linux-gate.so in memory(shows up in /proc/pid/maps as // [vdso], but we can't guarantee that it's the only virtual dynamic diff --git a/src/client/linux/minidump_writer/linux_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_dumper_unittest.cc index f5ed914b..e060e53e 100644 --- a/src/client/linux/minidump_writer/linux_dumper_unittest.cc +++ b/src/client/linux/minidump_writer/linux_dumper_unittest.cc @@ -27,13 +27,25 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#include #include #include "client/linux/minidump_writer/linux_dumper.h" +#include "common/linux/file_id.h" #include "breakpad_googletest_includes.h" using namespace google_breakpad; +// This provides a wrapper around system calls which may be +// interrupted by a signal and return EINTR. See man 7 signal. +#define HANDLE_EINTR(x) ({ \ + typeof(x) __eintr_result__; \ + do { \ + __eintr_result__ = x; \ + } while (__eintr_result__ == -1 && errno == EINTR); \ + __eintr_result__;\ +}) + namespace { typedef testing::Test LinuxDumperTest; } @@ -116,3 +128,56 @@ TEST(LinuxDumperTest, MappingsIncludeLinuxGate) { EXPECT_EQ(0, memcmp(linux_gate_loc, ELFMAG, SELFMAG)); } } + +TEST(LinuxDumperTest, FileIDsMatch) { + // Calculate the File ID of our binary using both + // FileID::ElfFileIdentifier and LinuxDumper::ElfFileIdentifierForMapping + // and ensure that we get the same result from both. + char exe_name[PATH_MAX]; + ssize_t len = readlink("/proc/self/exe", exe_name, PATH_MAX - 1); + ASSERT_NE(len, -1); + exe_name[len] = '\0'; + + int fds[2]; + ASSERT_NE(-1, pipe(fds)); + + // fork a child so we can ptrace it + const pid_t child = fork(); + if (child == 0) { + close(fds[1]); + // now wait forever for the parent + char b; + HANDLE_EINTR(read(fds[0], &b, sizeof(b))); + close(fds[0]); + syscall(__NR_exit); + } + close(fds[0]); + + LinuxDumper dumper(child); + ASSERT_TRUE(dumper.Init()); + const wasteful_vector mappings = dumper.mappings(); + bool found_exe = false; + unsigned i; + for (i = 0; i < mappings.size(); ++i) { + const MappingInfo* mapping = mappings[i]; + if (!strcmp(mapping->name, exe_name)) { + found_exe = true; + break; + } + } + ASSERT_TRUE(found_exe); + + uint8_t identifier1[sizeof(MDGUID)]; + uint8_t identifier2[sizeof(MDGUID)]; + EXPECT_TRUE(dumper.ElfFileIdentifierForMapping(i, identifier1)); + FileID fileid(exe_name); + EXPECT_TRUE(fileid.ElfFileIdentifier(identifier2)); + char identifier_string1[37]; + char identifier_string2[37]; + FileID::ConvertIdentifierToString(identifier1, identifier_string1, + 37); + FileID::ConvertIdentifierToString(identifier2, identifier_string2, + 37); + EXPECT_STREQ(identifier_string1, identifier_string2); + close(fds[1]); +} diff --git a/src/client/linux/minidump_writer/minidump_writer.cc b/src/client/linux/minidump_writer/minidump_writer.cc index 09f23723..166b8e5c 100644 --- a/src/client/linux/minidump_writer/minidump_writer.cc +++ b/src/client/linux/minidump_writer/minidump_writer.cc @@ -547,26 +547,11 @@ class MinidumpWriter { const uint32_t cv_signature = MD_CVINFOPDB70_SIGNATURE; memcpy(cv_ptr, &cv_signature, sizeof(cv_signature)); cv_ptr += sizeof(cv_signature); - - { - // We XOR the first page of the file to get a signature for it. - uint8_t xor_buf[sizeof(MDGUID)]; - size_t done = 0; - uint8_t* signature = cv_ptr; - cv_ptr += sizeof(xor_buf); - - my_memset(signature, 0, sizeof(xor_buf)); - while (done < 4096) { - dumper_.CopyFromProcess(xor_buf, crashing_tid_, - (void *) (mod.base_of_image + done), - sizeof(xor_buf)); - for (unsigned i = 0; i < sizeof(xor_buf); ++i) - signature[i] ^= xor_buf[i]; - done += sizeof(xor_buf); - } - my_memset(cv_ptr, 0, sizeof(uint32_t)); // Set age to 0 on Linux. - cv_ptr += sizeof(uint32_t); - } + uint8_t* signature = cv_ptr; + cv_ptr += sizeof(MDGUID); + dumper_.ElfFileIdentifierForMapping(i, signature); + my_memset(cv_ptr, 0, sizeof(uint32_t)); // Set age to 0 on Linux. + cv_ptr += sizeof(uint32_t); // Write pdb_file_name memcpy(cv_ptr, filename_ptr, filename_len + 1); diff --git a/src/common/linux/file_id.cc b/src/common/linux/file_id.cc index 34c9e508..8fae273a 100644 --- a/src/common/linux/file_id.cc +++ b/src/common/linux/file_id.cc @@ -33,6 +33,8 @@ // #include "common/linux/file_id.h" +#include "common/linux/linux_libc_support.h" +#include "common/linux/linux_syscall_support.h" #include #include @@ -42,6 +44,7 @@ #include #include +#include #include #include @@ -51,33 +54,94 @@ FileID::FileID(const char* path) { strncpy(path_, path, sizeof(path_)); } +// These two functions are also used inside the crashed process, so be safe +// and use the syscall/libc wrappers instead of direct syscalls or libc. + static bool FindElfTextSection(const void *elf_mapped_base, + const void **text_start, + int *text_size) { + assert(elf_mapped_base); + assert(text_start); + assert(text_size); + + const char* elf_base = + static_cast(elf_mapped_base); + const ElfW(Ehdr)* elf_header = + reinterpret_cast(elf_base); + if (my_strncmp(elf_base, ELFMAG, SELFMAG) != 0) + return false; +#if __ELF_NATIVE_CLASS == 32 +#define ELFCLASS ELFCLASS32 +#else +#define ELFCLASS ELFCLASS64 +#endif + //TODO: support dumping 32-bit binaries from a 64-bit dump_syms? + if (elf_header->e_ident[EI_CLASS] != ELFCLASS) + return false; + *text_start = NULL; + *text_size = 0; + const ElfW(Shdr)* sections = + reinterpret_cast(elf_base + elf_header->e_shoff); + const char* text_section_name = ".text"; + int name_len = my_strlen(text_section_name); + const ElfW(Shdr)* string_section = sections + elf_header->e_shstrndx; + const ElfW(Shdr)* text_section = NULL; + for (int i = 0; i < elf_header->e_shnum; ++i) { + if (sections[i].sh_type == SHT_PROGBITS) { + const char* section_name = (char*)(elf_base + + string_section->sh_offset + + sections[i].sh_name); + if (!my_strncmp(section_name, text_section_name, name_len)) { + text_section = §ions[i]; + break; + } + } + } + if (text_section != NULL && text_section->sh_size > 0) { + *text_start = elf_base + text_section->sh_offset; + *text_size = text_section->sh_size; + } + return true; +} + +// static +bool FileID::ElfFileIdentifierFromMappedFile(void* base, + uint8_t identifier[kMDGUIDSize]) +{ + const void* text_section = NULL; + int text_size = 0; + bool success = false; + if (FindElfTextSection(base, &text_section, &text_size) && (text_size > 0)) { + my_memset(identifier, 0, kMDGUIDSize); + const uint8_t* ptr = reinterpret_cast(text_section); + const uint8_t* ptr_end = ptr + std::min(text_size, 4096); + while (ptr < ptr_end) { + for (unsigned i = 0; i < kMDGUIDSize; i++) + identifier[i] ^= ptr[i]; + ptr += kMDGUIDSize; + } + success = true; + } + return success; +} + bool FileID::ElfFileIdentifier(uint8_t identifier[kMDGUIDSize]) { - const ssize_t mapped_len = 4096; // Page size (matches WriteMappings()) int fd = open(path_, O_RDONLY); if (fd < 0) return false; struct stat st; - if (fstat(fd, &st) != 0 || st.st_size <= mapped_len) { + if (fstat(fd, &st) != 0) { close(fd); return false; } - void* base = mmap(NULL, mapped_len, + void* base = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); close(fd); if (base == MAP_FAILED) return false; - memset(identifier, 0, kMDGUIDSize); - uint8_t* ptr = reinterpret_cast(base); - uint8_t* ptr_end = ptr + mapped_len; - while (ptr < ptr_end) { - for (unsigned i = 0; i < kMDGUIDSize; i++) - identifier[i] ^= ptr[i]; - ptr += kMDGUIDSize; - } - - munmap(base, mapped_len); - return true; + bool success = ElfFileIdentifierFromMappedFile(base, identifier); + munmap(base, st.st_size); + return success; } // static diff --git a/src/common/linux/file_id.h b/src/common/linux/file_id.h index 31bb5e4a..2cd4953e 100644 --- a/src/common/linux/file_id.h +++ b/src/common/linux/file_id.h @@ -49,10 +49,16 @@ class FileID { // Load the identifier for the elf file path specified in the constructor into // |identifier|. Return false if the identifier could not be created for the // file. - // The current implementation will XOR the first page of data to generate an - // identifier. + // The current implementation will XOR the first 4096 bytes of the + // .text section to generate an identifier. bool ElfFileIdentifier(uint8_t identifier[kMDGUIDSize]); + // Load the identifier for the elf file mapped into memory at |base| into + // |identifier|. Return false if the identifier could not be created for the + // file. + static bool ElfFileIdentifierFromMappedFile(void* base, + uint8_t identifier[kMDGUIDSize]); + // Convert the |identifier| data to a NULL terminated string. The string will // be formatted as a UUID (e.g., 22F065BB-FC9C-49F7-80FE-26A7CEBD7BCE). // The |buffer| should be at least 37 bytes long to receive all of the data diff --git a/src/common/linux/file_id_unittest.cc b/src/common/linux/file_id_unittest.cc new file mode 100644 index 00000000..e15d39fb --- /dev/null +++ b/src/common/linux/file_id_unittest.cc @@ -0,0 +1,76 @@ +// Copyright (c) 2009, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Unit tests for FileID + +#include + +#include "common/linux/file_id.h" +#include "breakpad_googletest_includes.h" + +using namespace google_breakpad; + + +namespace { +typedef testing::Test FileIDTest; +} + +TEST(FileIDTest, FileIDStrip) { + // Calculate the File ID of our binary using + // FileID::ElfFileIdentifier, then make a copy of our binary, + // strip it, and ensure that we still get the same result. + char exe_name[PATH_MAX]; + ssize_t len = readlink("/proc/self/exe", exe_name, PATH_MAX - 1); + ASSERT_NE(len, -1); + exe_name[len] = '\0'; + + // copy our binary to a temp file, and strip it + char templ[] = "/tmp/file-id-unittest-XXXXXX"; + mktemp(templ); + char cmdline[4096]; + sprintf(cmdline, "cp \"%s\" \"%s\"", exe_name, templ); + ASSERT_EQ(system(cmdline), 0); + sprintf(cmdline, "strip \"%s\"", templ); + ASSERT_EQ(system(cmdline), 0); + + uint8_t identifier1[sizeof(MDGUID)]; + uint8_t identifier2[sizeof(MDGUID)]; + FileID fileid1(exe_name); + EXPECT_TRUE(fileid1.ElfFileIdentifier(identifier1)); + FileID fileid2(templ); + EXPECT_TRUE(fileid2.ElfFileIdentifier(identifier2)); + char identifier_string1[37]; + char identifier_string2[37]; + FileID::ConvertIdentifierToString(identifier1, identifier_string1, + 37); + FileID::ConvertIdentifierToString(identifier2, identifier_string2, + 37); + EXPECT_STREQ(identifier_string1, identifier_string2); + unlink(templ); +} diff --git a/src/tools/linux/dump_syms/Makefile b/src/tools/linux/dump_syms/Makefile index a0424c05..94871da7 100644 --- a/src/tools/linux/dump_syms/Makefile +++ b/src/tools/linux/dump_syms/Makefile @@ -113,6 +113,22 @@ stabs_reader_unittest.o: override CPPFLAGS += $(GTEST_CPPFLAGS) \ clean:: rm -f stabs_reader_unittest +### Unit tests for google_breakpad::FileID. +check: check-file_id_unittest +check-file_id_unittest: file_id_unittest +file_id_unittest: \ + gmock-all.o \ + gtest-all.o \ + gtest_main.o \ + file_id.o \ + $(empty) +CPP_EXECUTABLES += file_id_unittest +file_id_unittest.o: file_id_unittest.cc +file_id_unittest.o: override CPPFLAGS += $(GTEST_CPPFLAGS) \ + $(GMOCK_CPPFLAGS) +clean:: + rm -f file_id_unittest + ### Generic compilation rules. -- cgit v1.2.1