Skip to content

Commit

Permalink
Replace std::unique_ptr<T[]> with HeapArray in process_info.cc and
Browse files Browse the repository at this point in the history 
process_info_test.cc

Bug: crashpad: 326459035,326458915,326459055
Change-Id: Ifb91297b6097aa81a9d5c883b2c284e9fdd512a8
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5463361
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Arthur Wang <wuwang@chromium.org>
  • Loading branch information
@wuwang-wang
wuwang-wang authored and Crashpad LUCI CQ committed May 2, 2024
1 parent 7e0af1d commit 76badd4
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 36 deletions.
2 changes: 1 addition & 1 deletion DEPS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ deps = {
'9719c1e1e676814c456b55f5f070eabad6709d31',
'crashpad/third_party/mini_chromium/mini_chromium':
Var('chromium_git') + '/chromium/mini_chromium@' +
'dce72d97d1c2e9beb5e206c6a05a702269794ca3',
'8b56c7718412ec7d12d05522f7af0cbb787cbb00',
'crashpad/third_party/libfuzzer/src':
Var('chromium_git') + '/chromium/llvm-project/compiler-rt/lib/fuzzer.git@' +
'fda403cf93ecb8792cb1d061564d89a6553ca020',
Expand Down
46 changes: 24 additions & 22 deletions util/win/process_info.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <type_traits>

#include "base/check_op.h"
#include "base/containers/heap_array.h"
#include "base/logging.h"
#include "base/memory/free_deleter.h"
#include "base/process/memory.h"
Expand Down Expand Up @@ -147,33 +148,34 @@ MEMORY_BASIC_INFORMATION64 MemoryBasicInformationToMemoryBasicInformation64(

// NtQueryObject with a retry for size mismatch as well as a minimum size to
// retrieve (and expect).
std::unique_ptr<uint8_t[]> QueryObject(
base::HeapArray<uint8_t> QueryObject(
HANDLE handle,
OBJECT_INFORMATION_CLASS object_information_class,
ULONG minimum_size) {
ULONG size = minimum_size;
ULONG return_length;
std::unique_ptr<uint8_t[]> buffer(new uint8_t[size]);
NTSTATUS status = crashpad::NtQueryObject(
handle, object_information_class, buffer.get(), size, &return_length);
auto buffer = base::HeapArray<uint8_t>::Uninit(minimum_size);
NTSTATUS status = crashpad::NtQueryObject(handle,
object_information_class,
buffer.data(),
(ULONG)buffer.size(),
&return_length);
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);
DCHECK_GT(return_length, buffer.size());

buffer = base::HeapArray<uint8_t>::Uninit(return_length);
status = crashpad::NtQueryObject(handle,
object_information_class,
buffer.data(),
(ULONG)buffer.size(),
&return_length);
}

if (!NT_SUCCESS(status)) {
NTSTATUS_LOG(ERROR, status) << "NtQueryObject";
return nullptr;
return base::HeapArray<uint8_t>();
}

DCHECK_LE(return_length, size);
DCHECK_LE(return_length, buffer.size());
DCHECK_GE(return_length, minimum_size);
return buffer;
}
Expand Down Expand Up @@ -413,14 +415,14 @@ std::vector<ProcessInfo::Handle> ProcessInfo::BuildHandleVector(
// information, but include the information that we do have already.
ScopedKernelHANDLE scoped_dup_handle(dup_handle);

std::unique_ptr<uint8_t[]> object_basic_information_buffer =
auto object_basic_information_buffer =
QueryObject(dup_handle,
ObjectBasicInformation,
sizeof(PUBLIC_OBJECT_BASIC_INFORMATION));
if (object_basic_information_buffer) {
if (!object_basic_information_buffer.empty()) {
PUBLIC_OBJECT_BASIC_INFORMATION* object_basic_information =
reinterpret_cast<PUBLIC_OBJECT_BASIC_INFORMATION*>(
object_basic_information_buffer.get());
object_basic_information_buffer.data());
// The Attributes and GrantedAccess sometimes differ slightly between
// the data retrieved in SYSTEM_HANDLE_INFORMATION_EX and
// PUBLIC_OBJECT_TYPE_INFORMATION. We prefer the values in
Expand All @@ -439,14 +441,14 @@ std::vector<ProcessInfo::Handle> ProcessInfo::BuildHandleVector(
result_handle.handle_count = object_basic_information->HandleCount - 1;
}

std::unique_ptr<uint8_t[]> object_type_information_buffer =
auto object_type_information_buffer =
QueryObject(dup_handle,
ObjectTypeInformation,
sizeof(PUBLIC_OBJECT_TYPE_INFORMATION));
if (object_type_information_buffer) {
if (!object_type_information_buffer.empty()) {
PUBLIC_OBJECT_TYPE_INFORMATION* object_type_information =
reinterpret_cast<PUBLIC_OBJECT_TYPE_INFORMATION*>(
object_type_information_buffer.get());
object_type_information_buffer.data());

DCHECK_EQ(object_type_information->TypeName.Length %
sizeof(result_handle.type_name[0]),
Expand Down
24 changes: 11 additions & 13 deletions util/win/process_info_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include <memory>

#include "base/containers/heap_array.h"
#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/strings/stringprintf.h"
Expand Down Expand Up @@ -507,26 +508,23 @@ TEST(ProcessInfo, ReadableRanges) {

// Also make sure what we think we can read corresponds with what we can
// actually read.
std::unique_ptr<unsigned char[]> into(new unsigned char[kBlockSize * 6]);
auto into = base::HeapArray<unsigned char>::Uninit(kBlockSize * 6);
SIZE_T bytes_read;

EXPECT_TRUE(ReadProcessMemory(
current_process, readable1, into.get(), kBlockSize, &bytes_read));
current_process, readable1, into.data(), kBlockSize, &bytes_read));
EXPECT_EQ(bytes_read, kBlockSize);

EXPECT_TRUE(ReadProcessMemory(
current_process, readable2, into.get(), kBlockSize * 2, &bytes_read));
current_process, readable2, into.data(), kBlockSize * 2, &bytes_read));
EXPECT_EQ(bytes_read, kBlockSize * 2);

EXPECT_FALSE(ReadProcessMemory(
current_process, no_access, into.get(), kBlockSize, &bytes_read));
current_process, no_access, into.data(), kBlockSize, &bytes_read));
EXPECT_FALSE(ReadProcessMemory(
current_process, reserve_region, into.get(), kBlockSize, &bytes_read));
EXPECT_FALSE(ReadProcessMemory(current_process,
reserve_region,
into.get(),
kBlockSize * 6,
&bytes_read));
current_process, reserve_region, into.data(), kBlockSize, &bytes_read));
EXPECT_FALSE(ReadProcessMemory(
current_process, reserve_region, into.data(), into.size(), &bytes_read));
}

TEST(ProcessInfo, Handles) {
Expand Down Expand Up @@ -633,15 +631,15 @@ TEST(ProcessInfo, Handles) {
}

TEST(ProcessInfo, OutOfRangeCheck) {
constexpr size_t kAllocationSize = 12345;
std::unique_ptr<char[]> safe_memory(new char[kAllocationSize]);
auto safe_memory = base::HeapArray<char>::Uninit(12345);

ProcessInfo info;
info.Initialize(GetCurrentProcess());

EXPECT_TRUE(
info.LoggingRangeIsFullyReadable(CheckedRange<WinVMAddress, WinVMSize>(
FromPointerCast<WinVMAddress>(safe_memory.get()), kAllocationSize)));
FromPointerCast<WinVMAddress>(safe_memory.data()),
safe_memory.size())));
EXPECT_FALSE(info.LoggingRangeIsFullyReadable(
CheckedRange<WinVMAddress, WinVMSize>(0, 1024)));
}
Expand Down

0 comments on commit 76badd4

Please sign in to comment.