From 79425e4d9737ebec7d7a932b03be9d21e279dae5 Mon Sep 17 00:00:00 2001
From: Mark Mentovai <mark@chromium.org>
Date: Tue, 11 Apr 2017 17:28:16 -0400
Subject: [PATCH] win: Free an old buffer before attempting to allocate a
 resized one

When GetProcessInformation() obtains SystemProcessInformation, it
resizes its buffer as directed by NtQuerySystemInformation(). Nothing of
value resides in the old buffer if a resize is attempted, so it can be
freed before attempting to allocate a resized one.

This may help crashes like go/crash/f385e94c80000000, which experience
out-of-memory while attempting to allocate a resized buffer. It also may
not help, because the required buffer size may just be too large to fit
in memory. See https://crashpad.chromium.org/bug/143#c19.

Change-Id: I63b9b8c1efda22d2fdbf05ef2b74975b92556bbd
Reviewed-on: https://chromium-review.googlesource.com/473792
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
---
 snapshot/win/process_reader_win.cc | 13 +++++++++++--
 util/win/process_info.cc           |  5 +++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/snapshot/win/process_reader_win.cc b/snapshot/win/process_reader_win.cc
index b7bae6ac..f8a2f928 100644
--- a/snapshot/win/process_reader_win.cc
+++ b/snapshot/win/process_reader_win.cc
@@ -57,6 +57,7 @@ process_types::SYSTEM_PROCESS_INFORMATION<Traits>* GetProcessInformation(
     HANDLE process_handle,
     std::unique_ptr<uint8_t[]>* buffer) {
   ULONG buffer_size = 16384;
+  ULONG actual_size;
   buffer->reset(new uint8_t[buffer_size]);
   NTSTATUS status;
   // This must be in retry loop, as we're racing with process creation on the
@@ -66,13 +67,19 @@ process_types::SYSTEM_PROCESS_INFORMATION<Traits>* GetProcessInformation(
         SystemProcessInformation,
         reinterpret_cast<void*>(buffer->get()),
         buffer_size,
-        &buffer_size);
+        &actual_size);
     if (status == STATUS_BUFFER_TOO_SMALL ||
         status == STATUS_INFO_LENGTH_MISMATCH) {
+      DCHECK_GT(actual_size, buffer_size);
+
       // Add a little extra to try to avoid an additional loop iteration. We're
       // racing with system-wide process creation between here and the next call
       // to NtQuerySystemInformation().
-      buffer_size += 4096;
+      buffer_size = actual_size + 4096;
+
+      // Free the old buffer before attempting to allocate a new one.
+      buffer->reset();
+
       buffer->reset(new uint8_t[buffer_size]);
     } else {
       break;
@@ -84,6 +91,8 @@ process_types::SYSTEM_PROCESS_INFORMATION<Traits>* GetProcessInformation(
     return nullptr;
   }
 
+  DCHECK_LE(actual_size, buffer_size);
+
   process_types::SYSTEM_PROCESS_INFORMATION<Traits>* process =
       reinterpret_cast<process_types::SYSTEM_PROCESS_INFORMATION<Traits>*>(
           buffer->get());
diff --git a/util/win/process_info.cc b/util/win/process_info.cc
index cb2051c0..f0e455db 100644
--- a/util/win/process_info.cc
+++ b/util/win/process_info.cc
@@ -157,6 +157,10 @@ std::unique_ptr<uint8_t[]> QueryObject(
   if (status == STATUS_INFO_LENGTH_MISMATCH) {
     DCHECK_GT(return_length, size);
     size = return_length;
+
+    // Free the old buffer before attempting to allocate a new one.
+    buffer.reset();
+
     buffer.reset(new uint8_t[size]);
     status = crashpad::NtQueryObject(
         handle, object_information_class, buffer.get(), size, &return_length);
@@ -167,6 +171,7 @@ std::unique_ptr<uint8_t[]> QueryObject(
     return nullptr;
   }
 
+  DCHECK_LE(return_length, size);
   DCHECK_GE(return_length, minimum_size);
   return buffer;
 }