Change ProcessMemory to accept VMSizes

As Mark noted in [1] ProcessMemory should accept VMSize instead of
size_t, the two types can differ on platforms where a cross-bitness
handler could cause a 32-bit handler to inspect a 64-bit process. By
centralizing the checks in ProcessMemory, we can leave the individual
platform-specific implementations (in ProcessMemory*::ReadUpTo) to
accept size_ts.

[1] crrev.com/c/1388017/2/snapshot/crashpad_types/crashpad_info_reader.cc#70

Bug: crashpad:270
Change-Id: I3aab483221de36f3b1478cb9503101b142dae681
Reviewed-on: https://chromium-review.googlesource.com/c/1387756
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
This commit is contained in:
Vlad Tsyrklevich 2018-12-21 13:06:12 -08:00 committed by Commit Bot
parent 760da9b96a
commit 60ff012872
2 changed files with 25 additions and 12 deletions

View File

@ -17,13 +17,20 @@
#include <algorithm>
#include "base/logging.h"
#include "util/numeric/safe_assignment.h"
namespace crashpad {
bool ProcessMemory::Read(VMAddress address, size_t size, void* buffer) const {
bool ProcessMemory::Read(VMAddress address, VMSize size, void* buffer) const {
size_t local_size;
if (!AssignIfInRange(&local_size, size)) {
LOG(ERROR) << "size " << size << " out of bounds for size_t";
return false;
}
char* buffer_c = static_cast<char*>(buffer);
while (size > 0) {
ssize_t bytes_read = ReadUpTo(address, size, buffer_c);
while (local_size > 0) {
ssize_t bytes_read = ReadUpTo(address, local_size, buffer_c);
if (bytes_read < 0) {
return false;
}
@ -31,8 +38,8 @@ bool ProcessMemory::Read(VMAddress address, size_t size, void* buffer) const {
LOG(ERROR) << "short read";
return false;
}
DCHECK_LE(static_cast<size_t>(bytes_read), size);
size -= bytes_read;
DCHECK_LE(static_cast<size_t>(bytes_read), local_size);
local_size -= bytes_read;
address += bytes_read;
buffer_c += bytes_read;
}
@ -41,15 +48,21 @@ bool ProcessMemory::Read(VMAddress address, size_t size, void* buffer) const {
bool ProcessMemory::ReadCStringInternal(VMAddress address,
bool has_size,
size_t size,
VMSize size,
std::string* string) const {
size_t local_size;
if (!AssignIfInRange(&local_size, size)) {
LOG(ERROR) << "size " << size << " out of bounds for size_t";
return false;
}
string->clear();
char buffer[4096];
do {
size_t read_size;
if (has_size) {
read_size = std::min(sizeof(buffer), size);
read_size = std::min(sizeof(buffer), local_size);
} else {
read_size = sizeof(buffer);
}
@ -70,8 +83,8 @@ bool ProcessMemory::ReadCStringInternal(VMAddress address,
string->append(buffer, bytes_read);
address += bytes_read;
size -= bytes_read;
} while (!has_size || size > 0);
local_size -= bytes_read;
} while (!has_size || local_size > 0);
LOG(ERROR) << "unterminated string";
return false;

View File

@ -46,7 +46,7 @@ class ProcessMemory {
//!
//! \return `true` on success, with \a buffer filled appropriately. `false` on
//! failure, with a message logged.
bool Read(VMAddress address, size_t size, void* buffer) const;
bool Read(VMAddress address, VMSize size, void* buffer) const;
//! \brief Reads a `NUL`-terminated C string from the target process into a
//! string in the current process.
@ -79,7 +79,7 @@ class ProcessMemory {
//! a `NUL` terminator is not found within \a size bytes, or when
//! encountering unmapped or unreadable pages.
bool ReadCStringSizeLimited(VMAddress address,
size_t size,
VMSize size,
std::string* string) const {
return ReadCStringInternal(address, true, size, string);
}
@ -124,7 +124,7 @@ class ProcessMemory {
//! encountering unmapped or unreadable pages.
virtual bool ReadCStringInternal(VMAddress address,
bool has_size,
size_t size,
VMSize size,
std::string* string) const;
};