Tolerate safe size mismatches in the CrashpadInfo struct

The handler will now be less strict about checking CrashpadInfo struct
sizes. Assuming the signature and version fields match:

 - If the handler sees a struct smaller than it’s expecting, the module
   was likely built with an earlier version of the client library, and
   it’s safe to treat the unknown fields as though they were zero or
   other suitable default values.
 - If the handler sees a struct larger than it’s expecting, the module
   was likely built with a later version of the client library. In that
   case, actions desired by the client will not be performed, but this
   is not otherwise an error condition.

The CrashpadInfo struct must always be at least large enough to contain
at least the size field. The signature and version fields are always
checked.

The section size must be at least as large as the size carried within
the struct. To account for possible section padding, strict equality is
not required.

Bug: chromium:784427
Test: crashpad_snapshot_test CrashpadInfoSizes_ClientOptions/*.*
Change-Id: Ibb0690ca6ed5e7619d1278a68ba7e893d55f19fb
Reviewed-on: https://chromium-review.googlesource.com/767709
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
This commit is contained in:
Mark Mentovai 2017-11-15 12:43:44 -05:00 committed by Commit Bot
parent 22e8c33b21
commit d7798a4e28
15 changed files with 516 additions and 78 deletions

View File

@ -25,6 +25,11 @@
namespace {
// Dont change this when simply adding fields. Readers will size-check the
// structure and ignore fields theyre aware of when not present, as well as
// fields theyre not aware of. Only change this when introducing an
// incompatible layout, with the understanding that existing readers will not
// understand new versions.
constexpr uint32_t kCrashpadInfoVersion = 1;
} // namespace
@ -106,13 +111,7 @@ CrashpadInfo::CrashpadInfo()
extra_memory_ranges_(nullptr),
simple_annotations_(nullptr),
user_data_minidump_stream_head_(nullptr),
annotations_list_(nullptr)
#if !defined(NDEBUG) && defined(OS_WIN)
,
invalid_read_detection_(0xbadc0de)
#endif
{
}
annotations_list_(nullptr) {}
void CrashpadInfo::AddUserDataMinidumpStream(uint32_t stream_type,
const void* data,

View File

@ -233,10 +233,10 @@ struct CrashpadInfo {
#pragma clang diagnostic ignored "-Wunused-private-field"
#endif
// Fields present in version 1:
// Fields present in version 1, subject to a check of the size_ field:
uint32_t signature_; // kSignature
uint32_t size_; // The size of the entire CrashpadInfo structure.
uint32_t version_; // kVersion
uint32_t version_; // kCrashpadInfoVersion
uint32_t indirectly_referenced_memory_cap_;
uint32_t padding_0_;
TriState crashpad_handler_behavior_;
@ -248,9 +248,11 @@ struct CrashpadInfo {
internal::UserDataMinidumpStreamListEntry* user_data_minidump_stream_head_;
AnnotationList* annotations_list_; // weak
#if !defined(NDEBUG) && defined(OS_WIN)
uint32_t invalid_read_detection_;
#endif
// Its generally safe to add new fields without changing
// kCrashpadInfoVersion, because readers should check size_ and ignore fields
// that arent present, as well as unknown fields.
//
// Adding fields? Consider snapshot/crashpad_info_size_test_module.cc too.
#if defined(__clang__)
#pragma clang diagnostic pop

View File

@ -14,6 +14,7 @@
#include "snapshot/crashpad_info_client_options.h"
#include "base/auto_reset.h"
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/strings/utf_string_conversions.h"
@ -229,6 +230,101 @@ TEST(CrashpadInfoClientOptions, TwoModules) {
}
}
class CrashpadInfoSizes_ClientOptions
: public testing::TestWithParam<base::FilePath::StringType> {};
TEST_P(CrashpadInfoSizes_ClientOptions, DifferentlySizedStruct) {
base::FilePath::StringType artifact(FILE_PATH_LITERAL("module_"));
artifact += GetParam();
// Open the module, which has a CrashpadInfo-like structure thats smaller or
// larger than the current versions CrashpadInfo structure defined in the
// client library.
base::FilePath module_path =
TestPaths::BuildArtifact(FILE_PATH_LITERAL("snapshot"),
artifact,
TestPaths::FileType::kLoadableModule);
#if defined(OS_MACOSX)
ScopedModuleHandle module(
dlopen(module_path.value().c_str(), RTLD_LAZY | RTLD_LOCAL));
ASSERT_TRUE(module.valid())
<< "dlopen " << module_path.value() << ": " << dlerror();
#elif defined(OS_WIN)
ScopedModuleHandle module(LoadLibrary(module_path.value().c_str()));
ASSERT_TRUE(module.valid())
<< "LoadLibrary " << base::UTF16ToUTF8(module_path.value()) << ": "
<< ErrorMessage();
#else
#error Port.
#endif // OS_MACOSX
// Get the function pointer from the module.
CrashpadInfo* (*TestModule_GetCrashpadInfo)() =
module.LookUpSymbol<CrashpadInfo* (*)()>("TestModule_GetCrashpadInfo");
ASSERT_TRUE(TestModule_GetCrashpadInfo);
auto options = SelfProcessSnapshotAndGetCrashpadOptions();
// Make sure that the initial state has all values unset.
EXPECT_EQ(options.crashpad_handler_behavior, TriState::kUnset);
EXPECT_EQ(options.system_crash_reporter_forwarding, TriState::kUnset);
EXPECT_EQ(options.gather_indirectly_referenced_memory, TriState::kUnset);
// Get the remote CrashpadInfo structure.
CrashpadInfo* remote_crashpad_info = TestModule_GetCrashpadInfo();
ASSERT_TRUE(remote_crashpad_info);
{
ScopedUnsetCrashpadInfoOptions unset_remote(remote_crashpad_info);
// Make sure that a change in the remote structure can be read back out,
// even though its a different size.
remote_crashpad_info->set_crashpad_handler_behavior(TriState::kEnabled);
remote_crashpad_info->set_system_crash_reporter_forwarding(
TriState::kDisabled);
options = SelfProcessSnapshotAndGetCrashpadOptions();
EXPECT_EQ(options.crashpad_handler_behavior, TriState::kEnabled);
EXPECT_EQ(options.system_crash_reporter_forwarding, TriState::kDisabled);
EXPECT_EQ(options.gather_indirectly_referenced_memory, TriState::kUnset);
}
{
ScopedUnsetCrashpadInfoOptions unset_remote(remote_crashpad_info);
// Make sure that the portion of the remote structure lying beyond its
// declared size reads as zero.
// 4 = offsetof(CrashpadInfo, size_), but its private.
uint32_t* size = reinterpret_cast<uint32_t*>(
reinterpret_cast<char*>(remote_crashpad_info) + 4);
// 21 = offsetof(CrashpadInfo, system_crash_reporter_forwarding_, but its
// private.
base::AutoReset<uint32_t> reset_size(size, 21);
// system_crash_reporter_forwarding_ is now beyond the structs declared
// size. Storage has actually been allocated for it, so its safe to set
// here.
remote_crashpad_info->set_crashpad_handler_behavior(TriState::kEnabled);
remote_crashpad_info->set_system_crash_reporter_forwarding(
TriState::kDisabled);
// Since system_crash_reporter_forwarding_ is beyond the structs declared
// size, it should read as 0 (TriState::kUnset), even though it was set to
// a different value above.
options = SelfProcessSnapshotAndGetCrashpadOptions();
EXPECT_EQ(options.crashpad_handler_behavior, TriState::kEnabled);
EXPECT_EQ(options.system_crash_reporter_forwarding, TriState::kUnset);
EXPECT_EQ(options.gather_indirectly_referenced_memory, TriState::kUnset);
}
}
INSTANTIATE_TEST_CASE_P(CrashpadInfoSizes_ClientOptions,
CrashpadInfoSizes_ClientOptions,
testing::Values(FILE_PATH_LITERAL("small"),
FILE_PATH_LITERAL("large")));
} // namespace
} // namespace test
} // namespace crashpad

View File

@ -0,0 +1,113 @@
// Copyright 2017 The Crashpad Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include <stdint.h>
#include "build/build_config.h"
#if defined(OS_MACOSX)
#include <mach-o/loader.h>
#elif defined(OS_WIN)
#include <windows.h>
#endif // OS_MACOSX
namespace crashpad {
namespace {
#if defined(CRASHPAD_INFO_SIZE_TEST_MODULE_SMALL) == \
defined(CRASHPAD_INFO_SIZE_TEST_MODULE_LARGE)
#error Define exactly one of these macros
#endif
// This module contains a CrashpadInfo structure thats either smaller or larger
// than the one defined in the client library, depending on which macro is
// defined when its compiled. This tests the snapshot layers ability to read
// smaller structures (as might be found in modules built with older versions of
// the client library than a handlers snapshot library) and larger ones (the
// “vice-versa” situation). This needs to be done without taking a dependency on
// the client library, which would bring with it a correct copy of the
// CrashpadInfo structure. As a result, all types have been simplified to
// fixed-size integers and void* pointers.
struct TestCrashpadInfo {
uint32_t signature_;
uint32_t size_;
uint32_t version_;
uint32_t indirectly_referenced_memory_cap_;
uint32_t padding_0_;
uint8_t crashpad_handler_behavior_;
uint8_t system_crash_reporter_forwarding_;
uint8_t gather_indirectly_referenced_memory_;
uint8_t padding_1_;
void* extra_memory_ranges_;
void* simple_annotations_;
#if !defined(CRASHPAD_INFO_SIZE_TEST_MODULE_SMALL)
void* user_data_minidump_stream_head_;
void* annotations_list_;
#endif // CRASHPAD_INFO_SIZE_TEST_MODULE_SMALL
#if defined(CRASHPAD_INFO_SIZE_TEST_MODULE_LARGE)
uint8_t trailer_[64 * 1024];
#endif // CRASHPAD_INFO_SIZE_TEST_MODULE_LARGE
};
// Put it in the correct section.
//
// The initializer also duplicates constants from the client library, sufficient
// to get this test version to be interpreted as a genuine CrashpadInfo
// structure. The size is set to the actual size of this structure (thats kind
// of the point of this test).
#if defined(OS_POSIX)
__attribute__((
#if defined(OS_MACOSX)
section(SEG_DATA ",crashpad_info"),
#elif defined(OS_LINUX) || defined(OS_ANDROID)
section("crashpad_info"),
#else // !defined(OS_MACOSX) && !defined(OS_LINUX) && !defined(OS_ANDROID)
#error Port
#endif // !defined(OS_MACOSX) && !defined(OS_LINUX) && !defined(OS_ANDROID)
#if defined(ADDRESS_SANITIZER)
aligned(64),
#endif // defined(ADDRESS_SANITIZER)
used,
visibility("hidden")))
#elif defined(OS_WIN)
#pragma section("CPADinfo", read, write)
__declspec(allocate("CPADinfo"))
#else // !defined(OS_POSIX) && !defined(OS_WIN)
#error Port
#endif // !defined(OS_POSIX) && !defined(OS_WIN)
TestCrashpadInfo g_test_crashpad_info = {'CPad', sizeof(TestCrashpadInfo), 1};
} // namespace
} // namespace crashpad
extern "C" {
#if defined(OS_POSIX)
__attribute__((visibility("default")))
#elif defined(OS_WIN)
__declspec(dllexport)
#else
#error Port
#endif // OS_POSIX
crashpad::TestCrashpadInfo* TestModule_GetCrashpadInfo() {
return &crashpad::g_test_crashpad_info;
}
} // extern "C"
#if defined(OS_WIN)
BOOL WINAPI DllMain(HINSTANCE hinstance, DWORD reason, LPVOID reserved) {
return TRUE;
}
#endif // OS_WIN

View File

@ -153,11 +153,8 @@ void MachOImageAnnotationsReader::ReadDyldErrorStringAnnotation(
void MachOImageAnnotationsReader::ReadCrashpadSimpleAnnotations(
std::map<std::string, std::string>* simple_map_annotations) const {
process_types::CrashpadInfo crashpad_info;
if (!image_reader_->GetCrashpadInfo(&crashpad_info)) {
return;
}
if (!crashpad_info.simple_annotations) {
if (!image_reader_->GetCrashpadInfo(&crashpad_info) ||
!crashpad_info.simple_annotations) {
return;
}
@ -189,11 +186,8 @@ void MachOImageAnnotationsReader::ReadCrashpadSimpleAnnotations(
void MachOImageAnnotationsReader::ReadCrashpadAnnotationsList(
std::vector<AnnotationSnapshot>* annotations) const {
process_types::CrashpadInfo crashpad_info;
if (!image_reader_->GetCrashpadInfo(&crashpad_info)) {
return;
}
if (!crashpad_info.annotations_list) {
if (!image_reader_->GetCrashpadInfo(&crashpad_info) ||
!crashpad_info.annotations_list) {
return;
}

View File

@ -481,24 +481,43 @@ bool MachOImageReader::GetCrashpadInfo(
}
if (crashpad_info_section->size <
crashpad_info->ExpectedSize(process_reader_)) {
crashpad_info->MinimumSize(process_reader_)) {
LOG(WARNING) << "small crashpad info section size "
<< crashpad_info_section->size << module_info_;
return false;
}
// This Read() will zero out anything beyond the structures declared size.
if (!crashpad_info->Read(process_reader_, crashpad_info_address)) {
LOG(WARNING) << "could not read crashpad info" << module_info_;
return false;
}
if (crashpad_info->signature != CrashpadInfo::kSignature ||
crashpad_info->size != crashpad_info_section->size ||
crashpad_info->version < 1) {
LOG(WARNING) << "unexpected crashpad info data" << module_info_;
crashpad_info->version != 1) {
LOG(WARNING) << base::StringPrintf(
"unexpected crashpad info signature 0x%x, version %u%s",
crashpad_info->signature,
crashpad_info->version,
module_info_.c_str());
return false;
}
// Dont require strict equality, to leave wiggle room for sloppy linkers.
if (crashpad_info->size > crashpad_info_section->size) {
LOG(WARNING) << "crashpad info struct size " << crashpad_info->size
<< " large for section size " << crashpad_info_section->size
<< module_info_;
return false;
}
if (crashpad_info->size > crashpad_info->ExpectedSize(process_reader_)) {
// This isnt strictly a problem, because unknown fields will simply be
// ignored, but it may be of diagnostic interest.
LOG(INFO) << "large crashpad info size " << crashpad_info->size
<< module_info_;
}
return true;
}

View File

@ -14,6 +14,7 @@
#include "snapshot/mac/process_types.h"
#include <stddef.h>
#include <string.h>
#include <uuid/uuid.h>
@ -140,6 +141,8 @@ inline void Assign<UInt64Array4, UInt32Array4>(UInt64Array4* destination,
#define PROCESS_TYPE_STRUCT_VERSIONED(struct_name, version_field)
#define PROCESS_TYPE_STRUCT_SIZED(struct_name, size_field)
#define PROCESS_TYPE_STRUCT_END(struct_name) \
} \
} /* namespace internal */ \
@ -151,6 +154,7 @@ inline void Assign<UInt64Array4, UInt32Array4>(UInt64Array4* destination,
#undef PROCESS_TYPE_STRUCT_BEGIN
#undef PROCESS_TYPE_STRUCT_MEMBER
#undef PROCESS_TYPE_STRUCT_VERSIONED
#undef PROCESS_TYPE_STRUCT_SIZED
#undef PROCESS_TYPE_STRUCT_END
#undef PROCESS_TYPE_STRUCT_IMPLEMENT
@ -183,6 +187,8 @@ inline void Assign<UInt64Array4, UInt32Array4>(UInt64Array4* destination,
#define PROCESS_TYPE_STRUCT_VERSIONED(struct_name, version_field)
#define PROCESS_TYPE_STRUCT_SIZED(struct_name, size_field)
#define PROCESS_TYPE_STRUCT_END(struct_name)
#include "snapshot/mac/process_types/all.proctype"
@ -190,6 +196,7 @@ inline void Assign<UInt64Array4, UInt32Array4>(UInt64Array4* destination,
#undef PROCESS_TYPE_STRUCT_BEGIN
#undef PROCESS_TYPE_STRUCT_MEMBER
#undef PROCESS_TYPE_STRUCT_VERSIONED
#undef PROCESS_TYPE_STRUCT_SIZED
#undef PROCESS_TYPE_STRUCT_END
#undef PROCESS_TYPE_STRUCT_IMPLEMENT_INTERNAL_READ_INTO
@ -254,6 +261,8 @@ inline void Assign<UInt64Array4, UInt32Array4>(UInt64Array4* destination,
#define PROCESS_TYPE_STRUCT_VERSIONED(struct_name, version_field)
#define PROCESS_TYPE_STRUCT_SIZED(struct_name, size_field)
#define PROCESS_TYPE_STRUCT_END(struct_name)
#include "snapshot/mac/process_types/all.proctype"
@ -261,6 +270,7 @@ inline void Assign<UInt64Array4, UInt32Array4>(UInt64Array4* destination,
#undef PROCESS_TYPE_STRUCT_BEGIN
#undef PROCESS_TYPE_STRUCT_MEMBER
#undef PROCESS_TYPE_STRUCT_VERSIONED
#undef PROCESS_TYPE_STRUCT_SIZED
#undef PROCESS_TYPE_STRUCT_END
#undef PROCESS_TYPE_STRUCT_IMPLEMENT_ARRAY
@ -297,6 +307,8 @@ inline void Assign<UInt64Array4, UInt32Array4>(UInt64Array4* destination,
} /* namespace process_types */ \
} /* namespace crashpad */
#define PROCESS_TYPE_STRUCT_SIZED(struct_name, size_field)
#define PROCESS_TYPE_STRUCT_END(struct_name)
#include "snapshot/mac/process_types/all.proctype"
@ -304,5 +316,62 @@ inline void Assign<UInt64Array4, UInt32Array4>(UInt64Array4* destination,
#undef PROCESS_TYPE_STRUCT_BEGIN
#undef PROCESS_TYPE_STRUCT_MEMBER
#undef PROCESS_TYPE_STRUCT_VERSIONED
#undef PROCESS_TYPE_STRUCT_SIZED
#undef PROCESS_TYPE_STRUCT_END
#undef PROCESS_TYPE_STRUCT_IMPLEMENT_VERSIONED
// Implement the generic crashpad::process_types::struct_name MinimumSize() and
// its templatized equivalent. The generic version delegates to the templatized
// one, which returns the minimum size of a sized structure. This can be used to
// ensure that enough of a sized structure is available to interpret its size
// field. This is only implemented for structures that use
// PROCESS_TYPE_STRUCT_SIZED().
#define PROCESS_TYPE_STRUCT_IMPLEMENT_SIZED 1
#define PROCESS_TYPE_STRUCT_BEGIN(struct_name)
#define PROCESS_TYPE_STRUCT_MEMBER(member_type, member_name, ...)
#define PROCESS_TYPE_STRUCT_VERSIONED(struct_name, version_field)
#define PROCESS_TYPE_STRUCT_SIZED(struct_name, size_field) \
namespace crashpad { \
namespace process_types { \
\
namespace internal { \
\
/* static */ \
template <typename Traits> \
size_t struct_name<Traits>::MinimumSize() { \
return offsetof(struct_name<Traits>, size_field) + \
sizeof(struct_name<Traits>::size_field); \
} \
\
/* Explicit instantiations of the above. */ \
template size_t struct_name<Traits32>::MinimumSize(); \
template size_t struct_name<Traits64>::MinimumSize(); \
\
} /* namespace internal */ \
\
/* static */ \
size_t struct_name::MinimumSize(ProcessReader* process_reader) { \
if (!process_reader->Is64Bit()) { \
return internal::struct_name<internal::Traits32>::MinimumSize(); \
} else { \
return internal::struct_name<internal::Traits64>::MinimumSize(); \
} \
} \
\
} /* namespace process_types */ \
} /* namespace crashpad */
#define PROCESS_TYPE_STRUCT_END(struct_name)
#include "snapshot/mac/process_types/all.proctype"
#undef PROCESS_TYPE_STRUCT_BEGIN
#undef PROCESS_TYPE_STRUCT_MEMBER
#undef PROCESS_TYPE_STRUCT_VERSIONED
#undef PROCESS_TYPE_STRUCT_SIZED
#undef PROCESS_TYPE_STRUCT_END
#undef PROCESS_TYPE_STRUCT_IMPLEMENT_SIZED

View File

@ -94,12 +94,14 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64)
\
/* Returns the size of the object that was read. This is the size of the \
* storage in the process that the data is read from, and is not the same \
* as the size of the generic struct. */ \
* as the size of the generic struct. For versioned and sized structures, \
* the size of the full structure is returned. */ \
size_t Size() const { return size_; } \
\
/* Similar to Size(), but computes the expected size of a structure based \
* on the process bitness. This can be used prior to reading any data \
* from a process. */ \
* from a process. For versioned and sized structures, \
* ExpectedSizeForVersion() and MinimumSize() may also be useful. */ \
static size_t ExpectedSize(ProcessReader* process_reader);
#define PROCESS_TYPE_STRUCT_MEMBER(member_type, member_name, ...) \
@ -114,6 +116,13 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64)
ProcessReader* process_reader, \
decltype(struct_name::version_field) version);
#define PROCESS_TYPE_STRUCT_SIZED(struct_name, size_field) \
/* Similar to ExpectedSize(), but computes the minimum size of a \
* structure based on the process bitness, typically including enough of \
* a structure to contain its size field. This can be used prior to \
* reading any data from a process. */ \
static size_t MinimumSize(ProcessReader* process_reader);
#define PROCESS_TYPE_STRUCT_END(struct_name) \
private: \
/* The static form of Read(). Populates the struct at |generic|. */ \
@ -140,6 +149,7 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64)
#undef PROCESS_TYPE_STRUCT_BEGIN
#undef PROCESS_TYPE_STRUCT_MEMBER
#undef PROCESS_TYPE_STRUCT_VERSIONED
#undef PROCESS_TYPE_STRUCT_SIZED
#undef PROCESS_TYPE_STRUCT_END
#undef PROCESS_TYPE_STRUCT_DECLARE
@ -195,6 +205,10 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64)
static size_t ExpectedSizeForVersion( \
decltype(struct_name::version_field) version);
#define PROCESS_TYPE_STRUCT_SIZED(struct_name, size_field) \
/* MinimumSize() is as in the generic user-visible struct above. */ \
static size_t MinimumSize();
#define PROCESS_TYPE_STRUCT_END(struct_name) \
private: \
/* ReadInto() is as in the generic user-visible struct above. */ \
@ -211,6 +225,7 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64)
#undef PROCESS_TYPE_STRUCT_BEGIN
#undef PROCESS_TYPE_STRUCT_MEMBER
#undef PROCESS_TYPE_STRUCT_VERSIONED
#undef PROCESS_TYPE_STRUCT_SIZED
#undef PROCESS_TYPE_STRUCT_END
#undef PROCESS_TYPE_STRUCT_DECLARE_INTERNAL

View File

@ -23,10 +23,25 @@
// Client Mach-O images will contain a __DATA,crashpad_info section formatted
// according to this structure.
//
// CrashpadInfo is variable-length. Its length dictated by its |size| field
// which is always present. A custom implementation of the flavored
// ReadSpecificInto function that understands how to use this field is provided
// in snapshot/mac/process_types/custom.cc. No implementation of ReadArrayInto
// is provided because CrashpadInfo structs are singletons in a module and are
// never present in arrays, so the functionality is unnecessary.
#if !defined(PROCESS_TYPE_STRUCT_IMPLEMENT_INTERNAL_READ_INTO) && \
!defined(PROCESS_TYPE_STRUCT_IMPLEMENT_ARRAY)
PROCESS_TYPE_STRUCT_BEGIN(CrashpadInfo)
PROCESS_TYPE_STRUCT_MEMBER(uint32_t, signature)
PROCESS_TYPE_STRUCT_MEMBER(uint32_t, size)
PROCESS_TYPE_STRUCT_SIZED(CrashpadInfo, size)
PROCESS_TYPE_STRUCT_MEMBER(uint32_t, version)
PROCESS_TYPE_STRUCT_MEMBER(uint32_t, indirectly_referenced_memory_cap)
PROCESS_TYPE_STRUCT_MEMBER(uint32_t, padding_0)
PROCESS_TYPE_STRUCT_MEMBER(uint8_t, crashpad_handler_behavior) // TriState
@ -51,3 +66,6 @@ PROCESS_TYPE_STRUCT_BEGIN(CrashpadInfo)
// AnnotationList*
PROCESS_TYPE_STRUCT_MEMBER(Pointer, annotations_list)
PROCESS_TYPE_STRUCT_END(CrashpadInfo)
#endif // ! PROCESS_TYPE_STRUCT_IMPLEMENT_INTERNAL_READ_INTO &&
// ! PROCESS_TYPE_STRUCT_IMPLEMENT_ARRAY

View File

@ -33,8 +33,8 @@
// flavored ReadSpecificInto function that understands how to map this field to
// the structures actual size is provided in
// snapshot/mac/process_types/custom.cc. No implementation of ReadArrayInto is
// provided because dyld_all_image_infos structs are singletons in a process and
// are never present in arrays, so the functionality is unnecessary.
// provided because crashreporter_annotations_t structs are singletons in a
// module and are never present in arrays, so the functionality is unnecessary.
#if !defined(PROCESS_TYPE_STRUCT_IMPLEMENT_INTERNAL_READ_INTO) && \
!defined(PROCESS_TYPE_STRUCT_IMPLEMENT_ARRAY)

View File

@ -18,9 +18,12 @@
#include <string.h>
#include <sys/types.h>
#include <algorithm>
#include <type_traits>
#include "base/logging.h"
#include "base/numerics/safe_math.h"
#include "base/strings/stringprintf.h"
#include "snapshot/mac/process_types/internal.h"
#include "util/mach/task_memory.h"
@ -33,32 +36,88 @@ namespace internal {
namespace {
template <typename T>
bool ReadIntoVersioned(ProcessReader* process_reader,
mach_vm_address_t address,
T* specific) {
TaskMemory* task_memory = process_reader->Memory();
if (!task_memory->Read(
address, sizeof(specific->version), &specific->version)) {
return false;
}
mach_vm_size_t size = T::ExpectedSizeForVersion(specific->version);
bool ReadIntoAndZero(TaskMemory* task_memory,
mach_vm_address_t address,
mach_vm_size_t size,
T* specific) {
DCHECK_LE(size, sizeof(*specific));
if (!task_memory->Read(address, size, specific)) {
return false;
}
// Zero out the rest of the structure in case anything accesses fields without
// checking the version.
size_t remaining = sizeof(*specific) - size;
// checking the version or size.
const size_t remaining = sizeof(*specific) - size;
if (remaining > 0) {
char* start = reinterpret_cast<char*>(specific) + size;
char* const start = reinterpret_cast<char*>(specific) + size;
memset(start, 0, remaining);
}
return true;
}
template <typename T>
bool FieldAddressIfInRange(mach_vm_address_t address,
size_t offset,
mach_vm_address_t* field_address) {
base::CheckedNumeric<typename T::Pointer> checked_field_address(address);
checked_field_address += offset;
typename T::Pointer local_field_address;
if (!checked_field_address.AssignIfValid(&local_field_address)) {
LOG(ERROR) << base::StringPrintf(
"address 0x%llx + offset 0x%zx out of range", address, offset);
return false;
}
*field_address = local_field_address;
return true;
}
template <typename T>
bool ReadIntoVersioned(ProcessReader* process_reader,
mach_vm_address_t address,
T* specific) {
mach_vm_address_t field_address;
if (!FieldAddressIfInRange<T>(
address, offsetof(T, version), &field_address)) {
return false;
}
TaskMemory* task_memory = process_reader->Memory();
decltype(specific->version) version;
if (!task_memory->Read(field_address, sizeof(version), &version)) {
return false;
}
const size_t size = T::ExpectedSizeForVersion(version);
return ReadIntoAndZero(task_memory, address, size, specific);
}
template <typename T>
bool ReadIntoSized(ProcessReader* process_reader,
mach_vm_address_t address,
T* specific) {
mach_vm_address_t field_address;
if (!FieldAddressIfInRange<T>(address, offsetof(T, size), &field_address)) {
return false;
}
TaskMemory* task_memory = process_reader->Memory();
decltype(specific->size) size;
if (!task_memory->Read(address + offsetof(T, size), sizeof(size), &size)) {
return false;
}
if (size < T::MinimumSize()) {
LOG(ERROR) << "small size " << size;
return false;
}
size = std::min(static_cast<size_t>(size), sizeof(*specific));
return ReadIntoAndZero(task_memory, address, size, specific);
}
} // namespace
// static
@ -125,22 +184,32 @@ bool crashreporter_annotations_t<Traits>::ReadInto(
return ReadIntoVersioned(process_reader, address, specific);
}
// static
template <typename Traits>
bool CrashpadInfo<Traits>::ReadInto(ProcessReader* process_reader,
mach_vm_address_t address,
CrashpadInfo<Traits>* specific) {
return ReadIntoSized(process_reader, address, specific);
}
// Explicit template instantiation of the above.
#define PROCESS_TYPE_FLAVOR_TRAITS(lp_bits) \
template size_t \
dyld_all_image_infos<Traits##lp_bits>::ExpectedSizeForVersion( \
decltype(dyld_all_image_infos<Traits##lp_bits>::version)); \
dyld_all_image_infos<Traits##lp_bits>::ExpectedSizeForVersion( \
decltype(dyld_all_image_infos<Traits##lp_bits>::version)); \
template bool dyld_all_image_infos<Traits##lp_bits>::ReadInto( \
ProcessReader*, \
mach_vm_address_t, \
dyld_all_image_infos<Traits##lp_bits>*); \
template size_t \
crashreporter_annotations_t<Traits##lp_bits>::ExpectedSizeForVersion( \
decltype(crashreporter_annotations_t<Traits##lp_bits>::version)); \
crashreporter_annotations_t<Traits##lp_bits>::ExpectedSizeForVersion( \
decltype(crashreporter_annotations_t<Traits##lp_bits>::version)); \
template bool crashreporter_annotations_t<Traits##lp_bits>::ReadInto( \
ProcessReader*, \
mach_vm_address_t, \
crashreporter_annotations_t<Traits##lp_bits>*);
crashreporter_annotations_t<Traits##lp_bits>*); \
template bool CrashpadInfo<Traits##lp_bits>::ReadInto( \
ProcessReader*, mach_vm_address_t, CrashpadInfo<Traits##lp_bits>*);
#include "snapshot/mac/process_types/flavors.h"

View File

@ -53,6 +53,8 @@
'type': 'executable',
'dependencies': [
'crashpad_snapshot_test_module',
'crashpad_snapshot_test_module_large',
'crashpad_snapshot_test_module_small',
'snapshot.gyp:crashpad_snapshot',
'snapshot.gyp:crashpad_snapshot_api',
'../client/client.gyp:crashpad_client',
@ -161,6 +163,32 @@
'crashpad_info_client_options_test_module.cc',
],
},
{
'target_name': 'crashpad_snapshot_test_module_large',
'type': 'loadable_module',
'dependencies': [
'../third_party/mini_chromium/mini_chromium.gyp:base',
],
'defines': [
'CRASHPAD_INFO_SIZE_TEST_MODULE_LARGE=1',
],
'sources': [
'crashpad_info_size_test_module.cc',
],
},
{
'target_name': 'crashpad_snapshot_test_module_small',
'type': 'loadable_module',
'dependencies': [
'../third_party/mini_chromium/mini_chromium.gyp:base',
],
'defines': [
'CRASHPAD_INFO_SIZE_TEST_MODULE_SMALL=1',
],
'sources': [
'crashpad_info_size_test_module.cc',
],
},
],
'conditions': [
['OS=="mac"', {

View File

@ -272,11 +272,10 @@ template <class Traits>
void ModuleSnapshotWin::GetCrashpadExtraMemoryRanges(
std::set<CheckedRange<uint64_t>>* ranges) const {
process_types::CrashpadInfo<Traits> crashpad_info;
if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info))
return;
if (!crashpad_info.extra_address_ranges)
if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info) ||
!crashpad_info.extra_address_ranges) {
return;
}
std::vector<SimpleAddressRangeBag::Entry> simple_ranges(
SimpleAddressRangeBag::num_entries);

View File

@ -85,11 +85,10 @@ template <class Traits>
void PEImageAnnotationsReader::ReadCrashpadSimpleAnnotations(
std::map<std::string, std::string>* simple_map_annotations) const {
process_types::CrashpadInfo<Traits> crashpad_info;
if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info))
return;
if (!crashpad_info.simple_annotations)
if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info) ||
!crashpad_info.simple_annotations) {
return;
}
std::vector<SimpleStringDictionary::Entry>
simple_annotations(SimpleStringDictionary::num_entries);
@ -122,11 +121,8 @@ template <class Traits>
void PEImageAnnotationsReader::ReadCrashpadAnnotationsList(
std::vector<AnnotationSnapshot>* vector_annotations) const {
process_types::CrashpadInfo<Traits> crashpad_info;
if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info)) {
return;
}
if (!crashpad_info.annotations_list) {
if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info) ||
!crashpad_info.annotations_list) {
return;
}

