From 0dbae0cf3f2d3614bd7cb6ec9b6ad006e6ec1917 Mon Sep 17 00:00:00 2001
From: "Liu.andrew.x@gmail.com" <Liu.andrew.x@gmail.com>
Date: Fri, 31 Jul 2015 15:26:39 +0000
Subject: [PATCH] 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
---
 src/google_breakpad/processor/minidump.h |  4 ----
 src/processor/minidump.cc                | 23 +++++++++++++++--------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/google_breakpad/processor/minidump.h b/src/google_breakpad/processor/minidump.h
index 9c8448a3..c6273cff 100644
--- a/src/google_breakpad/processor/minidump.h
+++ b/src/google_breakpad/processor/minidump.h
@@ -901,10 +901,6 @@ class MinidumpLinuxMaps : public MinidumpObject {
   // This caller owns the pointer.
   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.
   MappedMemoryRegion region_;
 
diff --git a/src/processor/minidump.cc b/src/processor/minidump.cc
index abe4d5dc..f6f642be 100644
--- a/src/processor/minidump.cc
+++ b/src/processor/minidump.cc
@@ -3999,15 +3999,17 @@ MinidumpLinuxMapsList::MinidumpLinuxMapsList(Minidump *minidump)
 }
 
 MinidumpLinuxMapsList::~MinidumpLinuxMapsList() {
-  for (unsigned int i = 0; i < maps_->size(); i++) {
-    delete (*maps_)[i];
+  if (maps_) {
+    for (unsigned int i = 0; i < maps_->size(); i++) {
+      delete (*maps_)[i];
+    }
+    delete maps_;
   }
-  delete maps_;
 }
 
 const MinidumpLinuxMaps *MinidumpLinuxMapsList::GetLinuxMapsForAddress(
     uint64_t address) const {
-  if (!valid_) {
+  if (!valid_ || (maps_ == NULL)) {
     BPLOG(ERROR) << "Invalid MinidumpLinuxMapsList for GetLinuxMapsForAddress";
     return NULL;
   }
@@ -4029,13 +4031,13 @@ const MinidumpLinuxMaps *MinidumpLinuxMapsList::GetLinuxMapsForAddress(
 
 const MinidumpLinuxMaps *MinidumpLinuxMapsList::GetLinuxMapsAtIndex(
     unsigned int index) const {
-  if (!valid_) {
+  if (!valid_ || (maps_ == NULL)) {
     BPLOG(ERROR) << "Invalid MinidumpLinuxMapsList for GetLinuxMapsAtIndex";
     return NULL;
   }
 
   // Index out of bounds.
-  if (index >= maps_count_) {
+  if (index >= maps_count_ || (maps_ == NULL)) {
     BPLOG(ERROR) << "MinidumpLinuxMapsList index of out range: "
                  << index
                  << "/"
@@ -4047,7 +4049,12 @@ const MinidumpLinuxMaps *MinidumpLinuxMapsList::GetLinuxMapsAtIndex(
 
 bool MinidumpLinuxMapsList::Read(uint32_t expected_size) {
   // Invalidate cached data.
-  delete maps_;
+  if (maps_) {
+    for (unsigned int i = 0; i < maps_->size(); i++) {
+      delete (*maps_)[i];
+    }
+    delete maps_;
+  }
   maps_ = NULL;
   maps_count_ = 0;
 
@@ -4100,7 +4107,7 @@ bool MinidumpLinuxMapsList::Read(uint32_t expected_size) {
 }
 
 void MinidumpLinuxMapsList::Print() {
-  if (!valid_) {
+  if (!valid_ || (maps_ == NULL)) {
     BPLOG(ERROR) << "MinidumpLinuxMapsList cannot print valid data";
     return;
   }