[Lldb-commits] [lldb] r336918 - Restructure the minidump loading path and add early & explicit consistency checks
Leonard Mosescu via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 12 10:27:19 PDT 2018
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) {
More information about the lldb-commits
mailing list