Wire up stack sanitization and skipping to WriteMinidump

This makes the parameters stored in the MinidumpDescriptor structure
functional for minidumps, analogously to how they are applied to
microdumps.

BUG=664460

Change-Id: I7578e7a1638cea8f0445b18d4bbdaf5e0a32d808
Reviewed-on: https://chromium-review.googlesource.com/435380
Reviewed-by: Robert Sesek <rsesek@chromium.org>

src/client/linux/handler/exception_handler.cc

@@ -586,15 +586,20 @@ void ExceptionHandler::WaitForContinueSignal() {
 // Runs on the cloned process.
 bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context,
                               size_t context_size) {
+  const bool may_skip_dump =
+      minidump_descriptor_.skip_dump_if_principal_mapping_not_referenced();
+  const uintptr_t principal_mapping_address =
+      minidump_descriptor_.address_within_principal_mapping();
+  const bool sanitize_stacks = minidump_descriptor_.sanitize_stacks();
   if (minidump_descriptor_.IsMicrodumpOnConsole()) {
     return google_breakpad::WriteMicrodump(
         crashing_process,
         context,
         context_size,
         mapping_list_,
-        minidump_descriptor_.skip_dump_if_principal_mapping_not_referenced(),
+        may_skip_dump,
-        minidump_descriptor_.address_within_principal_mapping(),
+        principal_mapping_address,
-        minidump_descriptor_.sanitize_stacks(),
+        sanitize_stacks,
         *minidump_descriptor_.microdump_extra_info());
   }
   if (minidump_descriptor_.IsFD()) {
@@ -604,7 +609,10 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context,
                                           context,
                                           context_size,
                                           mapping_list_,
-                                          app_memory_list_);
+                                          app_memory_list_,
+                                          may_skip_dump,
+                                          principal_mapping_address,
+                                          sanitize_stacks);
   }
   return google_breakpad::WriteMinidump(minidump_descriptor_.path(),
                                         minidump_descriptor_.size_limit(),
@@ -612,7 +620,10 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context,
                                         context,
                                         context_size,
                                         mapping_list_,
-                                        app_memory_list_);
+                                        app_memory_list_,
+                                        may_skip_dump,
+                                        principal_mapping_address,
+                                        sanitize_stacks);
 }
 
 // static

src/client/linux/microdump_writer/microdump_writer.cc

