Author: lemo Date: Thu Jul 12 10:27:18 2018 New Revision: 336918 URL: http://llvm.org/viewvc/llvm-project?rev=336918&view=rev Log: Restructure the minidump loading path and add early & explicit consistency checks
Corrupted minidumps was leading to unpredictable behavior. This change adds explicit consistency checks for the minidump early on. The checks are not comprehensive but they should catch obvious structural violations: streams with type == 0 duplicate streams (same type) overlapping streams truncated minidumps Another early check is to make sure we actually support the minidump architecture instead of crashing at a random place deep inside LLDB. Differential Revision: https://reviews.llvm.org/D49202 Added: lldb/trunk/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp (with props) lldb/trunk/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp (with props) Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp lldb/trunk/unittests/Process/minidump/CMakeLists.txt lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=336918&r1=336917&r2=336918&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original) +++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Thu Jul 12 10:27:18 2018 @@ -14,10 +14,13 @@ // Other libraries and framework includes #include "lldb/Target/MemoryRegionInfo.h" +#include "lldb/Utility/LLDBAssert.h" // C includes // C++ includes +#include <algorithm> #include <map> +#include <vector> using namespace lldb_private; using namespace minidump; @@ -27,46 +30,11 @@ MinidumpParser::Create(const lldb::DataB if (data_buf_sp->GetByteSize() < sizeof(MinidumpHeader)) { return llvm::None; } - - llvm::ArrayRef<uint8_t> header_data(data_buf_sp->GetBytes(), - sizeof(MinidumpHeader)); - const MinidumpHeader *header = MinidumpHeader::Parse(header_data); - - if (header == nullptr) { - return llvm::None; - } - - lldb::offset_t directory_list_offset = header->stream_directory_rva; - // check if there is enough data for the parsing of the directory list - if ((directory_list_offset + - sizeof(MinidumpDirectory) * header->streams_count) > - data_buf_sp->GetByteSize()) { - return llvm::None; - } - - const MinidumpDirectory *directory = nullptr; - Status error; - llvm::ArrayRef<uint8_t> directory_data( - data_buf_sp->GetBytes() + directory_list_offset, - sizeof(MinidumpDirectory) * header->streams_count); - llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> directory_map; - - for (uint32_t i = 0; i < header->streams_count; ++i) { - error = consumeObject(directory_data, directory); - if (error.Fail()) { - return llvm::None; - } - directory_map[static_cast<const uint32_t>(directory->stream_type)] = - directory->location; - } - - return MinidumpParser(data_buf_sp, std::move(directory_map)); + return MinidumpParser(data_buf_sp); } -MinidumpParser::MinidumpParser( - const lldb::DataBufferSP &data_buf_sp, - llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> &&directory_map) - : m_data_sp(data_buf_sp), m_directory_map(directory_map) {} +MinidumpParser::MinidumpParser(const lldb::DataBufferSP &data_buf_sp) + : m_data_sp(data_buf_sp) {} llvm::ArrayRef<uint8_t> MinidumpParser::GetData() { return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes(), @@ -480,3 +448,109 @@ MinidumpParser::GetMemoryRegionInfo(lldb // appear truncated. return info; } + +Status MinidumpParser::Initialize() { + Status error; + + lldbassert(m_directory_map.empty()); + + llvm::ArrayRef<uint8_t> header_data(m_data_sp->GetBytes(), + sizeof(MinidumpHeader)); + const MinidumpHeader *header = MinidumpHeader::Parse(header_data); + if (header == nullptr) { + error.SetErrorString("invalid minidump: can't parse the header"); + return error; + } + + // A minidump without at least one stream is clearly ill-formed + if (header->streams_count == 0) { + error.SetErrorString("invalid minidump: no streams present"); + return error; + } + + struct FileRange { + uint32_t offset = 0; + uint32_t size = 0; + + FileRange(uint32_t offset, uint32_t size) : offset(offset), size(size) {} + uint32_t end() const { return offset + size; } + }; + + const uint32_t file_size = m_data_sp->GetByteSize(); + + // Build a global minidump file map, checking for: + // - overlapping streams/data structures + // - truncation (streams pointing past the end of file) + std::vector<FileRange> minidump_map; + + // Add the minidump header to the file map + if (sizeof(MinidumpHeader) > file_size) { + error.SetErrorString("invalid minidump: truncated header"); + return error; + } + minidump_map.emplace_back( 0, sizeof(MinidumpHeader) ); + + // Add the directory entries to the file map + FileRange directory_range(header->stream_directory_rva, + header->streams_count * + sizeof(MinidumpDirectory)); + if (directory_range.end() > file_size) { + error.SetErrorString("invalid minidump: truncated streams directory"); + return error; + } + minidump_map.push_back(directory_range); + + // Parse stream directory entries + llvm::ArrayRef<uint8_t> directory_data( + m_data_sp->GetBytes() + directory_range.offset, directory_range.size); + for (uint32_t i = 0; i < header->streams_count; ++i) { + const MinidumpDirectory *directory_entry = nullptr; + error = consumeObject(directory_data, directory_entry); + if (error.Fail()) + return error; + if (directory_entry->stream_type == 0) { + // Ignore dummy streams (technically ill-formed, but a number of + // existing minidumps seem to contain such streams) + if (directory_entry->location.data_size == 0) + continue; + error.SetErrorString("invalid minidump: bad stream type"); + return error; + } + // Update the streams map, checking for duplicate stream types + if (!m_directory_map + .insert({directory_entry->stream_type, directory_entry->location}) + .second) { + error.SetErrorString("invalid minidump: duplicate stream type"); + return error; + } + // Ignore the zero-length streams for layout checks + if (directory_entry->location.data_size != 0) { + minidump_map.emplace_back(directory_entry->location.rva, + directory_entry->location.data_size); + } + } + + // Sort the file map ranges by start offset + std::sort(minidump_map.begin(), minidump_map.end(), + [](const FileRange &a, const FileRange &b) { + return a.offset < b.offset; + }); + + // Check for overlapping streams/data structures + for (size_t i = 1; i < minidump_map.size(); ++i) { + const auto &prev_range = minidump_map[i - 1]; + if (prev_range.end() > minidump_map[i].offset) { + error.SetErrorString("invalid minidump: overlapping streams"); + return error; + } + } + + // Check for streams past the end of file + const auto &last_range = minidump_map.back(); + if (last_range.end() > file_size) { + error.SetErrorString("invalid minidump: truncated stream"); + return error; + } + + return error; +} Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h?rev=336918&r1=336917&r2=336918&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h (original) +++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h Thu Jul 12 10:27:18 2018 @@ -89,13 +89,15 @@ public: llvm::Optional<MemoryRegionInfo> GetMemoryRegionInfo(lldb::addr_t); + // Perform consistency checks and initialize internal data structures + Status Initialize(); + +private: + MinidumpParser(const lldb::DataBufferSP &data_buf_sp); + private: lldb::DataBufferSP m_data_sp; llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> m_directory_map; - - MinidumpParser( - const lldb::DataBufferSP &data_buf_sp, - llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> &&directory_map); }; } // end namespace minidump Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp?rev=336918&r1=336917&r2=336918&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp (original) +++ lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp Thu Jul 12 10:27:18 2018 @@ -33,9 +33,6 @@ const MinidumpHeader *MinidumpHeader::Pa version != MinidumpHeaderConstants::Version) return nullptr; - // TODO check for max number of streams ? - // TODO more sanity checks ? - return header; } Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp?rev=336918&r1=336917&r2=336918&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp (original) +++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp Thu Jul 12 10:27:18 2018 @@ -105,7 +105,7 @@ lldb::ProcessSP ProcessMinidump::CreateI if (!DataPtr) return nullptr; - assert(DataPtr->GetByteSize() == header_size); + lldbassert(DataPtr->GetByteSize() == header_size); // first, only try to parse the header, beacuse we need to be fast llvm::ArrayRef<uint8_t> HeaderBytes = DataPtr->GetData(); @@ -164,10 +164,29 @@ void ProcessMinidump::Terminate() { Status ProcessMinidump::DoLoadCore() { Status error; + // Minidump parser initialization & consistency checks + error = m_minidump_parser.Initialize(); + if (error.Fail()) + return error; + + // Do we support the minidump's architecture? + ArchSpec arch = GetArchitecture(); + switch (arch.GetMachine()) { + case llvm::Triple::x86: + case llvm::Triple::x86_64: + // supported + break; + + default: + error.SetErrorStringWithFormat("unsupported minidump architecture: %s", + arch.GetArchitectureName()); + return error; + } + m_thread_list = m_minidump_parser.GetThreads(); m_active_exception = m_minidump_parser.GetExceptionStream(); ReadModuleList(); - GetTarget().SetArchitecture(GetArchitecture()); + GetTarget().SetArchitecture(arch); llvm::Optional<lldb::pid_t> pid = m_minidump_parser.GetPid(); if (!pid) { Modified: lldb/trunk/unittests/Process/minidump/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/CMakeLists.txt?rev=336918&r1=336917&r2=336918&view=diff ============================================================================== --- lldb/trunk/unittests/Process/minidump/CMakeLists.txt (original) +++ lldb/trunk/unittests/Process/minidump/CMakeLists.txt Thu Jul 12 10:27:18 2018 @@ -17,6 +17,8 @@ set(test_inputs linux-x86_64.dmp linux-x86_64_not_crashed.dmp fizzbuzz_no_heap.dmp - fizzbuzz_wow64.dmp) + fizzbuzz_wow64.dmp + bad_duplicate_streams.dmp + bad_overlapping_streams.dmp) add_unittest_inputs(LLDBMinidumpTests "${test_inputs}") Added: lldb/trunk/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp?rev=336918&view=auto ============================================================================== Binary file - no diff available. Propchange: lldb/trunk/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp ------------------------------------------------------------------------------ svn:mime-type = application/octet-stream Added: lldb/trunk/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp?rev=336918&view=auto ============================================================================== Binary file - no diff available. Propchange: lldb/trunk/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp ------------------------------------------------------------------------------ svn:mime-type = application/octet-stream Modified: lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp?rev=336918&r1=336917&r2=336918&view=diff ============================================================================== --- lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp (original) +++ lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp Thu Jul 12 10:27:18 2018 @@ -38,16 +38,32 @@ using namespace minidump; class MinidumpParserTest : public testing::Test { public: - void SetUpData(const char *minidump_filename, - uint64_t load_size = UINT64_MAX) { + void SetUpData(const char *minidump_filename) { std::string filename = GetInputFilePath(minidump_filename); - auto BufferPtr = DataBufferLLVM::CreateSliceFromPath(filename, load_size, 0); + auto BufferPtr = DataBufferLLVM::CreateSliceFromPath(filename, -1, 0); + ASSERT_NE(BufferPtr, nullptr); + llvm::Optional<MinidumpParser> optional_parser = + MinidumpParser::Create(BufferPtr); + ASSERT_TRUE(optional_parser.hasValue()); + parser.reset(new MinidumpParser(optional_parser.getValue())); + ASSERT_GT(parser->GetData().size(), 0UL); + auto result = parser->Initialize(); + ASSERT_TRUE(result.Success()) << result.AsCString(); + } + + void InvalidMinidump(const char *minidump_filename, uint64_t load_size) { + std::string filename = GetInputFilePath(minidump_filename); + auto BufferPtr = + DataBufferLLVM::CreateSliceFromPath(filename, load_size, 0); + ASSERT_NE(BufferPtr, nullptr); llvm::Optional<MinidumpParser> optional_parser = MinidumpParser::Create(BufferPtr); ASSERT_TRUE(optional_parser.hasValue()); parser.reset(new MinidumpParser(optional_parser.getValue())); ASSERT_GT(parser->GetData().size(), 0UL); + auto result = parser->Initialize(); + ASSERT_TRUE(result.Fail()); } std::unique_ptr<MinidumpParser> parser; @@ -68,12 +84,15 @@ TEST_F(MinidumpParserTest, GetThreadsAnd EXPECT_EQ(1232UL, context.size()); } -TEST_F(MinidumpParserTest, GetThreadsTruncatedFile) { - SetUpData("linux-x86_64.dmp", 200); - llvm::ArrayRef<MinidumpThread> thread_list; +TEST_F(MinidumpParserTest, TruncatedMinidumps) { + InvalidMinidump("linux-x86_64.dmp", 32); + InvalidMinidump("linux-x86_64.dmp", 100); + InvalidMinidump("linux-x86_64.dmp", 20 * 1024); +} - thread_list = parser->GetThreads(); - ASSERT_EQ(0UL, thread_list.size()); +TEST_F(MinidumpParserTest, IllFormedMinidumps) { + InvalidMinidump("bad_duplicate_streams.dmp", -1); + InvalidMinidump("bad_overlapping_streams.dmp", -1); } TEST_F(MinidumpParserTest, GetArchitecture) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits