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>
This commit is contained in:
Mark Mentovai 2017-04-11 17:28:16 -04:00 committed by Commit Bot
parent 30385d4e47
commit 79425e4d97
2 changed files with 16 additions and 2 deletions

View File

@ -57,6 +57,7 @@ process_types::SYSTEM_PROCESS_INFORMATION<Traits>* GetProcessInformation(
HANDLE process_handle, HANDLE process_handle,
std::unique_ptr<uint8_t[]>* buffer) { std::unique_ptr<uint8_t[]>* buffer) {
ULONG buffer_size = 16384; ULONG buffer_size = 16384;
ULONG actual_size;
buffer->reset(new uint8_t[buffer_size]); buffer->reset(new uint8_t[buffer_size]);
NTSTATUS status; NTSTATUS status;
// This must be in retry loop, as we're racing with process creation on the // 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, SystemProcessInformation,
reinterpret_cast<void*>(buffer->get()), reinterpret_cast<void*>(buffer->get()),
buffer_size, buffer_size,
&buffer_size); &actual_size);
if (status == STATUS_BUFFER_TOO_SMALL || if (status == STATUS_BUFFER_TOO_SMALL ||
status == STATUS_INFO_LENGTH_MISMATCH) { 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 // 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 // racing with system-wide process creation between here and the next call
// to NtQuerySystemInformation(). // 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]); buffer->reset(new uint8_t[buffer_size]);
} else { } else {
break; break;
@ -84,6 +91,8 @@ process_types::SYSTEM_PROCESS_INFORMATION<Traits>* GetProcessInformation(
return nullptr; return nullptr;
} }
DCHECK_LE(actual_size, buffer_size);
process_types::SYSTEM_PROCESS_INFORMATION<Traits>* process = process_types::SYSTEM_PROCESS_INFORMATION<Traits>* process =
reinterpret_cast<process_types::SYSTEM_PROCESS_INFORMATION<Traits>*>( reinterpret_cast<process_types::SYSTEM_PROCESS_INFORMATION<Traits>*>(
buffer->get()); buffer->get());

View File

@ -157,6 +157,10 @@ std::unique_ptr<uint8_t[]> QueryObject(
if (status == STATUS_INFO_LENGTH_MISMATCH) { if (status == STATUS_INFO_LENGTH_MISMATCH) {
DCHECK_GT(return_length, size); DCHECK_GT(return_length, size);
size = return_length; size = return_length;
// Free the old buffer before attempting to allocate a new one.
buffer.reset();
buffer.reset(new uint8_t[size]); buffer.reset(new uint8_t[size]);
status = crashpad::NtQueryObject( status = crashpad::NtQueryObject(
handle, object_information_class, buffer.get(), size, &return_length); handle, object_information_class, buffer.get(), size, &return_length);
@ -167,6 +171,7 @@ std::unique_ptr<uint8_t[]> QueryObject(
return nullptr; return nullptr;
} }
DCHECK_LE(return_length, size);
DCHECK_GE(return_length, minimum_size); DCHECK_GE(return_length, minimum_size);
return buffer; return buffer;
} }