@@ -265,8 +265,8 @@ class MicrodumpWriter {
         dumper_->FindMappingNoBias(address_within_principal_mapping_);
     if (!principal_mapping) return CAPTURE_UNINTERESTING;
 
-    uintptr_t low_addr = principal_mapping->start_addr;
+    uintptr_t low_addr = principal_mapping->system_mapping_info.start_addr;
-    uintptr_t high_addr = principal_mapping->start_addr + principal_mapping->size;
+    uintptr_t high_addr = principal_mapping->system_mapping_info.end_addr;
     uintptr_t pc = UContextReader::GetInstructionPointer(ucontext_);
     if (low_addr <= pc && pc <= high_addr) return CAPTURE_OK;
 

src/client/linux/minidump_writer/minidump_writer.cc

@@ -129,6 +129,9 @@ class MinidumpWriter {
                  const ExceptionHandler::CrashContext* context,
                  const MappingList& mappings,
                  const AppMemoryList& appmem,
+                 bool skip_stacks_if_mapping_unreferenced,
+                 uintptr_t principal_mapping_address,
+                 bool sanitize_stacks,
                  LinuxDumper* dumper)
       : fd_(minidump_fd),
         path_(minidump_path),
@@ -140,7 +143,11 @@ class MinidumpWriter {
         minidump_size_limit_(-1),
         memory_blocks_(dumper_->allocator()),
         mapping_list_(mappings),
-        app_memory_list_(appmem) {
+        app_memory_list_(appmem),
+        skip_stacks_if_mapping_unreferenced_(
+            skip_stacks_if_mapping_unreferenced),
+        principal_mapping_address_(principal_mapping_address),
+    sanitize_stacks_(sanitize_stacks) {
     // Assert there should be either a valid fd or a valid path, not both.
     assert(fd_ != -1 || minidump_path);
     assert(fd_ == -1 || !minidump_path);
@@ -270,8 +277,12 @@ class MinidumpWriter {
     *stack_copy = NULL;
     const void* stack;
     size_t stack_len;
+
+    thread->stack.start_of_memory_range = stack_pointer;
+    thread->stack.memory.data_size = 0;
+    thread->stack.memory.rva = minidump_writer_.position();
+
     if (dumper_->GetStackInfo(&stack, &stack_len, stack_pointer)) {
-      UntypedMDRVA memory(&minidump_writer_);
       if (max_stack_len >= 0 &&
           stack_len > static_cast<unsigned int>(max_stack_len)) {
         stack_len = max_stack_len;
@@ -284,20 +295,41 @@ class MinidumpWriter {
         }
         stack = reinterpret_cast<const void*>(int_stack);
       }
-      if (!memory.Allocate(stack_len))
-        return false;
       *stack_copy = reinterpret_cast<uint8_t*>(Alloc(stack_len));
       dumper_->CopyFromProcess(*stack_copy, thread->thread_id, stack,
                                stack_len);
+
+      uintptr_t stack_pointer_offset =
+          stack_pointer - reinterpret_cast<uintptr_t>(stack);
+      if (skip_stacks_if_mapping_unreferenced_) {
+        const MappingInfo* principal_mapping =
+            dumper_->FindMappingNoBias(principal_mapping_address_);
+        if (!principal_mapping) {
+          return true;
+        }
+        uintptr_t low_addr = principal_mapping->system_mapping_info.start_addr;
+        uintptr_t high_addr = principal_mapping->system_mapping_info.end_addr;
+        uintptr_t pc = UContextReader::GetInstructionPointer(ucontext_);
+        if ((pc < low_addr || pc > high_addr) &&
+            !dumper_->StackHasPointerToMapping(*stack_copy, stack_len,
+                                               stack_pointer_offset,
+                                               *principal_mapping)) {
+          return true;
+        }
+      }
+
+      if (sanitize_stacks_) {
+        dumper_->SanitizeStackCopy(*stack_copy, stack_len, stack_pointer,
+                                   stack_pointer_offset);
+      }
+
+      UntypedMDRVA memory(&minidump_writer_);
+      if (!memory.Allocate(stack_len))
+        return false;
       memory.Copy(*stack_copy, stack_len);
-      thread->stack.start_of_memory_range =
+      thread->stack.start_of_memory_range = reinterpret_cast<uintptr_t>(stack);
-          reinterpret_cast<uintptr_t>(stack);
       thread->stack.memory = memory.location();
       memory_blocks_.push_back(thread->stack);
-    } else {
-      thread->stack.start_of_memory_range = stack_pointer;
-      thread->stack.memory.data_size = 0;
-      thread->stack.memory.rva = minidump_writer_.position();
     }
     return true;
   }
@@ -1265,6 +1297,12 @@ class MinidumpWriter {
   // Additional memory regions to be included in the dump,
   // provided by the caller.
   const AppMemoryList& app_memory_list_;
+  // If set, skip recording any threads that do not reference the
+  // mapping containing principal_mapping_address_.
+  bool skip_stacks_if_mapping_unreferenced_;
+  uintptr_t principal_mapping_address_;
+  // If true, apply stack sanitization to stored stack data.
+  bool sanitize_stacks_;
 };
 
 
@@ -1274,7 +1312,10 @@ bool WriteMinidumpImpl(const char* minidump_path,
                        pid_t crashing_process,
                        const void* blob, size_t blob_size,
                        const MappingList& mappings,
-                       const AppMemoryList& appmem) {
+                       const AppMemoryList& appmem,
+                       bool skip_stacks_if_mapping_unreferenced,
+                       uintptr_t principal_mapping_address,
+                       bool sanitize_stacks) {
   LinuxPtraceDumper dumper(crashing_process);
   const ExceptionHandler::CrashContext* context = NULL;
   if (blob) {
@@ -1287,7 +1328,8 @@ bool WriteMinidumpImpl(const char* minidump_path,
     dumper.set_crash_thread(context->tid);
   }
   MinidumpWriter writer(minidump_path, minidump_fd, context, mappings,
-                        appmem, &dumper);
+                        appmem, skip_stacks_if_mapping_unreferenced,
+                        principal_mapping_address, sanitize_stacks, &dumper);
   // Set desired limit for file size of minidump (-1 means no limit).
   writer.set_minidump_size_limit(minidump_size_limit);
   if (!writer.Init())
@@ -1300,17 +1342,29 @@ bool WriteMinidumpImpl(const char* minidump_path,
 namespace google_breakpad {
 
 bool WriteMinidump(const char* minidump_path, pid_t crashing_process,
-                   const void* blob, size_t blob_size) {
+                   const void* blob, size_t blob_size,
+                   bool skip_stacks_if_mapping_unreferenced,
+                   uintptr_t principal_mapping_address,
+                   bool sanitize_stacks) {
   return WriteMinidumpImpl(minidump_path, -1, -1,
                            crashing_process, blob, blob_size,
-                           MappingList(), AppMemoryList());
+                           MappingList(), AppMemoryList(),
+                           skip_stacks_if_mapping_unreferenced,
+                           principal_mapping_address,
+                           sanitize_stacks);
 }
 
 bool WriteMinidump(int minidump_fd, pid_t crashing_process,
-                   const void* blob, size_t blob_size) {
+                   const void* blob, size_t blob_size,
+                   bool skip_stacks_if_mapping_unreferenced,
+                   uintptr_t principal_mapping_address,
+                   bool sanitize_stacks) {
   return WriteMinidumpImpl(NULL, minidump_fd, -1,
                            crashing_process, blob, blob_size,
-                           MappingList(), AppMemoryList());
+                           MappingList(), AppMemoryList(),
+                           skip_stacks_if_mapping_unreferenced,
+                           principal_mapping_address,
+                           sanitize_stacks);
 }
 
 bool WriteMinidump(const char* minidump_path, pid_t process,
@@ -1320,7 +1374,7 @@ bool WriteMinidump(const char* minidump_path, pid_t process,
   dumper.set_crash_signal(MD_EXCEPTION_CODE_LIN_DUMP_REQUESTED);
   dumper.set_crash_thread(process_blamed_thread);
   MinidumpWriter writer(minidump_path, -1, NULL, MappingList(),
-                        AppMemoryList(), &dumper);
+                        AppMemoryList(), false, 0, false, &dumper);
   if (!writer.Init())
     return false;
   return writer.Dump();
@@ -1329,46 +1383,71 @@ bool WriteMinidump(const char* minidump_path, pid_t process,
 bool WriteMinidump(const char* minidump_path, pid_t crashing_process,
                    const void* blob, size_t blob_size,
                    const MappingList& mappings,
-                   const AppMemoryList& appmem) {
+                   const AppMemoryList& appmem,
+                   bool skip_stacks_if_mapping_unreferenced,
+                   uintptr_t principal_mapping_address,
+                   bool sanitize_stacks) {
   return WriteMinidumpImpl(minidump_path, -1, -1, crashing_process,
                            blob, blob_size,
-                           mappings, appmem);
+                           mappings, appmem,
+                           skip_stacks_if_mapping_unreferenced,
+                           principal_mapping_address,
+                           sanitize_stacks);
 }
 
 bool WriteMinidump(int minidump_fd, pid_t crashing_process,
                    const void* blob, size_t blob_size,
                    const MappingList& mappings,
-                   const AppMemoryList& appmem) {
+                   const AppMemoryList& appmem,
+                   bool skip_stacks_if_mapping_unreferenced,
+                   uintptr_t principal_mapping_address,
+                   bool sanitize_stacks) {
   return WriteMinidumpImpl(NULL, minidump_fd, -1, crashing_process,
                            blob, blob_size,
-                           mappings, appmem);
+                           mappings, appmem,
+                           skip_stacks_if_mapping_unreferenced,
+                           principal_mapping_address,
+                           sanitize_stacks);
 }
 
 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) {
+                   const AppMemoryList& appmem,
+                   bool skip_stacks_if_mapping_unreferenced,
+                   uintptr_t principal_mapping_address,
+                   bool sanitize_stacks) {
   return WriteMinidumpImpl(minidump_path, -1, minidump_size_limit,
                            crashing_process, blob, blob_size,
-                           mappings, appmem);
+                           mappings, appmem,
+                           skip_stacks_if_mapping_unreferenced,
+                           principal_mapping_address,
+                           sanitize_stacks);
 }
 
 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) {
+                   const AppMemoryList& appmem,
+                   bool skip_stacks_if_mapping_unreferenced,
+                   uintptr_t principal_mapping_address,
+                   bool sanitize_stacks) {
   return WriteMinidumpImpl(NULL, minidump_fd, minidump_size_limit,
                            crashing_process, blob, blob_size,
-                           mappings, appmem);
+                           mappings, appmem,
+                           skip_stacks_if_mapping_unreferenced,
+                           principal_mapping_address,
+                           sanitize_stacks);
 }
 
 bool WriteMinidump(const char* filename,
                    const MappingList& mappings,
                    const AppMemoryList& appmem,
                    LinuxDumper* dumper) {
-  MinidumpWriter writer(filename, -1, NULL, mappings, appmem, dumper);
+  MinidumpWriter writer(filename, -1, NULL, mappings, appmem,
+                        false, 0, false, dumper);
   if (!writer.Init())
     return false;
   return writer.Dump();

src/client/linux/minidump_writer/minidump_writer.h

@@ -78,10 +78,16 @@ typedef std::list<AppMemory> AppMemoryList;
 //
 // Returns true iff successful.
 bool WriteMinidump(const char* minidump_path, pid_t crashing_process,
-                   const void* blob, size_t blob_size);
+                   const void* blob, size_t blob_size,
+                   bool skip_stacks_if_mapping_unreferenced = false,
+                   uintptr_t principal_mapping_address = 0,
+                   bool sanitize_stacks = false);
 // Same as above but takes an open file descriptor instead of a path.
 bool WriteMinidump(int minidump_fd, pid_t crashing_process,
-                   const void* blob, size_t blob_size);
+                   const void* blob, size_t blob_size,
+                   bool skip_stacks_if_mapping_unreferenced = false,
+                   uintptr_t principal_mapping_address = 0,
+                   bool sanitize_stacks = false);
 
 // Alternate form of WriteMinidump() that works with processes that
 // are not expected to have crashed.  If |process_blamed_thread| is
@@ -96,23 +102,35 @@ bool WriteMinidump(const char* minidump_path, pid_t process,
 bool WriteMinidump(const char* minidump_path, pid_t crashing_process,
                    const void* blob, size_t blob_size,
                    const MappingList& mappings,
-                   const AppMemoryList& appdata);
+                   const AppMemoryList& appdata,
+                   bool skip_stacks_if_mapping_unreferenced = false,
+                   uintptr_t principal_mapping_address = 0,
+                   bool sanitize_stacks = false);
 bool WriteMinidump(int minidump_fd, pid_t crashing_process,
                    const void* blob, size_t blob_size,
                    const MappingList& mappings,
-                   const AppMemoryList& appdata);
+                   const AppMemoryList& appdata,
+                   bool skip_stacks_if_mapping_unreferenced = false,
+                   uintptr_t principal_mapping_address = 0,
+                   bool sanitize_stacks = false);
 
 // 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);
+                   const AppMemoryList& appdata,
+                   bool skip_stacks_if_mapping_unreferenced = false,
+                   uintptr_t principal_mapping_address = 0,
+                   bool sanitize_stacks = false);
 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);
+                   const AppMemoryList& appdata,
+                   bool skip_stacks_if_mapping_unreferenced = false,
+                   uintptr_t principal_mapping_address = 0,
+                   bool sanitize_stacks = false);
 
 bool WriteMinidump(const char* filename,
                    const MappingList& mappings,

src/client/linux/minidump_writer/minidump_writer_unittest.cc

@@ -179,7 +179,7 @@ TEST(MinidumpWriterTest, MappingInfo) {
   memcpy(mapping.second, kModuleGUID, sizeof(MDGUID));
   mappings.push_back(mapping);
   ASSERT_TRUE(WriteMinidump(templ.c_str(), child, &context, sizeof(context),
-                            mappings, memory_list));
+                            mappings, memory_list, false, 0, false));
 
   // Read the minidump. Load the module list, and ensure that
   // the mmap'ed |memory| is listed with the given module name
