Server-side workaround to handle overlapping modules.

This change is resolving an issue that was caused by the combination of:
 - Android system libraries being relro packed in N+.
 - Breakpad dealing with relro packed libraries in a hack way.

This is a fix for http://crbug/611824.

I also found an use-after-free issue (bug in Minidump::SeekToStreamType).  I disallowed the MinidumpStreamInfo copy and assign constructors and the compiler detected another similar issue in Minidump::Print.  Then I disabled the copy and assign constructors for most classes in minidump.h (just in case).  There are a couple of classes where I couldn't disallow them (since assign is used).  This will require a small refactor so I left it out of this CL.

R=mark@chromium.org

Review URL: https://codereview.chromium.org/2060663002 .
This commit is contained in:
Ivan Penkov 2016-06-20 11:14:47 -07:00
parent 67f738b7ad
commit 24f5931c5e
18 changed files with 239 additions and 39 deletions

View file

@ -169,6 +169,7 @@ TEST(MinidumpWriterTest, MappingInfo) {
info.start_addr = kMemoryAddress;
info.size = memory_size;
info.offset = 0;
info.exec = false;
strcpy(info.name, kMemoryName);
MappingList mappings;
@ -323,6 +324,7 @@ TEST(MinidumpWriterTest, MappingInfoContained) {
info.start_addr = kMemoryAddress - memory_size;
info.size = memory_size * 3;
info.offset = 0;
info.exec = false;
strcpy(info.name, kMemoryName);
MappingList mappings;

View file

@ -86,7 +86,14 @@ class CodeModule {
// ownership of. The new CodeModule may be of a different concrete class
// than the CodeModule being copied, but will behave identically to the
// copied CodeModule as far as the CodeModule interface is concerned.
virtual const CodeModule* Copy() const = 0;
virtual CodeModule* Copy() const = 0;
// Getter and setter for shrink_down_delta. This is used when the address
// range for a module is shrunk down due to address range conflicts with
// other modules. The base_address and size fields are not updated and they
// should always reflect the original values (reported in the minidump).
virtual uint64_t shrink_down_delta() const = 0;
virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) = 0;
};
} // namespace google_breakpad

View file

@ -35,7 +35,12 @@
#ifndef GOOGLE_BREAKPAD_PROCESSOR_CODE_MODULES_H__
#define GOOGLE_BREAKPAD_PROCESSOR_CODE_MODULES_H__
#include <stddef.h>
#include <vector>
#include "google_breakpad/common/breakpad_types.h"
#include "processor/linked_ptr.h"
namespace google_breakpad {
@ -91,6 +96,14 @@ class CodeModules {
// returns objects in may differ between a copy and the original CodeModules
// object.
virtual const CodeModules* Copy() const = 0;
// Returns a vector of all modules which address ranges needed to be shrunk
// down due to address range conflicts with other modules.
virtual std::vector<linked_ptr<const CodeModule> >
GetShrunkRangeModules() const = 0;
// Returns true, if module address range shrink is enabled.
virtual bool IsModuleShrinkEnabled() const = 0;
};
} // namespace google_breakpad

View file

@ -58,6 +58,9 @@ class MicrodumpModules : public BasicCodeModules {
public:
// Takes over ownership of |module|.
void Add(const CodeModule* module);
// Enables/disables module address range shrink.
void SetEnableModuleShrink(bool is_enabled);
};
// MicrodumpContext carries a CPU-specific context.

View file

