From 7c2799f3ba6f8a8186c8883b213c3e59768b1287 Mon Sep 17 00:00:00 2001 From: Tobias Sargeant Date: Tue, 31 Jan 2017 13:42:52 +0000 Subject: [PATCH] Sanitize dumped stacks to remove data that may be identifiable. In order to sanitize the stack contents we erase any pointer-aligned word that could not be interpreted as a pointer into one of the processes' memory mappings, or a small integer (+/-4096). This still retains enough information to unwind stack frames, and also to recover some register values. BUG=682278 Change-Id: I541a13b2e92a9d1aea2c06a50bd769a9e25601d3 Reviewed-on: https://chromium-review.googlesource.com/430050 Reviewed-by: Robert Sesek --- src/client/linux/handler/exception_handler.cc | 1 + .../linux/handler/minidump_descriptor.cc | 2 + .../linux/handler/minidump_descriptor.h | 21 +- .../microdump_writer/microdump_writer.cc | 16 +- .../linux/microdump_writer/microdump_writer.h | 1 + .../microdump_writer_unittest.cc | 84 +++++++- .../linux/minidump_writer/linux_dumper.cc | 105 +++++++++- .../linux/minidump_writer/linux_dumper.h | 13 ++ .../linux_ptrace_dumper_unittest.cc | 181 +++++++++++++----- 9 files changed, 362 insertions(+), 62 deletions(-) diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc index 8565bbb0..dd3cbc67 100644 --- a/src/client/linux/handler/exception_handler.cc +++ b/src/client/linux/handler/exception_handler.cc @@ -594,6 +594,7 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context, mapping_list_, minidump_descriptor_.skip_dump_if_principal_mapping_not_referenced(), minidump_descriptor_.address_within_principal_mapping(), + minidump_descriptor_.sanitize_stacks(), *minidump_descriptor_.microdump_extra_info()); } if (minidump_descriptor_.IsFD()) { diff --git a/src/client/linux/handler/minidump_descriptor.cc b/src/client/linux/handler/minidump_descriptor.cc index cdb5bf03..bd94474e 100644 --- a/src/client/linux/handler/minidump_descriptor.cc +++ b/src/client/linux/handler/minidump_descriptor.cc @@ -49,6 +49,7 @@ MinidumpDescriptor::MinidumpDescriptor(const MinidumpDescriptor& descriptor) descriptor.address_within_principal_mapping_), skip_dump_if_principal_mapping_not_referenced_( descriptor.skip_dump_if_principal_mapping_not_referenced_), + sanitize_stacks_(descriptor.sanitize_stacks_), microdump_extra_info_(descriptor.microdump_extra_info_) { // The copy constructor is not allowed to be called on a MinidumpDescriptor // with a valid path_, as getting its c_path_ would require the heap which @@ -74,6 +75,7 @@ MinidumpDescriptor& MinidumpDescriptor::operator=( descriptor.address_within_principal_mapping_; skip_dump_if_principal_mapping_not_referenced_ = descriptor.skip_dump_if_principal_mapping_not_referenced_; + sanitize_stacks_ = descriptor.sanitize_stacks_; microdump_extra_info_ = descriptor.microdump_extra_info_; return *this; } diff --git a/src/client/linux/handler/minidump_descriptor.h b/src/client/linux/handler/minidump_descriptor.h index f601427c..911beaef 100644 --- a/src/client/linux/handler/minidump_descriptor.h +++ b/src/client/linux/handler/minidump_descriptor.h @@ -64,7 +64,8 @@ class MinidumpDescriptor { c_path_(NULL), size_limit_(-1), address_within_principal_mapping_(0), - skip_dump_if_principal_mapping_not_referenced_(false) { + skip_dump_if_principal_mapping_not_referenced_(false), + sanitize_stacks_(false) { assert(!directory.empty()); } @@ -74,7 +75,8 @@ class MinidumpDescriptor { c_path_(NULL), size_limit_(-1), address_within_principal_mapping_(0), - skip_dump_if_principal_mapping_not_referenced_(false) { + skip_dump_if_principal_mapping_not_referenced_(false), + sanitize_stacks_(false) { assert(fd != -1); } @@ -83,7 +85,8 @@ class MinidumpDescriptor { fd_(-1), size_limit_(-1), address_within_principal_mapping_(0), - skip_dump_if_principal_mapping_not_referenced_(false) {} + skip_dump_if_principal_mapping_not_referenced_(false), + sanitize_stacks_(false) {} explicit MinidumpDescriptor(const MinidumpDescriptor& descriptor); MinidumpDescriptor& operator=(const MinidumpDescriptor& descriptor); @@ -126,6 +129,11 @@ class MinidumpDescriptor { skip_dump_if_principal_mapping_not_referenced; } + bool sanitize_stacks() const { return sanitize_stacks_; } + void set_sanitize_stacks(bool sanitize_stacks) { + sanitize_stacks_ = sanitize_stacks; + } + MicrodumpExtraInfo* microdump_extra_info() { assert(IsMicrodumpOnConsole()); return µdump_extra_info_; @@ -167,6 +175,13 @@ class MinidumpDescriptor { // stacks logged. bool skip_dump_if_principal_mapping_not_referenced_; + // If set, stacks are sanitized to remove PII. This involves + // overwriting any pointer-aligned words that are not either + // pointers into a process mapping or small integers (+/-4096). This + // leaves enough information to unwind stacks, and preserve some + // register values, but elides strings and other program data. + bool sanitize_stacks_; + // The extra microdump data (e.g. product name/version, build // fingerprint, gpu fingerprint) that should be appended to the dump // (microdump only). Microdumps don't have the ability of appending diff --git a/src/client/linux/microdump_writer/microdump_writer.cc b/src/client/linux/microdump_writer/microdump_writer.cc index 8109a981..341d7f5c 100644 --- a/src/client/linux/microdump_writer/microdump_writer.cc +++ b/src/client/linux/microdump_writer/microdump_writer.cc @@ -134,6 +134,7 @@ class MicrodumpWriter { const MappingList& mappings, bool skip_dump_if_principal_mapping_not_referenced, uintptr_t address_within_principal_mapping, + bool sanitize_stack, const MicrodumpExtraInfo& microdump_extra_info, LinuxDumper* dumper) : ucontext_(context ? &context->context : NULL), @@ -145,6 +146,7 @@ class MicrodumpWriter { skip_dump_if_principal_mapping_not_referenced_( skip_dump_if_principal_mapping_not_referenced), address_within_principal_mapping_(address_within_principal_mapping), + sanitize_stack_(sanitize_stack), microdump_extra_info_(microdump_extra_info), log_line_(NULL), stack_copy_(NULL), @@ -368,6 +370,11 @@ class MicrodumpWriter { } void DumpThreadStack() { + if (sanitize_stack_) { + dumper_->SanitizeStackCopy(stack_copy_, stack_len_, stack_pointer_, + stack_pointer_ - stack_lower_bound_); + } + LogAppend("S 0 "); LogAppend(stack_pointer_); LogAppend(" "); @@ -580,6 +587,7 @@ class MicrodumpWriter { const MappingList& mapping_list_; bool skip_dump_if_principal_mapping_not_referenced_; uintptr_t address_within_principal_mapping_; + bool sanitize_stack_; const MicrodumpExtraInfo microdump_extra_info_; char* log_line_; @@ -607,6 +615,7 @@ bool WriteMicrodump(pid_t crashing_process, const MappingList& mappings, bool skip_dump_if_principal_mapping_not_referenced, uintptr_t address_within_principal_mapping, + bool sanitize_stack, const MicrodumpExtraInfo& microdump_extra_info) { LinuxPtraceDumper dumper(crashing_process); const ExceptionHandler::CrashContext* context = NULL; @@ -619,9 +628,10 @@ bool WriteMicrodump(pid_t crashing_process, dumper.set_crash_signal(context->siginfo.si_signo); dumper.set_crash_thread(context->tid); } - MicrodumpWriter writer( - context, mappings, skip_dump_if_principal_mapping_not_referenced, - address_within_principal_mapping, microdump_extra_info, &dumper); + MicrodumpWriter writer(context, mappings, + skip_dump_if_principal_mapping_not_referenced, + address_within_principal_mapping, sanitize_stack, + microdump_extra_info, &dumper); if (!writer.Init()) return false; writer.Dump(); diff --git a/src/client/linux/microdump_writer/microdump_writer.h b/src/client/linux/microdump_writer/microdump_writer.h index 78af86df..a1e53df6 100644 --- a/src/client/linux/microdump_writer/microdump_writer.h +++ b/src/client/linux/microdump_writer/microdump_writer.h @@ -60,6 +60,7 @@ bool WriteMicrodump(pid_t crashing_process, const MappingList& mappings, bool skip_dump_if_main_module_not_referenced, uintptr_t address_within_main_module, + bool sanitize_stack, const MicrodumpExtraInfo& microdump_extra_info); } // namespace google_breakpad diff --git a/src/client/linux/microdump_writer/microdump_writer_unittest.cc b/src/client/linux/microdump_writer/microdump_writer_unittest.cc index 5d028bd3..e5e6f448 100644 --- a/src/client/linux/microdump_writer/microdump_writer_unittest.cc +++ b/src/client/linux/microdump_writer/microdump_writer_unittest.cc @@ -73,11 +73,14 @@ bool ContainsMicrodump(const std::string& buf) { std::string::npos != buf.find("-----END BREAKPAD MICRODUMP-----"); } +const char kIdentifiableString[] = "_IDENTIFIABLE_"; + void CrashAndGetMicrodump(const MappingList& mappings, const MicrodumpExtraInfo& microdump_extra_info, std::string* microdump, bool skip_dump_if_principal_mapping_not_referenced = false, - uintptr_t address_within_principal_mapping = 0) { + uintptr_t address_within_principal_mapping = 0, + bool sanitize_stack = false) { int fds[2]; ASSERT_NE(-1, pipe(fds)); @@ -86,6 +89,14 @@ void CrashAndGetMicrodump(const MappingList& mappings, int err_fd = open(stderr_file.c_str(), O_CREAT | O_RDWR, S_IRUSR | S_IWUSR); ASSERT_NE(-1, err_fd); + char identifiable_string[sizeof(kIdentifiableString)]; + + // This string should not appear in the resulting microdump if it + // has been sanitized. + strcpy(identifiable_string, kIdentifiableString); + // Force the strcpy to not be optimized away. + write(STDOUT_FILENO, identifiable_string, 0); + const pid_t child = fork(); if (child == 0) { close(fds[1]); @@ -112,7 +123,8 @@ void CrashAndGetMicrodump(const MappingList& mappings, ASSERT_TRUE(WriteMicrodump(child, &context, sizeof(context), mappings, skip_dump_if_principal_mapping_not_referenced, - address_within_principal_mapping, microdump_extra_info)); + address_within_principal_mapping, sanitize_stack, + microdump_extra_info)); // Revert stderr back to the console. dup2(save_err, STDERR_FILENO); @@ -134,6 +146,27 @@ void CrashAndGetMicrodump(const MappingList& mappings, close(fds[1]); } +void ExtractMicrodumpStackContents(const string& microdump_content, + string* result) { + std::istringstream iss(microdump_content); + result->clear(); + for (string line; std::getline(iss, line);) { + if (line.find("S ") == 0) { + std::istringstream stack_data(line); + std::string key; + std::string addr; + std::string data; + stack_data >> key >> addr >> data; + EXPECT_TRUE((data.size() & 1u) == 0u); + result->reserve(result->size() + data.size() / 2); + for (size_t i = 0; i < data.size(); i += 2) { + std::string byte = data.substr(i, 2); + result->push_back(static_cast(strtoul(byte.c_str(), NULL, 16))); + } + } + } +} + void CheckMicrodumpContents(const string& microdump_content, const MicrodumpExtraInfo& expected_info) { std::istringstream iss(microdump_content); @@ -175,6 +208,13 @@ void CheckMicrodumpContents(const string& microdump_content, ASSERT_TRUE(did_find_gpu_info); } +bool MicrodumpStackContains(const string& microdump_content, + const string& expected_content) { + string result; + ExtractMicrodumpStackContents(microdump_content, &result); + return result.find(kIdentifiableString) != string::npos; +} + void CheckMicrodumpContents(const string& microdump_content, const string& expected_fingerprint, const string& expected_product_info, @@ -244,6 +284,46 @@ TEST(MicrodumpWriterTest, NoOutputIfUninteresting) { ASSERT_FALSE(ContainsMicrodump(buf)); } +// Ensure that stack content does not contain an identifiable string if the +// stack is sanitized. +TEST(MicrodumpWriterTest, StringRemovedBySanitization) { + const char kProductInfo[] = "MockProduct:42.0.2311.99"; + const char kBuildFingerprint[] = + "aosp/occam/mako:5.1.1/LMY47W/12345678:userdegbug/dev-keys"; + const char kGPUFingerprint[] = + "Qualcomm;Adreno (TM) 330;OpenGL ES 3.0 V@104.0 AU@ (GIT@Id3510ff6dc)"; + + const MicrodumpExtraInfo kMicrodumpExtraInfo( + MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint)); + + std::string buf; + MappingList no_mappings; + + CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf, false, 0u, true); + ASSERT_TRUE(ContainsMicrodump(buf)); + ASSERT_FALSE(MicrodumpStackContains(buf, kIdentifiableString)); +} + +// Ensure that stack content does contain an identifiable string if the +// stack is not sanitized. +TEST(MicrodumpWriterTest, StringPresentIfNotSanitized) { + const char kProductInfo[] = "MockProduct:42.0.2311.99"; + const char kBuildFingerprint[] = + "aosp/occam/mako:5.1.1/LMY47W/12345678:userdegbug/dev-keys"; + const char kGPUFingerprint[] = + "Qualcomm;Adreno (TM) 330;OpenGL ES 3.0 V@104.0 AU@ (GIT@Id3510ff6dc)"; + + const MicrodumpExtraInfo kMicrodumpExtraInfo( + MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint)); + + std::string buf; + MappingList no_mappings; + + CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf, false, 0u, false); + ASSERT_TRUE(ContainsMicrodump(buf)); + ASSERT_TRUE(MicrodumpStackContains(buf, kIdentifiableString)); +} + // Ensure that output occurs if the interest region is set, and // does overlap something on the stack. TEST(MicrodumpWriterTest, OutputIfInteresting) { diff --git a/src/client/linux/minidump_writer/linux_dumper.cc b/src/client/linux/minidump_writer/linux_dumper.cc index 4a15f0cc..e1e7b49c 100644 --- a/src/client/linux/minidump_writer/linux_dumper.cc +++ b/src/client/linux/minidump_writer/linux_dumper.cc @@ -84,10 +84,15 @@ inline static bool IsMappedFileOpenUnsafe( namespace google_breakpad { -#if defined(__CHROMEOS__) - namespace { +bool MappingContainsAddress(const MappingInfo& mapping, uintptr_t address) { + return mapping.system_mapping_info.start_addr <= address && + address < mapping.system_mapping_info.end_addr; +} + +#if defined(__CHROMEOS__) + // Recover memory mappings before writing dump on ChromeOS // // On Linux, breakpad relies on /proc/[pid]/maps to associate symbols from @@ -248,9 +253,10 @@ void CrOSPostProcessMappings(wasteful_vector& mappings) { mappings.resize(f); } -} // namespace #endif // __CHROMEOS__ +} // namespace + // All interesting auvx entry types are below AT_SYSINFO_EHDR #define AT_MAX AT_SYSINFO_EHDR @@ -705,6 +711,99 @@ bool LinuxDumper::GetStackInfo(const void** stack, size_t* stack_len, return true; } +void LinuxDumper::SanitizeStackCopy(uint8_t* stack_copy, size_t stack_len, + uintptr_t stack_pointer, + uintptr_t sp_offset) { + // We optimize the search for containing mappings in three ways: + // 1) We expect that pointers into the stack mapping will be common, so + // we cache that address range. + // 2) The last referenced mapping is a reasonable predictor for the next + // referenced mapping, so we test that first. + // 3) We precompute a bitfield based upon bits 32:32-n of the start and + // stop addresses, and use that to short circuit any values that can + // not be pointers. (n=11) + const uintptr_t defaced = +#if defined(__LP64__) + 0x0defaced0defaced; +#else + 0x0defaced; +#endif + // the bitfield length is 2^test_bits long. + const unsigned int test_bits = 11; + // byte length of the corresponding array. + const unsigned int array_size = 1 << (test_bits - 3); + const unsigned int array_mask = array_size - 1; + // The amount to right shift pointers by. This captures the top bits + // on 32 bit architectures. On 64 bit architectures this would be + // uninformative so we take the same range of bits. + const unsigned int shift = 32 - 11; + const MappingInfo* last_hit_mapping = nullptr; + const MappingInfo* hit_mapping = nullptr; + const MappingInfo* stack_mapping = FindMappingNoBias(stack_pointer); + // The magnitude below which integers are considered to be to be + // 'small', and not constitute a PII risk. These are included to + // avoid eliding useful register values. + const ssize_t small_int_magnitude = 4096; + + char could_hit_mapping[array_size]; + my_memset(could_hit_mapping, 0, array_size); + + // Initialize the bitfield such that if the (pointer >> shift)'th + // bit, modulo the bitfield size, is not set then there does not + // exist a mapping in mappings_ that would contain that pointer. + for (size_t i = 0; i < mappings_.size(); ++i) { + // For each mapping, work out the (unmodulo'ed) range of bits to + // set. + uintptr_t start = mappings_[i]->start_addr; + uintptr_t end = start + mappings_[i]->size; + start >>= shift; + end >>= shift; + for (size_t bit = start; bit <= end; ++bit) { + // Set each bit in the range, applying the modulus. + could_hit_mapping[(bit >> 3) & array_mask] |= 1 << (bit & 7); + } + } + + // Zero memory that is below the current stack pointer. + const uintptr_t offset = + (sp_offset + sizeof(uintptr_t) - 1) & ~(sizeof(uintptr_t) - 1); + if (offset) { + my_memset(stack_copy, 0, offset); + } + + // Apply sanitization to each complete pointer-aligned word in the + // stack. + uint8_t* sp; + for (sp = stack_copy + offset; + sp <= stack_copy + stack_len - sizeof(uintptr_t); + sp += sizeof(uintptr_t)) { + uintptr_t addr; + my_memcpy(&addr, sp, sizeof(uintptr_t)); + if (static_cast(addr) <= small_int_magnitude && + static_cast(addr) >= -small_int_magnitude) { + continue; + } + if (stack_mapping && MappingContainsAddress(*stack_mapping, addr)) { + continue; + } + if (last_hit_mapping && MappingContainsAddress(*last_hit_mapping, addr)) { + continue; + } + uintptr_t test = addr >> shift; + if (could_hit_mapping[(test >> 3) & array_mask] & (1 << (test & 7)) && + (hit_mapping = FindMappingNoBias(addr)) != nullptr) { + last_hit_mapping = hit_mapping; + continue; + } + my_memcpy(sp, &defaced, sizeof(uintptr_t)); + } + // Zero any partial word at the top of the stack, if alignment is + // such that that is required. + if (sp < stack_copy + stack_len) { + my_memset(sp, 0, stack_copy + stack_len - sp); + } +} + bool LinuxDumper::StackHasPointerToMapping(const uint8_t* stack_copy, size_t stack_len, uintptr_t sp_offset, diff --git a/src/client/linux/minidump_writer/linux_dumper.h b/src/client/linux/minidump_writer/linux_dumper.h index 0e20209b..23c78e08 100644 --- a/src/client/linux/minidump_writer/linux_dumper.h +++ b/src/client/linux/minidump_writer/linux_dumper.h @@ -116,6 +116,19 @@ class LinuxDumper { // stack_top: the current top of the stack bool GetStackInfo(const void** stack, size_t* stack_len, uintptr_t stack_top); + // Sanitize a copy of the stack by overwriting words that are not + // pointers with a sentinel (0x0defaced). + // stack_copy: a copy of the stack to sanitize. |stack_copy| might + // not be word aligned, but it represents word aligned + // data copied from another location. + // stack_len: the length of the allocation pointed to by |stack_copy|. + // stack_pointer: the address of the stack pointer (used to locate + // the stack mapping, as an optimization). + // sp_offset: the offset relative to stack_copy that reflects the + // current value of the stack pointer. + void SanitizeStackCopy(uint8_t* stack_copy, size_t stack_len, + uintptr_t stack_pointer, uintptr_t sp_offset); + // Test whether |stack_copy| contains a pointer-aligned word that // could be an address within a given mapping. // stack_copy: a copy of the stack to check. |stack_copy| might diff --git a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc index be533e15..ae30e606 100644 --- a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc +++ b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc @@ -66,6 +66,62 @@ using namespace google_breakpad; namespace { +pid_t SetupChildProcess(int number_of_threads) { + char kNumberOfThreadsArgument[2]; + sprintf(kNumberOfThreadsArgument, "%d", number_of_threads); + + int fds[2]; + EXPECT_NE(-1, pipe(fds)); + + pid_t child_pid = fork(); + if (child_pid == 0) { + // In child process. + close(fds[0]); + + string helper_path(GetHelperBinary()); + if (helper_path.empty()) { + ADD_FAILURE() << "Couldn't find helper binary"; + return -1; + } + + // 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(), + "linux_dumper_unittest_helper", + pipe_fd_string, + kNumberOfThreadsArgument, + NULL); + // Kill if we get here. + printf("Errno from exec: %d", errno); + ADD_FAILURE() << "Exec of " << helper_path << " failed: " << strerror(errno); + return -1; + } + close(fds[1]); + + // Wait for all child threads to indicate that they have started + for (int threads = 0; threads < number_of_threads; 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)); + EXPECT_EQ(1, r); + EXPECT_TRUE(pfd.revents & POLLIN); + uint8_t junk; + EXPECT_EQ(read(fds[0], &junk, sizeof(junk)), + static_cast(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); + return child_pid; +} + typedef wasteful_vector id_vector; typedef testing::Test LinuxPtraceDumperTest; @@ -370,58 +426,9 @@ TEST_F(LinuxPtraceDumperChildTest, FileIDsMatch) { TEST(LinuxPtraceDumperTest, VerifyStackReadWithMultipleThreads) { static const int kNumberOfThreadsInHelperProgram = 5; - char kNumberOfThreadsArgument[2]; - sprintf(kNumberOfThreadsArgument, "%d", kNumberOfThreadsInHelperProgram); - int fds[2]; - ASSERT_NE(-1, pipe(fds)); - - pid_t child_pid = fork(); - if (child_pid == 0) { - // In child process. - close(fds[0]); - - string helper_path(GetHelperBinary()); - if (helper_path.empty()) { - FAIL() << "Couldn't find helper binary"; - exit(1); - } - - // 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(), - "linux_dumper_unittest_helper", - pipe_fd_string, - kNumberOfThreadsArgument, - NULL); - // Kill if we get here. - printf("Errno from exec: %d", errno); - FAIL() << "Exec of " << helper_path << " failed: " << strerror(errno); - exit(0); - } - 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)), - static_cast(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); + pid_t child_pid = SetupChildProcess(kNumberOfThreadsInHelperProgram); + ASSERT_NE(child_pid, -1); // Children are ready now. LinuxPtraceDumper dumper(child_pid); @@ -468,3 +475,75 @@ TEST(LinuxPtraceDumperTest, VerifyStackReadWithMultipleThreads) { ASSERT_TRUE(WIFSIGNALED(status)); ASSERT_EQ(SIGKILL, WTERMSIG(status)); } + +TEST_F(LinuxPtraceDumperTest, SanitizeStackCopy) { + static const int kNumberOfThreadsInHelperProgram = 1; + + pid_t child_pid = SetupChildProcess(kNumberOfThreadsInHelperProgram); + ASSERT_NE(child_pid, -1); + + LinuxPtraceDumper dumper(child_pid); + ASSERT_TRUE(dumper.Init()); + EXPECT_TRUE(dumper.ThreadsSuspend()); + + ThreadInfo thread_info; + EXPECT_TRUE(dumper.GetThreadInfoByIndex(0, &thread_info)); + + const void* stack; + size_t stack_len; + EXPECT_TRUE(dumper.GetStackInfo(&stack, &stack_len, thread_info.stack_pointer)); + + uint8_t* stack_copy = new uint8_t[stack_len]; + dumper.CopyFromProcess(stack_copy, child_pid, stack, stack_len); + + size_t stack_offset = + thread_info.stack_pointer - reinterpret_cast(stack); + + const size_t word_count = (stack_len - stack_offset) / sizeof(uintptr_t); + uintptr_t* stack_words = new uintptr_t[word_count]; + + memcpy(stack_words, stack_copy + stack_offset, word_count * sizeof(uintptr_t)); + std::map pre_sanitization_words; + for (size_t i = 0; i < word_count; ++i) + ++pre_sanitization_words[stack_words[i]]; + + fprintf(stderr, "stack_offset=%lu stack_len=%lu stack=%p\n", stack_offset, stack_len, stack); + dumper.SanitizeStackCopy(stack_copy, stack_len, thread_info.stack_pointer, + stack_offset); + + // Memory below the stack pointer should be zeroed. + for (size_t i = 0; i < stack_offset; ++i) { + ASSERT_EQ(0, stack_copy[i]); + } + + memcpy(stack_words, stack_copy + stack_offset, word_count * sizeof(uintptr_t)); + std::map post_sanitization_words; + for (size_t i = 0; i < word_count; ++i) + ++post_sanitization_words[stack_words[i]]; + + std::set words; + for (auto &word : pre_sanitization_words) words.insert(word.first); + for (auto &word : post_sanitization_words) words.insert(word.first); + + for (auto word : words) { + if (word == static_cast(0X0DEFACED0DEFACEDull)) { + continue; + } + + bool should_be_sanitized = true; + if (static_cast(word) <= 4096 && + static_cast(word) >= -4096) should_be_sanitized = false; + if (dumper.FindMappingNoBias(word)) should_be_sanitized = false; + + ASSERT_EQ(should_be_sanitized, post_sanitization_words[word] == 0); + } + + EXPECT_TRUE(dumper.ThreadsResume()); + kill(child_pid, SIGKILL); + + // Reap child + int status; + ASSERT_NE(-1, HANDLE_EINTR(waitpid(child_pid, &status, 0))); + ASSERT_TRUE(WIFSIGNALED(status)); + ASSERT_EQ(SIGKILL, WTERMSIG(status)); +}