Skip to content

Commit

Permalink
android/linux: add a client interface to control sanitization
Browse files Browse the repository at this point in the history 
Sanitization is controlled by a SanitizationInformation struct to be
read from the client's memory. The address of this struct is either
passed in a ClientInformation when the client requests a crash dump,
or as a flag to the handler --sanitization_information.

Bug: crashpad:30
Change-Id: I2744f8fb85b4fea7362b2b88faa4bef1da74e36b
Reviewed-on: https://chromium-review.googlesource.com/1083143
Commit-Queue: Joshua Peraza <[email protected]>
Reviewed-by: Scott Graham <[email protected]>
  • Loading branch information
Joshua Peraza authored and Commit Bot committed Jun 12, 2018
1 parent a42b526 commit d1e6a21
Show file tree
Hide file tree
Showing 18 changed files with 384 additions and 46 deletions.
47 changes: 36 additions & 11 deletions client/crashpad_client_linux_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "gtest/gtest.h"
#include "snapshot/annotation_snapshot.h"
#include "snapshot/minidump/process_snapshot_minidump.h"
#include "snapshot/sanitized/sanitization_information.h"
#include "test/multiprocess.h"
#include "test/multiprocess_exec.h"
#include "test/scoped_temp_dir.h"
Expand Down Expand Up @@ -210,7 +211,9 @@ class StartHandlerForClientTest {
StartHandlerForClientTest() = default;
~StartHandlerForClientTest() = default;

bool Initialize() {
bool Initialize(bool sanitize) {
sanitize_ = sanitize;

int socks[2];
if (socketpair(AF_UNIX, SOCK_STREAM, 0, socks) != 0) {
PLOG(ERROR) << "socketpair";
Expand Down Expand Up @@ -253,19 +256,23 @@ class StartHandlerForClientTest {
ASSERT_TRUE(database);

std::vector<CrashReportDatabase::Report> reports;
ASSERT_EQ(database->GetPendingReports(&reports),
ASSERT_EQ(database->GetCompletedReports(&reports),
CrashReportDatabase::kNoError);
EXPECT_EQ(reports.size(), 1u);
EXPECT_EQ(reports.size(), 0u);

reports.clear();
ASSERT_EQ(database->GetCompletedReports(&reports),
ASSERT_EQ(database->GetPendingReports(&reports),
CrashReportDatabase::kNoError);
EXPECT_EQ(reports.size(), 0u);
if (sanitize_) {
EXPECT_EQ(reports.size(), 0u);
} else {
EXPECT_EQ(reports.size(), 1u);
}
}

bool InstallHandler() {
auto signal_handler = SandboxedHandler::Get();
return signal_handler->Initialize(client_sock_.get());
return signal_handler->Initialize(client_sock_.get(), sanitize_);
}

private:
Expand All @@ -278,8 +285,9 @@ class StartHandlerForClientTest {
return instance;
}

bool Initialize(FileHandle client_sock) {
bool Initialize(FileHandle client_sock, bool sanitize) {
client_sock_ = client_sock;
sanitize_ = sanitize;
return Signals::InstallCrashHandlers(HandleCrash, 0, nullptr);
}

Expand All @@ -302,25 +310,36 @@ class StartHandlerForClientTest {
context);
exception_information.thread_id = syscall(SYS_gettid);

ClientInformation info = {};
ClientInformation info;
info.exception_information_address =
FromPointerCast<decltype(info.exception_information_address)>(
&exception_information);

SanitizationInformation sanitization_info = {};
if (state->sanitize_) {
info.sanitization_information_address =
FromPointerCast<VMAddress>(&sanitization_info);
// Target a non-module address to prevent a crash dump.
sanitization_info.target_module_address =
FromPointerCast<VMAddress>(&sanitization_info);
}

ExceptionHandlerClient handler_client(state->client_sock_);
CHECK_EQ(handler_client.RequestCrashDump(info), 0);

Signals::RestoreHandlerAndReraiseSignalOnReturn(siginfo, nullptr);
}

FileHandle client_sock_;
bool sanitize_;

DISALLOW_COPY_AND_ASSIGN(SandboxedHandler);
};

ScopedTempDir temp_dir_;
ScopedFileHandle client_sock_;
ScopedFileHandle server_sock_;
bool sanitize_;

DISALLOW_COPY_AND_ASSIGN(StartHandlerForClientTest);
};
Expand All @@ -331,9 +350,9 @@ class StartHandlerForChildTest : public Multiprocess {
StartHandlerForChildTest() = default;
~StartHandlerForChildTest() = default;

bool Initialize() {
bool Initialize(bool sanitize) {
SetExpectedChildTerminationBuiltinTrap();
return test_state_.Initialize();
return test_state_.Initialize(sanitize);
}

private:
Expand Down Expand Up @@ -361,7 +380,13 @@ class StartHandlerForChildTest : public Multiprocess {

TEST(CrashpadClient, StartHandlerForChild) {
StartHandlerForChildTest test;
ASSERT_TRUE(test.Initialize());
ASSERT_TRUE(test.Initialize(/* sanitize= */ false));
test.Run();
}

TEST(CrashpadClient, SanitizedChild) {
StartHandlerForChildTest test;
ASSERT_TRUE(test.Initialize(/* sanitize= */ true));
test.Run();
}

Expand Down
35 changes: 31 additions & 4 deletions handler/handler_main.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ void Usage(const base::FilePath& me) {
" --trace-parent-with-exception=EXCEPTION_INFORMATION_ADDRESS\n"
" request a dump for the handler's parent process\n"
" --initial-client-fd=FD a socket connected to a client.\n"
" --sanitization_information=SANITIZATION_INFORMATION_ADDRESS\n"
" the address of a SanitizationInformation struct.\n"
#endif // OS_LINUX || OS_ANDROID
" --url=URL send crash reports to this Breakpad server URL,\n"
" only if uploads are enabled for the database\n"
Expand All @@ -167,6 +169,7 @@ struct Options {
#elif defined(OS_LINUX) || defined(OS_ANDROID)
VMAddress exception_information_address;
int initial_client_fd;
VMAddress sanitization_information_address;
#elif defined(OS_WIN)
std::string pipe_name;
InitialClientData initial_client_data;
Expand Down Expand Up @@ -535,6 +538,7 @@ int HandlerMain(int argc,
#if defined(OS_LINUX) || defined(OS_ANDROID)
kOptionTraceParentWithException,
kOptionInitialClientFD,
kOptionSanitizationInformation,
#endif
kOptionURL,

Expand Down Expand Up @@ -590,6 +594,10 @@ int HandlerMain(int argc,
nullptr,
kOptionTraceParentWithException},
{"initial-client-fd", required_argument, nullptr, kOptionInitialClientFD},
{"sanitization-information",
required_argument,
nullptr,
kOptionSanitizationInformation},
#endif // OS_LINUX || OS_ANDROID
{"url", required_argument, nullptr, kOptionURL},
{"help", no_argument, nullptr, kOptionHelp},
Expand All @@ -608,6 +616,7 @@ int HandlerMain(int argc,
#if defined(OS_LINUX) || defined(OS_ANDROID)
options.exception_information_address = 0;
options.initial_client_fd = kInvalidFileHandle;
options.sanitization_information_address = 0;
#endif

int opt;
Expand Down Expand Up @@ -714,6 +723,15 @@ int HandlerMain(int argc,
}
break;
}
case kOptionSanitizationInformation: {
if (!StringToNumber(optarg,
&options.sanitization_information_address)) {
ToolSupport::UsageHint(me,
"failed to parse --sanitization-information");
return ExitFailure();
}
break;
}
#endif // OS_LINUX || OS_ANDROID
case kOptionURL: {
options.url = optarg;
Expand Down Expand Up @@ -762,9 +780,15 @@ int HandlerMain(int argc,
#elif defined(OS_LINUX) || defined(OS_ANDROID)
if (!options.exception_information_address &&
options.initial_client_fd == kInvalidFileHandle) {
ToolSupport::UsageHint(
me, "--trace-parent-with-exception or --initial_client_fd is required");
return ExitFailure();
}
if (options.sanitization_information_address &&
!options.exception_information_address) {
ToolSupport::UsageHint(
me,
"--exception_information_address or --initial_client_fd is required");
"--sanitization_information requires --trace-parent-with-exception");
return ExitFailure();
}
#endif // OS_MACOSX
Expand Down Expand Up @@ -844,9 +868,12 @@ int HandlerMain(int argc,

#if defined(OS_LINUX) || defined(OS_ANDROID)
if (options.exception_information_address) {
return exception_handler.HandleException(getppid(),
options.exception_information_address) ?
EXIT_SUCCESS : ExitFailure();
ClientInformation info;
info.exception_information_address = options.exception_information_address;
info.sanitization_information_address =
options.sanitization_information_address;
return exception_handler.HandleException(getppid(), info) ? EXIT_SUCCESS
: ExitFailure();
}
#endif // OS_LINUX || OS_ANDROID

Expand Down
61 changes: 52 additions & 9 deletions handler/linux/crash_report_exception_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include "minidump/minidump_file_writer.h"
#include "snapshot/crashpad_info_client_options.h"
#include "snapshot/linux/process_snapshot_linux.h"
#include "snapshot/sanitized/process_snapshot_sanitized.h"
#include "snapshot/sanitized/sanitization_information.h"
#include "util/linux/direct_ptrace_connection.h"
#include "util/linux/ptrace_client.h"
#include "util/misc/metrics.h"
Expand All @@ -43,7 +45,7 @@ CrashReportExceptionHandler::~CrashReportExceptionHandler() = default;

bool CrashReportExceptionHandler::HandleException(
pid_t client_process_id,
VMAddress exception_info_address) {
const ClientInformation& info) {
Metrics::ExceptionEncountered();

DirectPtraceConnection connection;
Expand All @@ -53,12 +55,12 @@ bool CrashReportExceptionHandler::HandleException(
return false;
}

return HandleExceptionWithConnection(&connection, exception_info_address);
return HandleExceptionWithConnection(&connection, info);
}

bool CrashReportExceptionHandler::HandleExceptionWithBroker(
pid_t client_process_id,
VMAddress exception_info_address,
const ClientInformation& info,
int broker_sock) {
Metrics::ExceptionEncountered();

Expand All @@ -69,19 +71,20 @@ bool CrashReportExceptionHandler::HandleExceptionWithBroker(
return false;
}

return HandleExceptionWithConnection(&client, exception_info_address);
return HandleExceptionWithConnection(&client, info);
}

bool CrashReportExceptionHandler::HandleExceptionWithConnection(
PtraceConnection* connection,
VMAddress exception_info_address) {
const ClientInformation& info) {
ProcessSnapshotLinux process_snapshot;
if (!process_snapshot.Initialize(connection)) {
Metrics::ExceptionCaptureResult(Metrics::CaptureResult::kSnapshotFailed);
return false;
}

if (!process_snapshot.InitializeException(exception_info_address)) {
if (!process_snapshot.InitializeException(
info.exception_information_address)) {
Metrics::ExceptionCaptureResult(
Metrics::CaptureResult::kExceptionInitializationFailed);
return false;
Expand Down Expand Up @@ -116,10 +119,50 @@ bool CrashReportExceptionHandler::HandleExceptionWithConnection(

process_snapshot.SetReportID(new_report->ReportID());

ProcessSnapshot* snapshot = nullptr;
ProcessSnapshotSanitized sanitized;
std::vector<std::string> whitelist;
if (info.sanitization_information_address) {
SanitizationInformation sanitization_info;
ProcessMemoryRange range;
if (!range.Initialize(connection->Memory(), connection->Is64Bit()) ||
!range.Read(info.sanitization_information_address,
sizeof(sanitization_info),
&sanitization_info)) {
Metrics::ExceptionCaptureResult(
Metrics::CaptureResult::kSanitizationInitializationFailed);
return false;
}

if (sanitization_info.annotations_whitelist_address &&
!ReadAnnotationsWhitelist(
range,
sanitization_info.annotations_whitelist_address,
&whitelist)) {
Metrics::ExceptionCaptureResult(
Metrics::CaptureResult::kSanitizationInitializationFailed);
return false;
}

if (!sanitized.Initialize(&process_snapshot,
sanitization_info.annotations_whitelist_address
? &whitelist
: nullptr,
sanitization_info.target_module_address,
sanitization_info.sanitize_stacks)) {
Metrics::ExceptionCaptureResult(
Metrics::CaptureResult::kSkippedDueToSanitization);
return true;
}

snapshot = &sanitized;
} else {
snapshot = &process_snapshot;
}

MinidumpFileWriter minidump;
minidump.InitializeFromSnapshot(&process_snapshot);
AddUserExtensionStreams(
user_stream_data_sources_, &process_snapshot, &minidump);
minidump.InitializeFromSnapshot(snapshot);
AddUserExtensionStreams(user_stream_data_sources_, snapshot, &minidump);

if (!minidump.WriteEverything(new_report->Writer())) {
LOG(ERROR) << "WriteEverything failed";
Expand Down
7 changes: 4 additions & 3 deletions handler/linux/crash_report_exception_handler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "handler/crash_report_upload_thread.h"
#include "handler/linux/exception_handler_server.h"
#include "handler/user_stream_data_source.h"
#include "util/linux/exception_handler_protocol.h"
#include "util/linux/ptrace_connection.h"
#include "util/misc/address_types.h"

Expand Down Expand Up @@ -63,15 +64,15 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate {
// ExceptionHandlerServer::Delegate:

bool HandleException(pid_t client_process_id,
VMAddress exception_info_address) override;
const ClientInformation& info) override;

bool HandleExceptionWithBroker(pid_t client_process_id,
VMAddress exception_info_address,
const ClientInformation& info,
int broker_sock) override;

private:
bool HandleExceptionWithConnection(PtraceConnection* connection,
VMAddress exception_info_address);
const ClientInformation& info);

CrashReportDatabase* database_; // weak
CrashReportUploadThread* upload_thread_; // weak
Expand Down
7 changes: 2 additions & 5 deletions handler/linux/exception_handler_server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,15 +446,12 @@ bool ExceptionHandlerServer::HandleCrashDumpRequest(
ServerToClientMessage::kTypeCrashDumpFailed);

case PtraceStrategyDecider::Strategy::kDirectPtrace:
delegate_->HandleException(client_process_id,
client_info.exception_information_address);
delegate_->HandleException(client_process_id, client_info);
break;

case PtraceStrategyDecider::Strategy::kUseBroker:
delegate_->HandleExceptionWithBroker(
client_process_id,
client_info.exception_information_address,
client_sock);
client_process_id, client_info, client_sock);
break;
}

Expand Down
Loading

0 comments on commit d1e6a21

Please sign in to comment.