diff options
| -rw-r--r-- | src/client/linux/handler/exception_handler.cc | 2 | ||||
| -rw-r--r-- | src/client/linux/handler/minidump_descriptor.h | 15 | ||||
| -rw-r--r-- | src/client/linux/minidump_writer/minidump_writer.cc | 101 | ||||
| -rw-r--r-- | src/client/linux/minidump_writer/minidump_writer.h | 13 | ||||
| -rw-r--r-- | src/client/linux/minidump_writer/minidump_writer_unittest.cc | 152 |
5 files changed, 266 insertions, 17 deletions
diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc index 9a56c14b..fd12642c 100644 --- a/src/client/linux/handler/exception_handler.cc +++ b/src/client/linux/handler/exception_handler.cc @@ -521,6 +521,7 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context, size_t context_size) { if (minidump_descriptor_.IsFD()) { return google_breakpad::WriteMinidump(minidump_descriptor_.fd(), + minidump_descriptor_.size_limit(), crashing_process, context, context_size, @@ -528,6 +529,7 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context, app_memory_list_); } return google_breakpad::WriteMinidump(minidump_descriptor_.path(), + minidump_descriptor_.size_limit(), crashing_process, context, context_size, diff --git a/src/client/linux/handler/minidump_descriptor.h b/src/client/linux/handler/minidump_descriptor.h index dc2719ac..3036cadb 100644 --- a/src/client/linux/handler/minidump_descriptor.h +++ b/src/client/linux/handler/minidump_descriptor.h @@ -31,6 +31,8 @@ #define CLIENT_LINUX_HANDLER_MINIDUMP_DESCRIPTOR_H_ #include <assert.h> +#include <sys/types.h> + #include <string> #include "common/using_std_string.h" @@ -48,11 +50,15 @@ class MinidumpDescriptor { explicit MinidumpDescriptor(const string& directory) : fd_(-1), directory_(directory), - c_path_(NULL) { + c_path_(NULL), + size_limit_(-1) { assert(!directory.empty()); } - explicit MinidumpDescriptor(int fd) : fd_(fd), c_path_(NULL) { + explicit MinidumpDescriptor(int fd) + : fd_(fd), + c_path_(NULL), + size_limit_(-1) { assert(fd != -1); } @@ -71,6 +77,9 @@ class MinidumpDescriptor { // Should be called from a normal context: this methods uses the heap. void UpdatePath(); + off_t size_limit() const { return size_limit_; } + void set_size_limit(off_t limit) { size_limit_ = limit; } + private: // The file descriptor where the minidump is generated. int fd_; @@ -82,6 +91,8 @@ class MinidumpDescriptor { // The C string of |path_|. Precomputed so it can be access from a compromised // context. const char* c_path_; + + off_t size_limit_; }; } // namespace google_breakpad diff --git a/src/client/linux/minidump_writer/minidump_writer.cc b/src/client/linux/minidump_writer/minidump_writer.cc index 9fb40358..8f2f1950 100644 --- a/src/client/linux/minidump_writer/minidump_writer.cc +++ b/src/client/linux/minidump_writer/minidump_writer.cc @@ -55,6 +55,7 @@ #if defined(__ANDROID__) #include <sys/system_properties.h> #endif +#include <sys/types.h> #include <sys/ucontext.h> #include <sys/user.h> #include <sys/utsname.h> @@ -375,6 +376,22 @@ void CPUFillFromUContext(MDRawContextARM* out, const ucontext* uc, class MinidumpWriter { public: + // The following kLimit* constants are for when minidump_size_limit_ is set + // and the minidump size might exceed it. + // + // Estimate for how big each thread's stack will be (in bytes). + static const unsigned kLimitAverageThreadStackLength = 8 * 1024; + // Number of threads whose stack size we don't want to limit. These base + // threads will simply be the first N threads returned by the dumper (although + // the crashing thread will never be limited). Threads beyond this count are + // the extra threads. + static const unsigned kLimitBaseThreadCount = 20; + // Maximum stack size to dump for any extra thread (in bytes). + static const unsigned kLimitMaxExtraThreadStackLen = 2 * 1024; + // Make sure this number of additional bytes can fit in the minidump + // (exclude the stack data). + static const unsigned kLimitMinidumpFudgeFactor = 64 * 1024; + MinidumpWriter(const char* minidump_path, int minidump_fd, const ExceptionHandler::CrashContext* context, @@ -391,6 +408,7 @@ class MinidumpWriter { float_state_(NULL), #endif dumper_(dumper), + minidump_size_limit_(-1), memory_blocks_(dumper_->allocator()), mapping_list_(mappings), app_memory_list_(appmem) { @@ -629,12 +647,14 @@ class MinidumpWriter { } bool FillThreadStack(MDRawThread* thread, uintptr_t stack_pointer, - uint8_t** stack_copy) { + int max_stack_len, uint8_t** stack_copy) { *stack_copy = NULL; const void* stack; size_t stack_len; if (dumper_->GetStackInfo(&stack, &stack_len, stack_pointer)) { UntypedMDRVA memory(&minidump_writer_); + if (max_stack_len >= 0 && stack_len > max_stack_len) + stack_len = max_stack_len; if (!memory.Allocate(stack_len)) return false; *stack_copy = reinterpret_cast<uint8_t*>(Alloc(stack_len)); @@ -666,6 +686,21 @@ class MinidumpWriter { *list.get() = num_threads; + // If there's a minidump size limit, check if it might be exceeded. Since + // most of the space is filled with stack data, just check against that. + // If this expects to exceed the limit, set extra_thread_stack_len such + // that any thread beyond the first kLimitBaseThreadCount threads will + // have only kLimitMaxExtraThreadStackLen bytes dumped. + int extra_thread_stack_len = -1; // default to no maximum + if (minidump_size_limit_ >= 0) { + const unsigned estimated_total_stack_size = num_threads * + kLimitAverageThreadStackLength; + const off_t estimated_minidump_size = minidump_writer_.position() + + estimated_total_stack_size + kLimitMinidumpFudgeFactor; + if (estimated_minidump_size > minidump_size_limit_) + extra_thread_stack_len = kLimitMaxExtraThreadStackLen; + } + for (unsigned i = 0; i < num_threads; ++i) { MDRawThread thread; my_memset(&thread, 0, sizeof(thread)); @@ -679,7 +714,7 @@ class MinidumpWriter { ucontext_ && !dumper_->IsPostMortem()) { uint8_t* stack_copy; - if (!FillThreadStack(&thread, GetStackPointer(), &stack_copy)) + if (!FillThreadStack(&thread, GetStackPointer(), -1, &stack_copy)) return false; // Copy 256 bytes around crashing instruction pointer to minidump. @@ -740,7 +775,11 @@ class MinidumpWriter { return false; uint8_t* stack_copy; - if (!FillThreadStack(&thread, info.stack_pointer, &stack_copy)) + int max_stack_len = -1; // default to no maximum for this thread + if (minidump_size_limit_ >= 0 && i >= kLimitBaseThreadCount) + max_stack_len = extra_thread_stack_len; + if (!FillThreadStack(&thread, info.stack_pointer, max_stack_len, + &stack_copy)) return false; TypedMDRVA<RawContextCPU> cpu(&minidump_writer_); @@ -1112,6 +1151,8 @@ class MinidumpWriter { return true; } + void set_minidump_size_limit(off_t limit) { minidump_size_limit_ = limit; } + private: void* Alloc(unsigned bytes) { return dumper_->allocator()->Alloc(bytes); @@ -1437,6 +1478,7 @@ class MinidumpWriter { const struct _libc_fpstate* const float_state_; // ditto LinuxDumper* dumper_; MinidumpFileWriter minidump_writer_; + off_t minidump_size_limit_; MDLocationDescriptor crashing_thread_context_; // Blocks of memory written to the dump. These are all currently // written while writing the thread list stream, but saved here @@ -1452,21 +1494,26 @@ class MinidumpWriter { bool WriteMinidumpImpl(const char* minidump_path, int minidump_fd, + off_t minidump_size_limit, pid_t crashing_process, const void* blob, size_t blob_size, const MappingList& mappings, const AppMemoryList& appmem) { - if (blob_size != sizeof(ExceptionHandler::CrashContext)) - return false; - const ExceptionHandler::CrashContext* context = - reinterpret_cast<const ExceptionHandler::CrashContext*>(blob); LinuxPtraceDumper dumper(crashing_process); - dumper.set_crash_address( - reinterpret_cast<uintptr_t>(context->siginfo.si_addr)); - dumper.set_crash_signal(context->siginfo.si_signo); - dumper.set_crash_thread(context->tid); + const ExceptionHandler::CrashContext* context = NULL; + if (blob) { + if (blob_size != sizeof(ExceptionHandler::CrashContext)) + return false; + context = reinterpret_cast<const ExceptionHandler::CrashContext*>(blob); + dumper.set_crash_address( + reinterpret_cast<uintptr_t>(context->siginfo.si_addr)); + dumper.set_crash_signal(context->siginfo.si_signo); + dumper.set_crash_thread(context->tid); + } MinidumpWriter writer(minidump_path, minidump_fd, context, mappings, appmem, &dumper); + // Set desired limit for file size of minidump (-1 means no limit). + writer.set_minidump_size_limit(minidump_size_limit); if (!writer.Init()) return false; return writer.Dump(); @@ -1478,13 +1525,15 @@ namespace google_breakpad { bool WriteMinidump(const char* minidump_path, pid_t crashing_process, const void* blob, size_t blob_size) { - return WriteMinidumpImpl(minidump_path, -1, crashing_process, blob, blob_size, + return WriteMinidumpImpl(minidump_path, -1, -1, + crashing_process, blob, blob_size, MappingList(), AppMemoryList()); } bool WriteMinidump(int minidump_fd, pid_t crashing_process, const void* blob, size_t blob_size) { - return WriteMinidumpImpl(NULL, minidump_fd, crashing_process, blob, blob_size, + return WriteMinidumpImpl(NULL, minidump_fd, -1, + crashing_process, blob, blob_size, MappingList(), AppMemoryList()); } @@ -1505,7 +1554,8 @@ bool WriteMinidump(const char* minidump_path, pid_t crashing_process, const void* blob, size_t blob_size, const MappingList& mappings, const AppMemoryList& appmem) { - return WriteMinidumpImpl(minidump_path, -1, crashing_process, blob, blob_size, + return WriteMinidumpImpl(minidump_path, -1, -1, crashing_process, + blob, blob_size, mappings, appmem); } @@ -1513,7 +1563,28 @@ bool WriteMinidump(int minidump_fd, pid_t crashing_process, const void* blob, size_t blob_size, const MappingList& mappings, const AppMemoryList& appmem) { - return WriteMinidumpImpl(NULL, minidump_fd, crashing_process, blob, blob_size, + return WriteMinidumpImpl(NULL, minidump_fd, -1, crashing_process, + blob, blob_size, + mappings, appmem); +} + +bool WriteMinidump(const char* minidump_path, off_t minidump_size_limit, + pid_t crashing_process, + const void* blob, size_t blob_size, + const MappingList& mappings, + const AppMemoryList& appmem) { + return WriteMinidumpImpl(minidump_path, -1, minidump_size_limit, + crashing_process, blob, blob_size, + mappings, appmem); +} + +bool WriteMinidump(int minidump_fd, off_t minidump_size_limit, + pid_t crashing_process, + const void* blob, size_t blob_size, + const MappingList& mappings, + const AppMemoryList& appmem) { + return WriteMinidumpImpl(NULL, minidump_fd, minidump_size_limit, + crashing_process, blob, blob_size, mappings, appmem); } diff --git a/src/client/linux/minidump_writer/minidump_writer.h b/src/client/linux/minidump_writer/minidump_writer.h index 28bfbdaf..790ff723 100644 --- a/src/client/linux/minidump_writer/minidump_writer.h +++ b/src/client/linux/minidump_writer/minidump_writer.h @@ -31,6 +31,7 @@ #define CLIENT_LINUX_MINIDUMP_WRITER_MINIDUMP_WRITER_H_ #include <stdint.h> +#include <sys/types.h> #include <unistd.h> #include <list> @@ -102,6 +103,18 @@ bool WriteMinidump(int minidump_fd, pid_t crashing_process, const MappingList& mappings, const AppMemoryList& appdata); +// These overloads also allow passing a file size limit for the minidump. +bool WriteMinidump(const char* minidump_path, off_t minidump_size_limit, + pid_t crashing_process, + const void* blob, size_t blob_size, + const MappingList& mappings, + const AppMemoryList& appdata); +bool WriteMinidump(int minidump_fd, off_t minidump_size_limit, + pid_t crashing_process, + const void* blob, size_t blob_size, + const MappingList& mappings, + const AppMemoryList& appdata); + bool WriteMinidump(const char* filename, const MappingList& mappings, const AppMemoryList& appdata, diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest.cc b/src/client/linux/minidump_writer/minidump_writer_unittest.cc index 153f3528..6beae05d 100644 --- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc +++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc @@ -580,4 +580,156 @@ TEST(MinidumpWriterTest, InvalidStackPointer) { close(fds[1]); } +// Test that limiting the size of the minidump works. +TEST(MinidumpWriterTest, MinidumpSizeLimit) { + static const int kNumberOfThreadsInHelperProgram = 40; + + char number_of_threads_arg[3]; + sprintf(number_of_threads_arg, "%d", kNumberOfThreadsInHelperProgram); + + string helper_path(GetHelperBinary()); + if (helper_path.empty()) { + FAIL() << "Couldn't find helper binary"; + exit(1); + } + + int fds[2]; + ASSERT_NE(-1, pipe(fds)); + + pid_t child_pid = fork(); + if (child_pid == 0) { + // In child process. + close(fds[0]); + + // Pass the pipe fd and the number of threads as arguments. + char pipe_fd_string[8]; + sprintf(pipe_fd_string, "%d", fds[1]); + execl(helper_path.c_str(), + helper_path.c_str(), + pipe_fd_string, + number_of_threads_arg, + NULL); + } + close(fds[1]); + + // Wait for all child threads to indicate that they have started + for (int threads = 0; threads < kNumberOfThreadsInHelperProgram; threads++) { + struct pollfd pfd; + memset(&pfd, 0, sizeof(pfd)); + pfd.fd = fds[0]; + pfd.events = POLLIN | POLLERR; + + const int r = HANDLE_EINTR(poll(&pfd, 1, 1000)); + ASSERT_EQ(1, r); + ASSERT_TRUE(pfd.revents & POLLIN); + uint8_t junk; + ASSERT_EQ(read(fds[0], &junk, sizeof(junk)), sizeof(junk)); + } + close(fds[0]); + + // There is a race here because we may stop a child thread before + // it is actually running the busy loop. Empirically this sleep + // is sufficient to avoid the race. + usleep(100000); + + // Child and its threads are ready now. + + + off_t normal_file_size; + int total_normal_stack_size = 0; + AutoTempDir temp_dir; + + // First, write a minidump with no size limit. + { + string normal_dump = temp_dir.path() + + "/minidump-writer-unittest.dmp"; + ASSERT_TRUE(WriteMinidump(normal_dump.c_str(), -1, + child_pid, NULL, 0, + MappingList(), AppMemoryList())); + struct stat st; + ASSERT_EQ(0, stat(normal_dump.c_str(), &st)); + ASSERT_GT(st.st_size, 0u); + normal_file_size = st.st_size; + + Minidump minidump(normal_dump.c_str()); + ASSERT_TRUE(minidump.Read()); + MinidumpThreadList* dump_thread_list = minidump.GetThreadList(); + ASSERT_TRUE(dump_thread_list); + for (int i = 0; i < dump_thread_list->thread_count(); i++) { + MinidumpThread* thread = dump_thread_list->GetThreadAtIndex(i); + ASSERT_TRUE(thread->thread() != NULL); + // When the stack size is zero bytes, GetMemory() returns NULL. + MinidumpMemoryRegion* memory = thread->GetMemory(); + ASSERT_TRUE(memory != NULL); + total_normal_stack_size += memory->GetSize(); + } + } + + // Second, write a minidump with a size limit big enough to not trigger + // anything. + { + // Set size limit arbitrarily 1MB larger than the normal file size -- such + // that the limiting code will not kick in. + const off_t minidump_size_limit = normal_file_size + 1024*1024; + + string same_dump = temp_dir.path() + + "/minidump-writer-unittest-same.dmp"; + ASSERT_TRUE(WriteMinidump(same_dump.c_str(), minidump_size_limit, + child_pid, NULL, 0, + MappingList(), AppMemoryList())); + struct stat st; + ASSERT_EQ(0, stat(same_dump.c_str(), &st)); + // Make sure limiting wasn't actually triggered. NOTE: If you fail this, + // first make sure that "minidump_size_limit" above is indeed set to a + // large enough value -- the limit-checking code in minidump_writer.cc + // does just a rough estimate. + ASSERT_EQ(normal_file_size, st.st_size); + } + + // Third, write a minidump with a size limit small enough to be triggered. + { + // Set size limit to the normal file size minus some arbitrary amount -- + // enough to make the limiting code kick in. + const off_t minidump_size_limit = normal_file_size - 64*1024; + + string limit_dump = temp_dir.path() + + "/minidump-writer-unittest-limit.dmp"; + ASSERT_TRUE(WriteMinidump(limit_dump.c_str(), minidump_size_limit, + child_pid, NULL, 0, + MappingList(), AppMemoryList())); + struct stat st; + ASSERT_EQ(0, stat(limit_dump.c_str(), &st)); + ASSERT_GT(st.st_size, 0u); + // Make sure the file size is at least smaller than the original. + EXPECT_LT(st.st_size, normal_file_size); + + Minidump minidump(limit_dump.c_str()); + ASSERT_TRUE(minidump.Read()); + MinidumpThreadList* dump_thread_list = minidump.GetThreadList(); + ASSERT_TRUE(dump_thread_list); + int total_limit_stack_size = 0; + for (int i = 0; i < dump_thread_list->thread_count(); i++) { + MinidumpThread* thread = dump_thread_list->GetThreadAtIndex(i); + ASSERT_TRUE(thread->thread() != NULL); + // When the stack size is zero bytes, GetMemory() returns NULL. + MinidumpMemoryRegion* memory = thread->GetMemory(); + ASSERT_TRUE(memory != NULL); + total_limit_stack_size += memory->GetSize(); + } + + // Make sure stack size shrunk by at least 1KB per extra thread. The + // definition of kLimitBaseThreadCount here was copied from class + // MinidumpWriter in minidump_writer.cc. + const unsigned kLimitBaseThreadCount = 20; + const unsigned kMinPerExtraThreadStackReduction = 1024; + const int min_expected_reduction = (kNumberOfThreadsInHelperProgram - + kLimitBaseThreadCount) * kMinPerExtraThreadStackReduction; + EXPECT_LT(total_limit_stack_size, + total_normal_stack_size - min_expected_reduction); + } + + // Kill the helper program. + kill(child_pid, SIGKILL); +} + } // namespace |