From e17518a9e879f63b578db6c184c6bb17f1b13a06 Mon Sep 17 00:00:00 2001 From: Rich Mckeever Date: Thu, 26 Oct 2023 15:21:09 -0400 Subject: [PATCH] Add an option to start a Windows client with global hooks disabled. Change-Id: I645d6136788ca4ccebfc73005c8c2455dc4b2cee Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4949671 Reviewed-by: Mark Mentovai Commit-Queue: Rich Mckeever --- client/crashpad_client.h | 16 ++++ client/crashpad_client_win.cc | 7 +- client/crashpad_client_win_test.cc | 123 ++++++++++++++++++++++++++++- 3 files changed, 143 insertions(+), 3 deletions(-) diff --git a/client/crashpad_client.h b/client/crashpad_client.h index 3c966686..23a16ce7 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -486,6 +486,21 @@ class CrashpadClient { #endif // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_ANDROID) || // BUILDFLAG(IS_CHROMEOS) || DOXYGEN +#if BUILDFLAG(IS_WIN) || DOXYGEN + //! \brief Configures this client to not install any process-global hooks, + //! such as an unhandled exception filter or vectored exception handler. + //! + //! This may be useful if this client is being used in the context of an + //! extension library, which only wants to capture crashes in its own code, + //! via catch blocks, and not all crashes in the host process. + //! + //! This method must be called before calling StartHandler(), + //! SetHandlerSocket(), or other methods that install global hooks. + void DisableGlobalHooks() { + disable_global_hooks_ = true; + } +#endif // BUILDFLAG(IS_WIN) || DOXYGEN + #if BUILDFLAG(IS_IOS) || DOXYGEN //! \brief Observation callback invoked each time this object finishes //! processing and attempting to upload on-disk crash reports (whether or @@ -818,6 +833,7 @@ class CrashpadClient { std::wstring ipc_pipe_; ScopedKernelHANDLE handler_start_thread_; ScopedVectoredExceptionRegistration vectored_handler_; + bool disable_global_hooks_; #elif BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_ANDROID) std::set unhandled_signals_; #endif // BUILDFLAG(IS_APPLE) diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 2f02001d..4cbaf88e 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -592,7 +592,8 @@ void CommonInProcessInitialization() { } // namespace CrashpadClient::CrashpadClient() - : ipc_pipe_(), handler_start_thread_(), vectored_handler_() {} + : ipc_pipe_(), handler_start_thread_(), vectored_handler_(), + disable_global_hooks_(false) {} CrashpadClient::~CrashpadClient() {} @@ -667,6 +668,10 @@ bool CrashpadClient::StartHandler( } void CrashpadClient::RegisterHandlers() { + if (disable_global_hooks_) { + return; + } + SetUnhandledExceptionFilter(&UnhandledExceptionHandler); // Windows swallows heap corruption failures but we can intercept them with diff --git a/client/crashpad_client_win_test.cc b/client/crashpad_client_win_test.cc index 901cf81f..99c0e204 100644 --- a/client/crashpad_client_win_test.cc +++ b/client/crashpad_client_win_test.cc @@ -17,6 +17,7 @@ #include #include "base/files/file_path.h" +#include "client/crash_report_database.h" #include "gtest/gtest.h" #include "test/test_paths.h" #include "test/scoped_temp_dir.h" @@ -29,11 +30,10 @@ namespace crashpad { namespace test { namespace { -void StartAndUseHandler(const base::FilePath& temp_dir) { +void StartAndUseHandler(CrashpadClient& client, const base::FilePath& temp_dir) { base::FilePath handler_path = TestPaths::Executable().DirName().Append( FILE_PATH_LITERAL("crashpad_handler.com")); - CrashpadClient client; ASSERT_TRUE(client.StartHandler(handler_path, temp_dir, base::FilePath(), @@ -45,6 +45,11 @@ void StartAndUseHandler(const base::FilePath& temp_dir) { ASSERT_TRUE(client.WaitForHandlerStart(INFINITE)); } +void StartAndUseHandler(const base::FilePath& temp_dir) { + CrashpadClient client; + StartAndUseHandler(client, temp_dir); +} + class StartWithInvalidHandles final : public WinMultiprocessWithTempDir { public: StartWithInvalidHandles() : WinMultiprocessWithTempDir() {} @@ -192,6 +197,120 @@ TEST(CrashpadClient, HandlerLaunchFailureDumpWithoutCrash) { WinMultiprocess::Run(); } +class NoDumpExpected : public WinMultiprocessWithTempDir { + private: + void WinMultiprocessParentAfterChild(HANDLE child) override { + // Make sure no dump was generated. + std::unique_ptr database( + CrashReportDatabase::Initialize(GetTempDirPath())); + ASSERT_TRUE(database); + + std::vector reports; + ASSERT_EQ(database->GetPendingReports(&reports), + CrashReportDatabase::kNoError); + ASSERT_EQ(reports.size(), 0u); + } +}; + +// Crashing the process under test does not result in a crashed status as an +// exit code in debug builds, so we only verify this behavior in release +// builds. +#if defined(NDEBUG) +class CrashWithDisabledGlobalHooks final : public NoDumpExpected { + public: + CrashWithDisabledGlobalHooks() : NoDumpExpected() {} + ~CrashWithDisabledGlobalHooks() {} + + private: + void WinMultiprocessParent() override { + SetExpectedChildExitCode(STATUS_ACCESS_VIOLATION); + } + + void WinMultiprocessChild() override { + CrashpadClient client; + client.DisableGlobalHooks(); + StartAndUseHandler(client, GetTempDirPath()); + int* bad = nullptr; + *bad = 1; + } +}; + +TEST(CrashpadClient, CrashWithDisabledGlobalHooks) { + WinMultiprocessWithTempDir::Run(); +} +#endif // defined(NDEBUG) + +class DumpAndCrashWithDisabledGlobalHooks final + : public WinMultiprocessWithTempDir { + public: + DumpAndCrashWithDisabledGlobalHooks() : WinMultiprocessWithTempDir() {} + ~DumpAndCrashWithDisabledGlobalHooks() {} + + private: + static constexpr DWORD kExpectedExitCode = 0x1CEB00DA; + + void WinMultiprocessParent() override { + SetExpectedChildExitCode(kExpectedExitCode); + } + + void WinMultiprocessChild() override { + CrashpadClient client; + client.DisableGlobalHooks(); + StartAndUseHandler(client, GetTempDirPath()); + EXCEPTION_RECORD exception_record = {kExpectedExitCode, + EXCEPTION_NONCONTINUABLE}; + CONTEXT context; + CaptureContext(&context); + EXCEPTION_POINTERS exception_pointers = {&exception_record, &context}; + CrashpadClient::DumpAndCrash(&exception_pointers); + } + + void WinMultiprocessParentAfterChild(HANDLE child) override { + // Make sure the dump was generated. + std::unique_ptr database( + CrashReportDatabase::Initialize(GetTempDirPath())); + ASSERT_TRUE(database); + + std::vector reports; + ASSERT_EQ(database->GetPendingReports(&reports), + CrashReportDatabase::kNoError); + ASSERT_EQ(reports.size(), 1u); + + // Delegate the cleanup to the superclass. + WinMultiprocessWithTempDir::WinMultiprocessParentAfterChild(child); + } +}; + +TEST(CrashpadClient, DumpAndCrashWithDisabledGlobalHooks) { + WinMultiprocessWithTempDir::Run(); +} + +#if !defined(ADDRESS_SANITIZER) +class HeapCorruptionWithDisabledGlobalHooks final : public NoDumpExpected { + public: + HeapCorruptionWithDisabledGlobalHooks() : NoDumpExpected() {} + ~HeapCorruptionWithDisabledGlobalHooks() {} + + private: + void WinMultiprocessParent() override { + SetExpectedChildExitCode(STATUS_HEAP_CORRUPTION); + } + + void WinMultiprocessChild() override { + CrashpadClient client; + client.DisableGlobalHooks(); + StartAndUseHandler(client, GetTempDirPath()); + int* bad = reinterpret_cast(1); + delete bad; + } +}; + +TEST(CrashpadClient, HeapCorruptionWithDisabledGlobalHooks) { + WinMultiprocessWithTempDir::Run(); +} + +#endif // !defined(ADDRESS_SANITIZER) + } // namespace } // namespace test } // namespace crashpad