@@ -215,6 +215,90 @@ TEST(MinidumpWriterTest, MappingInfo) {
   close(fds[1]);
 }
 
+// Test that stacks can be skipped while writing minidumps.
+TEST(MinidumpWriterTest, StacksSkippedIfRequested) {
+  int fds[2];
+  ASSERT_NE(-1, pipe(fds));
+
+  const pid_t child = fork();
+  if (child == 0) {
+    close(fds[1]);
+    char b;
+    IGNORE_RET(HANDLE_EINTR(read(fds[0], &b, sizeof(b))));
+    close(fds[0]);
+    syscall(__NR_exit);
+  }
+  close(fds[0]);
+
+  ExceptionHandler::CrashContext context;
+  memset(&context, 0, sizeof(context));
+  ASSERT_EQ(0, getcontext(&context.context));
+  context.tid = child;
+
+  AutoTempDir temp_dir;
+  string templ = temp_dir.path() + kMDWriterUnitTestFileName;
+
+  // pass an invalid principal mapping address, which will force
+  // WriteMinidump to not dump any thread stacks.
+  ASSERT_TRUE(WriteMinidump(templ.c_str(), child, &context, sizeof(context),
+                            true, 0x0102030405060708, false));
+
+  // Read the minidump. And ensure that no thread memory was dumped.
+  Minidump minidump(templ);
+  ASSERT_TRUE(minidump.Read());
+
+  MinidumpThreadList *threads = minidump.GetThreadList();
+  for (unsigned int i = 0; i < threads->thread_count(); ++i) {
+    MinidumpThread *thread = threads->GetThreadAtIndex(i);
+    ASSERT_TRUE(thread->GetMemory() == nullptr);
+  }
+  close(fds[1]);
+}
+
+// Test that stacks can be sanitized while writing minidumps.
+TEST(MinidumpWriterTest, StacksAreSanitizedIfRequested) {
+  int fds[2];
+  ASSERT_NE(-1, pipe(fds));
+
+  const pid_t child = fork();
+  if (child == 0) {
+    close(fds[1]);
+    char b;
+    IGNORE_RET(HANDLE_EINTR(read(fds[0], &b, sizeof(b))));
+    close(fds[0]);
+    syscall(__NR_exit);
+  }
+  close(fds[0]);
+
+  ExceptionHandler::CrashContext context;
+  memset(&context, 0, sizeof(context));
+  ASSERT_EQ(0, getcontext(&context.context));
+  context.tid = child;
+
+  AutoTempDir temp_dir;
+  string templ = temp_dir.path() + kMDWriterUnitTestFileName;
+  // pass an invalid principal mapping address, which will force
+  // WriteMinidump to not dump any thread stacks.
+  ASSERT_TRUE(WriteMinidump(templ.c_str(), child, &context, sizeof(context),
+                            false, 0, true));
+
+  // Read the minidump. And ensure that thread memory contains a defaced value.
+  Minidump minidump(templ);
+  ASSERT_TRUE(minidump.Read());
+
+  const uintptr_t defaced = 0X0DEFACED0DEFACEDull;
+  MinidumpThreadList *threads = minidump.GetThreadList();
+  for (unsigned int i = 0; i < threads->thread_count(); ++i) {
+    MinidumpThread *thread = threads->GetThreadAtIndex(i);
+    MinidumpMemoryRegion *mem = thread->GetMemory();
+    ASSERT_TRUE(mem != nullptr);
+    uint32_t sz = mem->GetSize();
+    const uint8_t *data = mem->GetMemory();
+    ASSERT_TRUE(memmem(data, sz, &defaced, sizeof(defaced)) != nullptr);
+  }
+  close(fds[1]);
+}
+
 // Test that a binary with a longer-than-usual build id note
 // makes its way all the way through to the minidump unscathed.
 // The linux_client_unittest is linked with an explicit --build-id