From 76badd4c20acdd9f06f269a5566cc3f6a095f265 Mon Sep 17 00:00:00 2001 From: Arthur Wang Date: Wed, 1 May 2024 15:17:40 -0700 Subject: [PATCH] Replace std::unique_ptr with HeapArray in process_info.cc and 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 Commit-Queue: Arthur Wang --- DEPS | 2 +- util/win/process_info.cc | 44 ++++++++++++++++++----------------- util/win/process_info_test.cc | 24 +++++++++---------- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/DEPS b/DEPS index c8936e0e..1384fdd1 100644 --- a/DEPS +++ b/DEPS @@ -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', diff --git a/util/win/process_info.cc b/util/win/process_info.cc index 888b9ea2..910b183f 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -24,6 +24,7 @@ #include #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" @@ -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 QueryObject( +base::HeapArray QueryObject( HANDLE handle, OBJECT_INFORMATION_CLASS object_information_class, ULONG minimum_size) { - ULONG size = minimum_size; ULONG return_length; - std::unique_ptr buffer(new uint8_t[size]); - NTSTATUS status = crashpad::NtQueryObject( - handle, object_information_class, buffer.get(), size, &return_length); + auto buffer = base::HeapArray::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; + DCHECK_GT(return_length, buffer.size()); - // 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); + buffer = base::HeapArray::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(); } - DCHECK_LE(return_length, size); + DCHECK_LE(return_length, buffer.size()); DCHECK_GE(return_length, minimum_size); return buffer; } @@ -413,14 +415,14 @@ std::vector ProcessInfo::BuildHandleVector( // information, but include the information that we do have already. ScopedKernelHANDLE scoped_dup_handle(dup_handle); - std::unique_ptr 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( - 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 @@ -439,14 +441,14 @@ std::vector ProcessInfo::BuildHandleVector( result_handle.handle_count = object_basic_information->HandleCount - 1; } - std::unique_ptr 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( - object_type_information_buffer.get()); + object_type_information_buffer.data()); DCHECK_EQ(object_type_information->TypeName.Length % sizeof(result_handle.type_name[0]), diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index a4a5a57d..edd4c417 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -20,6 +20,7 @@ #include +#include "base/containers/heap_array.h" #include "base/files/file_path.h" #include "base/logging.h" #include "base/strings/stringprintf.h" @@ -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 into(new unsigned char[kBlockSize * 6]); + auto into = base::HeapArray::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) { @@ -633,15 +631,15 @@ TEST(ProcessInfo, Handles) { } TEST(ProcessInfo, OutOfRangeCheck) { - constexpr size_t kAllocationSize = 12345; - std::unique_ptr safe_memory(new char[kAllocationSize]); + auto safe_memory = base::HeapArray::Uninit(12345); ProcessInfo info; info.Initialize(GetCurrentProcess()); EXPECT_TRUE( info.LoggingRangeIsFullyReadable(CheckedRange( - FromPointerCast(safe_memory.get()), kAllocationSize))); + FromPointerCast(safe_memory.data()), + safe_memory.size()))); EXPECT_FALSE(info.LoggingRangeIsFullyReadable( CheckedRange(0, 1024))); }