[Crashpad/iOS] Harden CrashHandler against crashes during intermediate dump processing

https://crrev.com/c/3399252 fixed a heap overrun in iOS intermediate
dump processing.

This is a follow-up to that change to harden `CrashHandler` against
similar crashes:

1) Ensure the destructor of `ScopedAlternateWriter` is invoked
   to restore `InProcessHandler::writer_` state before processing
   the intermediate dump (otherwise, a signal raised by the intermediate
   dump handler would dereference the empty `std::unique_ptr` in
   `InProcessHandler::writer_`).

2) Harden `InProcessHandler` to check if `writer_` is empty before
   handling signals or exceptions

Change-Id: I1e63a496395b26681632302e8915b4433897037a
Bug: 391
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3401766
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
This commit is contained in:
Ben Hamilton 2022-01-20 10:04:00 -07:00 committed by Crashpad LUCI CQ
parent fd732953ce
commit 1cf99ea4d2
2 changed files with 31 additions and 6 deletions

View File

@ -106,13 +106,22 @@ class CrashHandler : public Thread,
void DumpWithoutCrash(NativeCPUContext* context, bool process_dump) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
internal::InProcessHandler::ScopedAlternateWriter scoper(
&in_process_handler_);
if (scoper.Open()) {
DumpWithContext(context);
if (process_dump) {
in_process_handler_.ProcessIntermediateDump(scoper.path());
base::FilePath path;
{
// Ensure ScopedAlternateWriter's destructor is invoked before processing
// the dump, or else any crashes handled during dump processing cannot be
// written.
internal::InProcessHandler::ScopedAlternateWriter scoper(
&in_process_handler_);
if (!scoper.Open()) {
LOG(ERROR) << "Could not open writer, ignoring dump request.";
return;
}
DumpWithContext(context);
path = scoper.path();
}
if (process_dump) {
in_process_handler_.ProcessIntermediateDump(path);
}
}

View File

@ -24,6 +24,7 @@
#include "minidump/minidump_file_writer.h"
#include "util/file/directory_reader.h"
#include "util/file/filesystem.h"
#include "util/ios/raw_logging.h"
namespace {
@ -116,6 +117,10 @@ void InProcessHandler::DumpExceptionFromSignal(
siginfo_t* siginfo,
ucontext_t* context) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
if (!writer_) {
CRASHPAD_RAW_LOG("Cannot DumpExceptionFromSignal without writer_");
return;
}
{
ScopedReport report(writer_.get(), system_data, annotations_);
InProcessIntermediateDumpHandler::WriteExceptionFromSignal(
@ -135,6 +140,10 @@ void InProcessHandler::DumpExceptionFromMachException(
ConstThreadState old_state,
mach_msg_type_number_t old_state_count) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
if (!writer_) {
CRASHPAD_RAW_LOG("Cannot DumpExceptionFromMachException without writer_");
return;
}
{
ScopedReport report(writer_.get(), system_data, annotations_);
InProcessIntermediateDumpHandler::WriteExceptionFromMachException(
@ -155,6 +164,12 @@ void InProcessHandler::DumpExceptionFromNSExceptionFrames(
const IOSSystemDataCollector& system_data,
const uint64_t* frames,
const size_t num_frames) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
if (!writer_) {
CRASHPAD_RAW_LOG(
"Cannot DumpExceptionFromNSExceptionFrames without writer_");
return;
}
{
ScopedReport report(
writer_.get(), system_data, annotations_, frames, num_frames);
@ -318,6 +333,7 @@ InProcessHandler::ScopedReport::ScopedReport(
frames_(frames),
num_frames_(num_frames),
rootMap_(writer) {
DCHECK(writer);
InProcessIntermediateDumpHandler::WriteHeader(writer);
InProcessIntermediateDumpHandler::WriteProcessInfo(writer, annotations);
InProcessIntermediateDumpHandler::WriteSystemInfo(writer, system_data);