@ -151,6 +151,8 @@ class MinidumpStream : public MinidumpObject {
// that implements MinidumpStream can compare expected_size to a
// known size as an integrity check.
virtual bool Read(uint32_t expected_size) = 0;
DISALLOW_COPY_AND_ASSIGN(MinidumpStream);
};
@ -191,6 +193,8 @@ class MinidumpContext : public DumpContext {
// for access to data about the minidump file itself, such as whether
// it should be byte-swapped.
Minidump* minidump_;
DISALLOW_COPY_AND_ASSIGN(MinidumpContext);
};
@ -358,6 +362,8 @@ class MinidumpThreadList : public MinidumpStream {
// The list of threads.
MinidumpThreads* threads_;
uint32_t thread_count_;
DISALLOW_COPY_AND_ASSIGN(MinidumpThreadList);
};
@ -392,7 +398,14 @@ class MinidumpModule : public MinidumpObject,
virtual string debug_file() const;
virtual string debug_identifier() const;
virtual string version() const;
virtual const CodeModule* Copy() const;
virtual CodeModule* Copy() const;
// Getter and setter for shrink_down_delta. This is used when the address
// range for a module is shrunk down due to address range conflicts with
// other modules. The base_address and size fields are not updated and they
// should always reflect the original values (reported in the minidump).
virtual uint64_t shrink_down_delta() const;
virtual void SetShrinkDownDelta(uint64_t shrink_down_delta);
// The CodeView record, which contains information to locate the module's
// debugging information (pdb). This is returned as uint8_t* because
@ -501,6 +514,13 @@ class MinidumpModuleList : public MinidumpStream,
virtual const MinidumpModule* GetModuleAtIndex(unsigned int index) const;
virtual const CodeModules* Copy() const;
// Returns a vector of all modules which address ranges needed to be shrunk
// down due to address range conflicts with other modules.
virtual vector<linked_ptr<const CodeModule> > GetShrunkRangeModules() const;
// Returns true, if module address range shrink is enabled.
virtual bool IsModuleShrinkEnabled() const;
// Print a human-readable representation of the object to stdout.
void Print();
@ -525,6 +545,8 @@ class MinidumpModuleList : public MinidumpStream,
MinidumpModules *modules_;
uint32_t module_count_;
DISALLOW_COPY_AND_ASSIGN(MinidumpModuleList);
};
@ -587,6 +609,8 @@ class MinidumpMemoryList : public MinidumpStream {
// The list of regions.
MemoryRegions *regions_;
uint32_t region_count_;
DISALLOW_COPY_AND_ASSIGN(MinidumpMemoryList);
};
@ -626,6 +650,8 @@ class MinidumpException : public MinidumpStream {
MDRawExceptionStream exception_;
MinidumpContext* context_;
DISALLOW_COPY_AND_ASSIGN(MinidumpException);
};
// MinidumpAssertion wraps MDRawAssertionInfo, which contains information
@ -666,6 +692,8 @@ class MinidumpAssertion : public MinidumpStream {
string expression_;
string function_;
string file_;
DISALLOW_COPY_AND_ASSIGN(MinidumpAssertion);
};
@ -719,6 +747,8 @@ class MinidumpSystemInfo : public MinidumpStream {
// A string identifying the CPU vendor, if known.
const string* cpu_vendor_;
DISALLOW_COPY_AND_ASSIGN(MinidumpSystemInfo);
};
@ -752,6 +782,8 @@ class MinidumpMiscInfo : public MinidumpStream {
string daylight_name_;
string build_string_;
string dbg_bld_str_;
DISALLOW_COPY_AND_ASSIGN(MinidumpMiscInfo);
};
@ -784,6 +816,8 @@ class MinidumpBreakpadInfo : public MinidumpStream {
bool Read(uint32_t expected_size_);
MDRawBreakpadInfo breakpad_info_;
DISALLOW_COPY_AND_ASSIGN(MinidumpBreakpadInfo);
};
// MinidumpMemoryInfo wraps MDRawMemoryInfo, which provides information
@ -854,6 +888,8 @@ class MinidumpMemoryInfoList : public MinidumpStream {
MinidumpMemoryInfos* infos_;
uint32_t info_count_;
DISALLOW_COPY_AND_ASSIGN(MinidumpMemoryInfoList);
};
// MinidumpLinuxMaps wraps information about a single mapped memory region
@ -1061,6 +1097,9 @@ class Minidump {
// Print a human-readable representation of the object to stdout.
void Print();
// Is the OS Android.
bool IsAndroid();
private:
// MinidumpStreamInfo is used in the MinidumpStreamMap. It lets
// the Minidump object locate interesting streams quickly, and
@ -1074,6 +1113,9 @@ class Minidump {
// Pointer to the stream if cached, or NULL if not yet populated
MinidumpStream* stream;
private:
DISALLOW_COPY_AND_ASSIGN(MinidumpStreamInfo);
};
typedef vector<MDRawDirectory> MinidumpDirectoryEntries;
@ -1121,6 +1163,8 @@ class Minidump {
// construction or after a failed Read(); true following a successful
// Read().
bool valid_;
DISALLOW_COPY_AND_ASSIGN(Minidump);
};

View file

@ -39,8 +39,10 @@
#include "common/using_std_string.h"
#include "google_breakpad/common/breakpad_types.h"
#include "google_breakpad/processor/system_info.h"
#include "google_breakpad/processor/code_modules.h"
#include "google_breakpad/processor/minidump.h"
#include "google_breakpad/processor/system_info.h"
#include "processor/linked_ptr.h"
namespace google_breakpad {
@ -109,6 +111,9 @@ class ProcessState {
}
const SystemInfo* system_info() const { return &system_info_; }
const CodeModules* modules() const { return modules_; }
const vector<linked_ptr<const CodeModule> >* shrunk_range_modules() const {
return &shrunk_range_modules_;
}
const vector<const CodeModule*>* modules_without_symbols() const {
return &modules_without_symbols_;
}
@ -172,6 +177,10 @@ class ProcessState {
// ProcessState.
const CodeModules *modules_;
// The modules which virtual address ranges were shrunk down due to
// virtual address conflicts.
vector<linked_ptr<const CodeModule> > shrunk_range_modules_;
// The modules that didn't have symbols when the report was processed.
vector<const CodeModule*> modules_without_symbols_;

View file

@ -57,6 +57,7 @@ class BasicCodeModule : public CodeModule {
explicit BasicCodeModule(const CodeModule *that)
: base_address_(that->base_address()),
size_(that->size()),
shrink_down_delta_(that->shrink_down_delta()),
code_file_(that->code_file()),
code_identifier_(that->code_identifier()),
debug_file_(that->debug_file()),
@ -71,6 +72,7 @@ class BasicCodeModule : public CodeModule {
const string &version)
: base_address_(base_address),
size_(size),
shrink_down_delta_(0),
code_file_(code_file),
code_identifier_(code_identifier),
debug_file_(debug_file),
@ -83,16 +85,21 @@ class BasicCodeModule : public CodeModule {
// members.
virtual uint64_t base_address() const { return base_address_; }
virtual uint64_t size() const { return size_; }
virtual uint64_t shrink_down_delta() const { return shrink_down_delta_; }
virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {
shrink_down_delta_ = shrink_down_delta;
}
virtual string code_file() const { return code_file_; }
virtual string code_identifier() const { return code_identifier_; }
virtual string debug_file() const { return debug_file_; }
virtual string debug_identifier() const { return debug_identifier_; }
virtual string version() const { return version_; }
virtual const CodeModule* Copy() const { return new BasicCodeModule(this); }
virtual CodeModule* Copy() const { return new BasicCodeModule(this); }
private:
uint64_t base_address_;
uint64_t size_;
uint64_t shrink_down_delta_;
string code_file_;
string code_identifier_;
string debug_file_;

View file

@ -38,6 +38,8 @@
#include <assert.h>
#include <vector>
#include "google_breakpad/processor/code_module.h"
#include "processor/linked_ptr.h"
#include "processor/logging.h"
@ -45,31 +47,50 @@
namespace google_breakpad {
using std::vector;
BasicCodeModules::BasicCodeModules(const CodeModules *that)
: main_address_(0), map_() {
BPLOG_IF(ERROR, !that) << "BasicCodeModules::BasicCodeModules requires "
"|that|";
assert(that);
map_.SetEnableShrinkDown(that->IsModuleShrinkEnabled());
const CodeModule *main_module = that->GetMainModule();
if (main_module)
main_address_ = main_module->base_address();
unsigned int count = that->module_count();
for (unsigned int module_sequence = 0;
module_sequence < count;
++module_sequence) {
for (unsigned int i = 0; i < count; ++i) {
// Make a copy of the module and insert it into the map. Use
// GetModuleAtIndex because ordering is unimportant when slurping the
// entire list, and GetModuleAtIndex may be faster than
// GetModuleAtSequence.
linked_ptr<const CodeModule> module(
that->GetModuleAtIndex(module_sequence)->Copy());
linked_ptr<const CodeModule> module(that->GetModuleAtIndex(i)->Copy());
if (!map_.StoreRange(module->base_address(), module->size(), module)) {
BPLOG(ERROR) << "Module " << module->code_file() <<
" could not be stored";
BPLOG(ERROR) << "Module " << module->code_file()
<< " could not be stored";
}
}
// Report modules with shrunk ranges.
for (unsigned int i = 0; i < count; ++i) {
linked_ptr<const CodeModule> module(that->GetModuleAtIndex(i)->Copy());
uint64_t delta = 0;
if (map_.RetrieveRange(module->base_address() + module->size() - 1,
&module, NULL /* base */, &delta, NULL /* size */) &&
delta > 0) {
BPLOG(INFO) << "The range for module " << module->code_file()
<< " was shrunk down by " << HexString(delta) << " bytes.";
linked_ptr<CodeModule> shrunk_range_module(module->Copy());
shrunk_range_module->SetShrinkDownDelta(delta);
shrunk_range_modules_.push_back(shrunk_range_module);
}
}
// TODO(ivanpe): Report modules with conflicting ranges. The list of such
// modules should be copied from |that|.
}
BasicCodeModules::BasicCodeModules() : main_address_(0), map_() { }
@ -122,4 +143,13 @@ const CodeModules* BasicCodeModules::Copy() const {
return new BasicCodeModules(this);
}
vector<linked_ptr<const CodeModule> >
BasicCodeModules::GetShrunkRangeModules() const {
return shrunk_range_modules_;
}
bool BasicCodeModules::IsModuleShrinkEnabled() const {
return map_.IsShrinkDownEnabled();
}
} // namespace google_breakpad

View file

@ -43,6 +43,8 @@
#include <stddef.h>
#include <vector>
#include "google_breakpad/processor/code_modules.h"
#include "processor/linked_ptr.h"
#include "processor/range_map.h"
@ -67,6 +69,9 @@ class BasicCodeModules : public CodeModules {
virtual const CodeModule* GetModuleAtSequence(unsigned int sequence) const;
virtual const CodeModule* GetModuleAtIndex(unsigned int index) const;
virtual const CodeModules* Copy() const;
virtual std::vector<linked_ptr<const CodeModule> >
GetShrunkRangeModules() const;
virtual bool IsModuleShrinkEnabled() const;
protected:
BasicCodeModules();
@ -78,6 +83,10 @@ class BasicCodeModules : public CodeModules {
// address range.
RangeMap<uint64_t, linked_ptr<const CodeModule> > map_;
// A vector of all CodeModules that were shrunk downs due to
// address range conflicts.
std::vector<linked_ptr<const CodeModule> > shrunk_range_modules_;
private:
// Disallow copy constructor and assignment operator.
BasicCodeModules(const BasicCodeModules &that);

View file

@ -68,9 +68,11 @@ class TestCodeModule : public CodeModule {
virtual string debug_file() const { return ""; }
virtual string debug_identifier() const { return ""; }
virtual string version() const { return ""; }
virtual const CodeModule* Copy() const {
virtual CodeModule* Copy() const {
return new TestCodeModule(code_file_);
}
virtual uint64_t shrink_down_delta() const { return 0; }
virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {}
private:
string code_file_;

View file

@ -79,9 +79,11 @@ class TestCodeModule : public CodeModule {
virtual string debug_file() const { return ""; }
virtual string debug_identifier() const { return ""; }
virtual string version() const { return ""; }
virtual const CodeModule* Copy() const {
virtual CodeModule* Copy() const {
return new TestCodeModule(code_file_);
}
virtual uint64_t shrink_down_delta() const { return 0; }
virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {}
private:
string code_file_;

View file

@ -110,6 +110,9 @@ void MicrodumpModules::Add(const CodeModule* module) {
}
}
void MicrodumpModules::SetEnableModuleShrink(bool is_enabled) {
map_.SetEnableShrinkDown(is_enabled);
}
//
// MicrodumpContext
@ -262,6 +265,7 @@ Microdump::Microdump(const string& contents)
} else if (os_id == "A") {
system_info_->os = "Android";
system_info_->os_short = "android";
modules_->SetEnableModuleShrink(true);
}
// OS line also contains release and version for future use.

View file

@ -201,7 +201,7 @@ TEST_F(MicrodumpProcessorTest, TestProcessX86) {
AnalyzeDump("microdump-x86.dmp", false /* omit_symbols */,
4 /* expected_cpu_count */, &state);
ASSERT_EQ(105U, state.modules()->module_count());
ASSERT_EQ(124U, state.modules()->module_count());
ASSERT_EQ("x86", state.system_info()->cpu);
ASSERT_EQ("asus/WW_Z00A/Z00A:5.0/LRX21V/2.19.40.22_20150627_5104_user:user/"
"release-keys", state.system_info()->os_version);
@ -216,11 +216,11 @@ TEST_F(MicrodumpProcessorTest, TestProcessMultiple) {
ProcessState state;
AnalyzeDump("microdump-multiple.dmp", false /* omit_symbols */,
6 /* expected_cpu_count */, &state);
ASSERT_EQ(133U, state.modules()->module_count());
ASSERT_EQ(156U, state.modules()->module_count());
ASSERT_EQ("arm", state.system_info()->cpu);
ASSERT_EQ("lge/p1_tmo_us/p1:6.0/MRA58K/1603210524c8d:user/release-keys",
state.system_info()->os_version);
ASSERT_EQ(2U, state.threads()->at(0)->frames()->size());
ASSERT_EQ(5U, state.threads()->at(0)->frames()->size());
}
TEST_F(MicrodumpProcessorTest, TestProcessMips) {
@ -249,7 +249,7 @@ TEST_F(MicrodumpProcessorTest, TestProcessMips64) {
AnalyzeDump("microdump-mips64.dmp", false /* omit_symbols */,
1 /* expected_cpu_count */, &state);
ASSERT_EQ(7U, state.modules()->module_count());
ASSERT_EQ(8U, state.modules()->module_count());
ASSERT_EQ("mips64", state.system_info()->cpu);
ASSERT_EQ("3.10.0-gf185e20 #112 PREEMPT Mon Oct 5 11:12:49 PDT 2015",
state.system_info()->os_version);

View file

@ -2109,11 +2109,21 @@ string MinidumpModule::version() const {
}
const CodeModule* MinidumpModule::Copy() const {
CodeModule* MinidumpModule::Copy() const {
return new BasicCodeModule(this);
}
uint64_t MinidumpModule::shrink_down_delta() const {
return 0;
}
void MinidumpModule::SetShrinkDownDelta(uint64_t shrink_down_delta) {
// Not implemented
assert(false);
}
const uint8_t* MinidumpModule::GetCVRecord(uint32_t* size) {
if (!module_valid_) {
BPLOG(ERROR) << "Invalid MinidumpModule for GetCVRecord";
@ -2497,6 +2507,7 @@ MinidumpModuleList::MinidumpModuleList(Minidump* minidump)
range_map_(new RangeMap<uint64_t, unsigned int>()),
modules_(NULL),
module_count_(0) {
range_map_->SetEnableShrinkDown(minidump_->IsAndroid());
}
@ -2741,6 +2752,14 @@ const CodeModules* MinidumpModuleList::Copy() const {
return new BasicCodeModules(this);
}
vector<linked_ptr<const CodeModule> >
MinidumpModuleList::GetShrunkRangeModules() const {
return vector<linked_ptr<const CodeModule> >();
}
bool MinidumpModuleList::IsModuleShrinkEnabled() const {
return range_map_->IsShrinkDownEnabled();
}
void MinidumpModuleList::Print() {
if (!valid_) {
@ -4532,6 +4551,24 @@ MinidumpLinuxMapsList *Minidump::GetLinuxMapsList() {
return GetStream(&linux_maps_list);
}
bool Minidump::IsAndroid() {
// Save the current stream position
off_t saved_position = Tell();
if (saved_position == -1) {
return false;
}
const MDRawSystemInfo* system_info =
GetSystemInfo() ? GetSystemInfo()->system_info() : NULL;
// Restore position and return
if (!SeekSet(saved_position)) {
BPLOG(ERROR) << "Couldn't seek back to saved position";
return false;
}
return system_info && system_info->platform_id == MD_OS_ANDROID;
}
static const char* get_stream_name(uint32_t stream_type) {
switch (stream_type) {
case MD_UNUSED_STREAM:
@ -4645,7 +4682,7 @@ void Minidump::Print() {
iterator != stream_map_->end();
++iterator) {
uint32_t stream_type = iterator->first;
MinidumpStreamInfo info = iterator->second;
const MinidumpStreamInfo& info = iterator->second;
printf(" stream type 0x%x (%s) at index %d\n", stream_type,
get_stream_name(stream_type),
info.stream_index);
@ -4802,7 +4839,7 @@ bool Minidump::SeekToStreamType(uint32_t stream_type,
return false;
}
MinidumpStreamInfo info = iterator->second;
const MinidumpStreamInfo& info = iterator->second;
if (info.stream_index >= header_.stream_count) {
BPLOG(ERROR) << "SeekToStreamType: type " << stream_type <<
" out of range: " <<

View file

@ -126,8 +126,20 @@ ProcessResult MinidumpProcessor::Process(
// Put a copy of the module list into ProcessState object. This is not
// necessarily a MinidumpModuleList, but it adheres to the CodeModules
// interface, which is all that ProcessState needs to expose.
if (module_list)
if (module_list) {
process_state->modules_ = module_list->Copy();
process_state->shrunk_range_modules_ =
process_state->modules_->GetShrunkRangeModules();
for (unsigned int i = 0;
i < process_state->shrunk_range_modules_.size();
i++) {
linked_ptr<const CodeModule> module =
process_state->shrunk_range_modules_[i];
BPLOG(INFO) << "The range for module " << module->code_file()
<< " was shrunk down by " << HexString(
module->shrink_down_delta()) << " bytes. ";
}
}
MinidumpMemoryList *memory_list = dump->GetMemoryList();
if (memory_list) {

View file

@ -52,6 +52,11 @@ void RangeMap<AddressType, EntryType>::SetEnableShrinkDown(
enable_shrink_down_ = enable_shrink_down;
}
template<typename AddressType, typename EntryType>
bool RangeMap<AddressType, EntryType>::IsShrinkDownEnabled() const {
return enable_shrink_down_;
}
template<typename AddressType, typename EntryType>
bool RangeMap<AddressType, EntryType>::StoreRange(const AddressType &base,
const AddressType &size,

View file

@ -60,6 +60,7 @@ class RangeMap {
// will be shrunk down by moving its start position to a higher address so
// that it does not overlap anymore.
void SetEnableShrinkDown(bool enable_shrink_down);
bool IsShrinkDownEnabled() const;
// Inserts a range into the map. Returns false for a parameter error,
// or if the location of the range would conflict with a range already

View file

@ -48,6 +48,7 @@
#include "google_breakpad/processor/memory_region.h"
#include "google_breakpad/processor/symbol_supplier.h"
#include "google_breakpad/processor/system_info.h"
#include "processor/linked_ptr.h"
class MockMemoryRegion: public google_breakpad::MemoryRegion {
public:
@ -114,9 +115,11 @@ class MockCodeModule: public google_breakpad::CodeModule {
string debug_file() const { return code_file_; }
string debug_identifier() const { return code_file_; }
string version() const { return version_; }
const google_breakpad::CodeModule *Copy() const {
google_breakpad::CodeModule *Copy() const {
abort(); // Tests won't use this.
}
virtual uint64_t shrink_down_delta() const { return 0; }
virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {}
private:
uint64_t base_address_;
@ -157,7 +160,17 @@ class MockCodeModules: public google_breakpad::CodeModules {
return modules_.at(index);
}
const CodeModules *Copy() const { abort(); } // Tests won't use this.
CodeModules *Copy() const { abort(); } // Tests won't use this
virtual std::vector<google_breakpad::linked_ptr<const CodeModule> >
GetShrunkRangeModules() const {
return std::vector<google_breakpad::linked_ptr<const CodeModule> >();
}
// Returns true, if module address range shrink is enabled.
bool IsModuleShrinkEnabled() const {
return false;
}
private:
typedef std::vector<const MockCodeModule *> ModuleVector;