View File

@ -17,9 +17,11 @@
#include <stddef.h>
#include <string.h>
#include <algorithm>
#include <memory>
#include "base/logging.h"
#include "base/strings/stringprintf.h"
#include "client/crashpad_info.h"
#include "snapshot/win/pe_image_resource_reader.h"
#include "util/misc/from_pointer_cast.h"
@ -80,39 +82,58 @@ bool PEImageReader::GetCrashpadInfo(
return false;
}
if (section.Misc.VirtualSize < sizeof(process_types::CrashpadInfo<Traits>)) {
if (section.Misc.VirtualSize <
offsetof(process_types::CrashpadInfo<Traits>, size) +
sizeof(crashpad_info->size)) {
LOG(WARNING) << "small crashpad info section size "
<< section.Misc.VirtualSize << ", "
<< module_subrange_reader_.name();
return false;
}
ProcessSubrangeReader crashpad_info_subrange_reader;
const WinVMAddress crashpad_info_address = Address() + section.VirtualAddress;
if (!crashpad_info_subrange_reader.InitializeSubrange(
module_subrange_reader_,
crashpad_info_address,
section.Misc.VirtualSize,
"crashpad_info")) {
return false;
}
if (!crashpad_info_subrange_reader.ReadMemory(
crashpad_info_address,
sizeof(process_types::CrashpadInfo<Traits>),
crashpad_info)) {
const WinVMSize crashpad_info_size =
std::min(static_cast<WinVMSize>(sizeof(*crashpad_info)),
static_cast<WinVMSize>(section.Misc.VirtualSize));
if (!module_subrange_reader_.ReadMemory(
crashpad_info_address, crashpad_info_size, crashpad_info)) {
LOG(WARNING) << "could not read crashpad info from "
<< module_subrange_reader_.name();
return false;
}
if (crashpad_info->size < sizeof(*crashpad_info)) {
// Zero out anything beyond the structures declared size.
memset(reinterpret_cast<char*>(crashpad_info) + crashpad_info->size,
0,
sizeof(*crashpad_info) - crashpad_info->size);
}
if (crashpad_info->signature != CrashpadInfo::kSignature ||
crashpad_info->version < 1) {
LOG(WARNING) << "unexpected crashpad info data in "
<< module_subrange_reader_.name();
crashpad_info->version != 1) {
LOG(WARNING) << base::StringPrintf(
"unexpected crashpad info signature 0x%x, version %u in %s",
crashpad_info->signature,
crashpad_info->version,
module_subrange_reader_.name().c_str());
return false;
}
// Dont require strict equality, to leave wiggle room for sloppy linkers.
if (crashpad_info->size > section.Misc.VirtualSize) {
LOG(WARNING) << "crashpad info struct size " << crashpad_info->size
<< " large for section size " << section.Misc.VirtualSize
<< " in " << module_subrange_reader_.name();
return false;
}
if (crashpad_info->size > sizeof(*crashpad_info)) {
// This isnt strictly a problem, because unknown fields will simply be
// ignored, but it may be of diagnostic interest.
LOG(INFO) << "large crashpad info size " << crashpad_info->size << ", "
<< module_subrange_reader_.name();
}
return true;
}