Fix potential null pointer dereference.

If a MinidumpLinuxMapsList was created and destroyed without its Read method,
the program would have a segmentation fault because the destructor did not
check for a null maps_ field. Additional changes include additional
supplementary null checks, a potential memory leak fix, and some comment
removal.

Review URL: https://codereview.chromium.org/1271543002

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1478 4c0a9323-5329-0410-9bdc-e9ce6186880e
This commit is contained in:
Liu.andrew.x@gmail.com 2015-07-31 15:26:39 +00:00
parent 4634d88f2e
commit 0dbae0cf3f
2 changed files with 15 additions and 12 deletions

View File

@ -901,10 +901,6 @@ class MinidumpLinuxMaps : public MinidumpObject {
// This caller owns the pointer. // This caller owns the pointer.
explicit MinidumpLinuxMaps(Minidump *minidump); explicit MinidumpLinuxMaps(Minidump *minidump);
// Read data about a single mapping from /proc/self/maps and load the data
// into this object. The input vector is in the same format as a line from
// /proc/self/maps.
// The memory region struct that this class wraps. // The memory region struct that this class wraps.
MappedMemoryRegion region_; MappedMemoryRegion region_;

View File

@ -3999,15 +3999,17 @@ MinidumpLinuxMapsList::MinidumpLinuxMapsList(Minidump *minidump)
} }
MinidumpLinuxMapsList::~MinidumpLinuxMapsList() { MinidumpLinuxMapsList::~MinidumpLinuxMapsList() {
for (unsigned int i = 0; i < maps_->size(); i++) { if (maps_) {
delete (*maps_)[i]; for (unsigned int i = 0; i < maps_->size(); i++) {
delete (*maps_)[i];
}
delete maps_;
} }
delete maps_;
} }
const MinidumpLinuxMaps *MinidumpLinuxMapsList::GetLinuxMapsForAddress( const MinidumpLinuxMaps *MinidumpLinuxMapsList::GetLinuxMapsForAddress(
uint64_t address) const { uint64_t address) const {
if (!valid_) { if (!valid_ || (maps_ == NULL)) {
BPLOG(ERROR) << "Invalid MinidumpLinuxMapsList for GetLinuxMapsForAddress"; BPLOG(ERROR) << "Invalid MinidumpLinuxMapsList for GetLinuxMapsForAddress";
return NULL; return NULL;
} }
@ -4029,13 +4031,13 @@ const MinidumpLinuxMaps *MinidumpLinuxMapsList::GetLinuxMapsForAddress(
const MinidumpLinuxMaps *MinidumpLinuxMapsList::GetLinuxMapsAtIndex( const MinidumpLinuxMaps *MinidumpLinuxMapsList::GetLinuxMapsAtIndex(
unsigned int index) const { unsigned int index) const {
if (!valid_) { if (!valid_ || (maps_ == NULL)) {
BPLOG(ERROR) << "Invalid MinidumpLinuxMapsList for GetLinuxMapsAtIndex"; BPLOG(ERROR) << "Invalid MinidumpLinuxMapsList for GetLinuxMapsAtIndex";
return NULL; return NULL;
} }
// Index out of bounds. // Index out of bounds.
if (index >= maps_count_) { if (index >= maps_count_ || (maps_ == NULL)) {
BPLOG(ERROR) << "MinidumpLinuxMapsList index of out range: " BPLOG(ERROR) << "MinidumpLinuxMapsList index of out range: "
<< index << index
<< "/" << "/"
@ -4047,7 +4049,12 @@ const MinidumpLinuxMaps *MinidumpLinuxMapsList::GetLinuxMapsAtIndex(
bool MinidumpLinuxMapsList::Read(uint32_t expected_size) { bool MinidumpLinuxMapsList::Read(uint32_t expected_size) {
// Invalidate cached data. // Invalidate cached data.
delete maps_; if (maps_) {
for (unsigned int i = 0; i < maps_->size(); i++) {
delete (*maps_)[i];
}
delete maps_;
}
maps_ = NULL; maps_ = NULL;
maps_count_ = 0; maps_count_ = 0;
@ -4100,7 +4107,7 @@ bool MinidumpLinuxMapsList::Read(uint32_t expected_size) {
} }
void MinidumpLinuxMapsList::Print() { void MinidumpLinuxMapsList::Print() {
if (!valid_) { if (!valid_ || (maps_ == NULL)) {
BPLOG(ERROR) << "MinidumpLinuxMapsList cannot print valid data"; BPLOG(ERROR) << "MinidumpLinuxMapsList cannot print valid data";
return; return;
} }