diff --git a/.gitignore b/.gitignore index c4f8fbb3..65bb4418 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ /out /third_party/gtest/gtest /third_party/gyp/gyp +/third_party/llvm /third_party/mini_chromium/mini_chromium /xcodebuild tags diff --git a/DEPS b/DEPS index 7a9be6d3..77428536 100644 --- a/DEPS +++ b/DEPS @@ -17,18 +17,28 @@ vars = { } deps = { + 'buildtools': + Var('chromium_git') + '/chromium/buildtools.git@' + + 'f8fc76ea5ce4a60cda2fa5d7df3d4a62935b3113', 'crashpad/third_party/gtest/gtest': Var('chromium_git') + '/external/github.com/google/googletest@' + 'ec44c6c1675c25b9827aacd08c02433cccde7780', 'crashpad/third_party/gyp/gyp': Var('chromium_git') + '/external/gyp@' + '93cc6e2c23e4d5ebd179f388e67aa907d0dfd43d', + + # TODO(scottmg): Consider pinning these. For now, we don't have any particular + # reason to do so. + 'crashpad/third_party/llvm': + Var('chromium_git') + '/external/llvm.org/llvm.git@HEAD', + 'crashpad/third_party/llvm/tools/clang': + Var('chromium_git') + '/external/llvm.org/clang.git@HEAD', + 'crashpad/third_party/llvm/tools/lldb': + Var('chromium_git') + '/external/llvm.org/lldb.git@HEAD', + 'crashpad/third_party/mini_chromium/mini_chromium': Var('chromium_git') + '/chromium/mini_chromium@' + - '414d59602ac38e24f1e93929fda3d79d72cea139', - 'buildtools': - Var('chromium_git') + '/chromium/buildtools.git@' + - 'f8fc76ea5ce4a60cda2fa5d7df3d4a62935b3113', + 'de1afb04f4afc074ec6d23bd9ee7b1e6b365427f', } hooks = [ @@ -41,7 +51,6 @@ hooks = [ '--no_resume', '--no_auth', '--bucket=chromium-clang-format', - '--output=buildtools/mac/clang-format', '--sha1_file', 'buildtools/mac/clang-format.sha1', ], @@ -55,11 +64,36 @@ hooks = [ '--no_resume', '--no_auth', '--bucket=chromium-clang-format', - '--output=buildtools/win/clang-format.exe', '--sha1_file', 'buildtools/win/clang-format.exe.sha1', ], }, + { + 'name': 'gn_mac', + 'pattern': '.', + 'action': [ + 'download_from_google_storage', + '--platform=^darwin$', + '--no_resume', + '--no_auth', + '--bucket=chromium-gn', + '--sha1_file', + 'buildtools/mac/gn.sha1', + ], + }, + { + 'name': 'gn_win', + 'pattern': '.', + 'action': [ + 'download_from_google_storage', + '--platform=^win32$', + '--no_resume', + '--no_auth', + '--bucket=chromium-gn', + '--sha1_file', + 'buildtools/win/gn.exe.sha1', + ], + }, { 'name': 'gyp', 'pattern': '\.gypi?$', diff --git a/client/capture_context_mac_test.cc b/client/capture_context_mac_test.cc index 436ac5ad..1904b2b6 100644 --- a/client/capture_context_mac_test.cc +++ b/client/capture_context_mac_test.cc @@ -21,6 +21,7 @@ #include "build/build_config.h" #include "gtest/gtest.h" +#include "util/misc/address_sanitizer.h" #include "util/misc/implicit_cast.h" namespace crashpad { @@ -103,13 +104,14 @@ void TestCaptureContext() { // captured program counter should be slightly greater than or equal to the // reference program counter. uintptr_t pc = ProgramCounterFromContext(context_1); -#if !__has_feature(address_sanitizer) + +#if !defined(ADDRESS_SANITIZER) // AddressSanitizer can cause enough code bloat that the “nearby” check would // likely fail. const uintptr_t kReferencePC = reinterpret_cast(TestCaptureContext); EXPECT_LT(pc - kReferencePC, 64u); -#endif +#endif // !defined(ADDRESS_SANITIZER) // Declare sp and context_2 here because all local variables need to be // declared before computing the stack pointer reference value, so that the diff --git a/client/crashpad_client.h b/client/crashpad_client.h index 844512f1..f6f3395c 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -173,9 +173,12 @@ class CrashpadClient { //! //! This method should not be used unless `asynchronous_start` was `true`. //! + //! \param[in] timeout_ms The number of milliseconds to wait for a result from + //! the background launch, or `0xffffffff` to block indefinitely. + //! //! \return `true` if the hander startup succeeded, `false` otherwise, and an //! error message will have been logged. - bool WaitForHandlerStart(); + bool WaitForHandlerStart(unsigned int timeout_ms); //! \brief Requests that the handler capture a dump even though there hasn't //! been a crash. diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 235ef185..7a3d0047 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -15,6 +15,7 @@ #include "client/crashpad_client.h" #include +#include #include #include @@ -31,6 +32,7 @@ #include "util/file/file_io.h" #include "util/misc/random_string.h" #include "util/win/address_types.h" +#include "util/win/capture_context.h" #include "util/win/command_line.h" #include "util/win/critical_section_with_debug_info.h" #include "util/win/get_function.h" @@ -107,7 +109,18 @@ StartupState BlockUntilHandlerStartedOrFailed() { return static_cast(startup_state); } +#if defined(ADDRESS_SANITIZER) +extern "C" LONG __asan_unhandled_exception_filter(EXCEPTION_POINTERS* info); +#endif + LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { +#if defined(ADDRESS_SANITIZER) + // In ASan builds, delegate to the ASan exception filter. + LONG status = __asan_unhandled_exception_filter(exception_pointers); + if (status != EXCEPTION_CONTINUE_SEARCH) + return status; +#endif + if (BlockUntilHandlerStartedOrFailed() == StartupState::kFailed) { // If we know for certain that the handler has failed to start, then abort // here, rather than trying to signal to a handler that will never arrive, @@ -163,6 +176,28 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { return EXCEPTION_CONTINUE_SEARCH; } +void HandleAbortSignal(int signum) { + DCHECK_EQ(signum, SIGABRT); + + CONTEXT context; + CaptureContext(&context); + + EXCEPTION_RECORD record = {}; + record.ExceptionCode = STATUS_FATAL_APP_EXIT; + record.ExceptionFlags = EXCEPTION_NONCONTINUABLE; +#if defined(ARCH_CPU_64_BITS) + record.ExceptionAddress = reinterpret_cast(context.Rip); +#else + record.ExceptionAddress = reinterpret_cast(context.Eip); +#endif // ARCH_CPU_64_BITS + + EXCEPTION_POINTERS exception_pointers; + exception_pointers.ContextRecord = &context; + exception_pointers.ExceptionRecord = &record; + + UnhandledExceptionHandler(&exception_pointers); +} + std::wstring FormatArgumentString(const std::string& name, const std::wstring& value) { return std::wstring(L"--") + base::UTF8ToUTF16(name) + L"=" + value; @@ -500,6 +535,32 @@ void CommonInProcessInitialization() { g_non_crash_dump_lock = new base::Lock(); } +void RegisterHandlers() { + SetUnhandledExceptionFilter(&UnhandledExceptionHandler); + + // The Windows CRT's signal.h lists: + // - SIGINT + // - SIGILL + // - SIGFPE + // - SIGSEGV + // - SIGTERM + // - SIGBREAK + // - SIGABRT + // SIGILL and SIGTERM are documented as not being generated. SIGBREAK and + // SIGINT are for Ctrl-Break and Ctrl-C, and aren't something for which + // capturing a dump is warranted. SIGFPE and SIGSEGV are captured as regular + // exceptions through the unhandled exception filter. This leaves SIGABRT. In + // the standard CRT, abort() is implemented as a synchronous call to the + // SIGABRT signal handler if installed, but after doing so, the unhandled + // exception filter is not triggered (it instead __fastfail()s). So, register + // to handle SIGABRT to catch abort() calls, as client code might use this and + // expect it to cause a crash dump. This will only work when the abort() + // that's called in client code is the same (or has the same behavior) as the + // one in use here. + void (*rv)(int) = signal(SIGABRT, HandleAbortSignal); + DCHECK_NE(rv, SIG_ERR); +} + } // namespace CrashpadClient::CrashpadClient() : ipc_pipe_(), handler_start_thread_() {} @@ -536,7 +597,7 @@ bool CrashpadClient::StartHandler( CommonInProcessInitialization(); - SetUnhandledExceptionFilter(&UnhandledExceptionHandler); + RegisterHandlers(); auto data = new BackgroundHandlerStartThreadData(handler, database, @@ -609,7 +670,8 @@ bool CrashpadClient::SetHandlerIPCPipe(const std::wstring& ipc_pipe) { } SetHandlerStartupState(StartupState::kSucceeded); - SetUnhandledExceptionFilter(&UnhandledExceptionHandler); + + RegisterHandlers(); // The server returns these already duplicated to be valid in this process. g_signal_exception = @@ -627,10 +689,16 @@ std::wstring CrashpadClient::GetHandlerIPCPipe() const { return ipc_pipe_; } -bool CrashpadClient::WaitForHandlerStart() { +bool CrashpadClient::WaitForHandlerStart(unsigned int timeout_ms) { DCHECK(handler_start_thread_.is_valid()); - if (WaitForSingleObject(handler_start_thread_.get(), INFINITE) != - WAIT_OBJECT_0) { + DWORD result = WaitForSingleObject(handler_start_thread_.get(), timeout_ms); + if (result == WAIT_TIMEOUT) { + LOG(ERROR) << "WaitForSingleObject timed out"; + return false; + } else if (result == WAIT_ABANDONED) { + LOG(ERROR) << "WaitForSingleObject abandoned"; + return false; + } else if (result != WAIT_OBJECT_0) { PLOG(ERROR) << "WaitForSingleObject"; return false; } diff --git a/client/crashpad_client_win_test.cc b/client/crashpad_client_win_test.cc index 94a9b0cc..47e7add9 100644 --- a/client/crashpad_client_win_test.cc +++ b/client/crashpad_client_win_test.cc @@ -39,7 +39,7 @@ void StartAndUseHandler() { std::vector(), true, true)); - ASSERT_TRUE(client.WaitForHandlerStart()); + ASSERT_TRUE(client.WaitForHandlerStart(INFINITE)); } class StartWithInvalidHandles final : public WinMultiprocess { diff --git a/client/crashpad_info.cc b/client/crashpad_info.cc index e8a6a9ee..e517f7b1 100644 --- a/client/crashpad_info.cc +++ b/client/crashpad_info.cc @@ -14,6 +14,7 @@ #include "client/crashpad_info.h" +#include "util/misc/address_sanitizer.h" #include "util/stdlib/cxx.h" #if defined(OS_MACOSX) @@ -72,14 +73,14 @@ __attribute__(( #error Port #endif // !defined(OS_MACOSX) && !defined(OS_LINUX) && !defined(OS_ANDROID) -#if __has_feature(address_sanitizer) +#if defined(ADDRESS_SANITIZER) // AddressSanitizer would add a trailing red zone of at least 32 bytes, // which would be reflected in the size of the custom section. This confuses // MachOImageReader::GetCrashpadInfo(), which finds that the section’s size // disagrees with the structure’s size_ field. By specifying an alignment // greater than the red zone size, the red zone will be suppressed. aligned(64), -#endif // __has_feature(address_sanitizer) +#endif // defined(ADDRESS_SANITIZER) // The “used” attribute prevents the structure from being dead-stripped. used, diff --git a/client/settings.cc b/client/settings.cc index d018e37f..7757ecb0 100644 --- a/client/settings.cc +++ b/client/settings.cc @@ -55,7 +55,7 @@ struct Settings::Data { uint32_t version; uint32_t options; uint32_t padding_0; - uint64_t last_upload_attempt_time; // time_t + int64_t last_upload_attempt_time; // time_t UUID client_id; }; @@ -136,7 +136,7 @@ bool Settings::SetLastUploadAttemptTime(time_t time) { if (!handle.is_valid()) return false; - settings.last_upload_attempt_time = InRangeCast(time, 0); + settings.last_upload_attempt_time = InRangeCast(time, 0); return WriteSettings(handle.get(), settings); } diff --git a/handler/handler.gyp b/handler/handler.gyp index d6900531..e2d993df 100644 --- a/handler/handler.gyp +++ b/handler/handler.gyp @@ -147,6 +147,20 @@ 'win/crashy_test_program.cc', ], }, + { + 'target_name': 'crashy_signal', + 'type': 'executable', + 'dependencies': [ + '../client/client.gyp:crashpad_client', + '../third_party/mini_chromium/mini_chromium.gyp:base', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'win/crashy_signal.cc', + ], + }, { 'target_name': 'crash_other_program', 'type': 'executable', diff --git a/handler/handler_main.cc b/handler/handler_main.cc index 29c5ddc1..3ada8c3e 100644 --- a/handler/handler_main.cc +++ b/handler/handler_main.cc @@ -129,9 +129,15 @@ void HandleSIGTERM(int sig, siginfo_t* siginfo, void* context) { #endif // OS_MACOSX #if defined(OS_WIN) +LONG(WINAPI* g_original_exception_filter)(EXCEPTION_POINTERS*) = nullptr; + LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { Metrics::HandlerCrashed(exception_pointers->ExceptionRecord->ExceptionCode); - return EXCEPTION_CONTINUE_SEARCH; + + if (g_original_exception_filter) + return g_original_exception_filter(exception_pointers); + else + return EXCEPTION_CONTINUE_SEARCH; } #endif // OS_WIN @@ -139,7 +145,8 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { int HandlerMain(int argc, char* argv[]) { #if defined(OS_WIN) - SetUnhandledExceptionFilter(&UnhandledExceptionHandler); + g_original_exception_filter = + SetUnhandledExceptionFilter(&UnhandledExceptionHandler); #endif const base::FilePath argv0( diff --git a/handler/mac/crash_report_exception_handler.cc b/handler/mac/crash_report_exception_handler.cc index 2a115623..7784afef 100644 --- a/handler/mac/crash_report_exception_handler.cc +++ b/handler/mac/crash_report_exception_handler.cc @@ -66,7 +66,7 @@ kern_return_t CrashReportExceptionHandler::CatchMachException( const mach_msg_trailer_t* trailer, bool* destroy_complex_request) { Metrics::ExceptionEncountered(); - Metrics::ExceptionCode(exception); + Metrics::ExceptionCode(ExceptionCodeForMetrics(exception, code[0])); *destroy_complex_request = true; // The expected behavior is EXCEPTION_STATE_IDENTITY | MACH_EXCEPTION_CODES, diff --git a/handler/win/crashy_signal.cc b/handler/win/crashy_signal.cc new file mode 100644 index 00000000..8caa6259 --- /dev/null +++ b/handler/win/crashy_signal.cc @@ -0,0 +1,90 @@ +// Copyright 2016 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 +#include +#include +#include + +#include "base/logging.h" +#include "client/crashpad_client.h" + +namespace crashpad { +namespace { + +enum WhereToSignalFrom { + kUnknown = -1, + kMain = 0, + kBackground = 1, +}; + +WhereToSignalFrom MainOrBackground(wchar_t* name) { + if (wcscmp(name, L"main") == 0) + return kMain; + if (wcscmp(name, L"background") == 0) + return kBackground; + return kUnknown; +} + +DWORD WINAPI BackgroundThread(void* arg) { + abort(); + return 0; +} + +int CrashySignalMain(int argc, wchar_t* argv[]) { + CrashpadClient client; + + WhereToSignalFrom from; + if (argc == 3 && (from = MainOrBackground(argv[2])) != kUnknown) { + if (!client.SetHandlerIPCPipe(argv[1])) { + LOG(ERROR) << "SetHandler"; + return EXIT_FAILURE; + } + } else { + fprintf(stderr, "Usage: %ls main|background\n", argv[0]); + return EXIT_FAILURE; + } + + // In debug builds part of abort() is to open a dialog. We don't want tests to + // block at that dialog, so disable it. + _set_abort_behavior(0, _WRITE_ABORT_MSG); + + if (from == kBackground) { + HANDLE thread = CreateThread(nullptr, + 0, + &BackgroundThread, + nullptr, + 0, + nullptr); + if (!thread) { + PLOG(ERROR) << "CreateThread"; + return EXIT_FAILURE; + } + if (WaitForSingleObject(thread, INFINITE) != WAIT_OBJECT_0) { + PLOG(ERROR) << "WaitForSingleObject"; + return EXIT_FAILURE; + } + } else { + abort(); + } + + return EXIT_SUCCESS; +} + +} // namespace +} // namespace crashpad + +int wmain(int argc, wchar_t* argv[]) { + return crashpad::CrashySignalMain(argc, argv); +} diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc index b6e044c8..58afa47f 100644 --- a/handler/win/crashy_test_program.cc +++ b/handler/win/crashy_test_program.cc @@ -201,8 +201,8 @@ int CrashyMain(int argc, wchar_t* argv[]) { AllocateExtraUnsavedMemory(extra_ranges); // Load and unload some uncommonly used modules so we can see them in the list - // reported by `lm`. At least two so that we confirm we got the size of - // RTL_UNLOAD_EVENT_TRACE right. + // reported by `lm`. At least two so that we confirm we got the element size + // advancement of RTL_UNLOAD_EVENT_TRACE correct. CHECK(GetModuleHandle(L"lz32.dll") == nullptr); CHECK(GetModuleHandle(L"wmerror.dll") == nullptr); HMODULE lz32 = LoadLibrary(L"lz32.dll"); diff --git a/snapshot/mac/exception_snapshot_mac.cc b/snapshot/mac/exception_snapshot_mac.cc index c5d32b5b..92c8450b 100644 --- a/snapshot/mac/exception_snapshot_mac.cc +++ b/snapshot/mac/exception_snapshot_mac.cc @@ -66,20 +66,7 @@ bool ExceptionSnapshotMac::Initialize(ProcessReader* process_reader, exception_ = ExcCrashRecoverOriginalException( exception_code_0, &exception_code_0, nullptr); - if (exception_ == EXC_CRASH || - exception_ == EXC_RESOURCE || - exception_ == EXC_GUARD) { - // EXC_CRASH should never be wrapped in another EXC_CRASH. - // - // EXC_RESOURCE and EXC_GUARD are software exceptions that are never - // wrapped in EXC_CRASH. The only time EXC_CRASH is generated is for - // processes exiting due to an unhandled core-generating signal or being - // killed by SIGKILL for code-signing reasons. Neither of these applies to - // EXC_RESOURCE or EXC_GUARD. See 10.10 xnu-2782.1.97/bsd/kern/kern_exit.c - // proc_prepareexit(). Receiving these exception types wrapped in - // EXC_CRASH would lose information because their code[0] uses all 64 bits - // (see below) and the code[0] recovered from EXC_CRASH only contains 20 - // significant bits. + if (!ExcCrashCouldContainException(exception_)) { LOG(WARNING) << base::StringPrintf( "exception %s invalid in EXC_CRASH", ExceptionToString(exception_, kUseFullName | kUnknownIsNumeric) diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index f469bcf4..42999df8 100755 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -143,6 +143,10 @@ def GetDumpFromOtherProgram(out_dir, pipe_name, *args): *args) +def GetDumpFromSignal(out_dir, pipe_name, *args): + return GetDumpFromProgram(out_dir, pipe_name, 'crashy_signal.exe', *args) + + def GetDumpFromSelfDestroyingProgram(out_dir, pipe_name): return GetDumpFromProgram(out_dir, pipe_name, 'self_destroying_program.exe') @@ -201,6 +205,8 @@ def RunTests(cdb_path, z7_dump_path, other_program_path, other_program_no_exception_path, + sigabrt_main_path, + sigabrt_background_path, pipe_name): """Runs various tests in sequence. Runs a new cdb instance on the dump for each block of tests to reduce the chances that output from one command is @@ -361,6 +367,13 @@ def RunTests(cdb_path, 'other program with no exception given') out.Check('!RaiseException', 'other program in RaiseException()') + out = CdbRun(cdb_path, sigabrt_main_path, '.ecxr') + out.Check('code 40000015', 'got sigabrt signal') + out.Check('::HandleAbortSignal', ' stack in expected location') + + out = CdbRun(cdb_path, sigabrt_background_path, '.ecxr') + out.Check('code 40000015', 'got sigabrt signal from background thread') + def main(args): try: @@ -411,6 +424,15 @@ def main(args): if not other_program_no_exception_path: return 1 + sigabrt_main_path = GetDumpFromSignal(args[0], pipe_name, 'main') + if not sigabrt_main_path: + return 1 + + sigabrt_background_path = GetDumpFromSignal( + args[0], pipe_name, 'background') + if not sigabrt_background_path: + return 1 + RunTests(cdb_path, crashy_dump_path, start_handler_dump_path, @@ -418,6 +440,8 @@ def main(args): z7_dump_path, other_program_path, other_program_no_exception_path, + sigabrt_main_path, + sigabrt_background_path, pipe_name) return 1 if g_had_failures else 0 diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc index 63543c76..11e83145 100644 --- a/snapshot/win/process_snapshot_win.cc +++ b/snapshot/win/process_snapshot_win.cc @@ -281,21 +281,43 @@ void ProcessSnapshotWin::InitializeUnloadedModules() { #error port #endif - RTL_UNLOAD_EVENT_TRACE* unload_event_trace_address = - RtlGetUnloadEventTrace(); - WinVMAddress address_in_target_process = - reinterpret_cast(unload_event_trace_address); + ULONG* element_size; + ULONG* element_count; + void* event_trace_address; + RtlGetUnloadEventTraceEx(&element_size, &element_count, &event_trace_address); - std::vector> events( - RTL_UNLOAD_EVENT_TRACE_NUMBER); - if (!process_reader_.ReadMemory(address_in_target_process, - events.size() * sizeof(events[0]), - &events[0])) { + if (*element_size < sizeof(RTL_UNLOAD_EVENT_TRACE)) { + LOG(ERROR) << "unexpected unloaded module list element size"; return; } - for (const RTL_UNLOAD_EVENT_TRACE& uet : events) { - if (uet.ImageName[0]) { + const WinVMAddress address_in_target_process = + reinterpret_cast(event_trace_address); + + Traits::Pointer pointer_to_array; + if (!process_reader_.ReadMemory(address_in_target_process, + sizeof(pointer_to_array), + &pointer_to_array)) { + LOG(ERROR) << "failed to read target address"; + return; + } + + // No unloaded modules. + if (pointer_to_array == 0) + return; + + const size_t data_size = *element_size * *element_count; + std::vector data(data_size); + if (!process_reader_.ReadMemory(pointer_to_array, data_size, &data[0])) { + LOG(ERROR) << "failed to read unloaded module data"; + return; + } + + for (ULONG i = 0; i < *element_count; ++i) { + const uint8_t* base_address = &data[i * *element_size]; + const auto& uet = + *reinterpret_cast*>(base_address); + if (uet.ImageName[0] != 0) { unloaded_modules_.push_back(UnloadedModuleSnapshot( uet.BaseAddress, uet.SizeOfImage, diff --git a/tools/crashpad_database_util.cc b/tools/crashpad_database_util.cc index acbadd02..e7d89bc4 100644 --- a/tools/crashpad_database_util.cc +++ b/tools/crashpad_database_util.cc @@ -38,6 +38,7 @@ #include "util/file/file_io.h" #include "util/file/file_reader.h" #include "util/misc/uuid.h" +#include "util/stdlib/string_number_conversion.h" namespace crashpad { namespace { @@ -130,16 +131,23 @@ std::string BoolToString(bool boolean) { return std::string(boolean ? "true" : "false"); } -// Converts |string| to |time|, returning true if a conversion could be +// Converts |string| to |out_time|, returning true if a conversion could be // performed, and false without setting |boolean| if no conversion could be // performed. Various time formats are recognized, including several string -// representations and a numeric time_t representation. The special string -// "never" is recognized as |string| and converts to a |time| value of 0. |utc|, -// when true, causes |string| to be interpreted as a UTC time rather than a -// local time when the time zone is ambiguous. -bool StringToTime(const char* string, time_t* time, bool utc) { +// representations and a numeric time_t representation. The special |string| +// "never" is recognized as converted to a |out_time| value of 0; "now" is +// converted to the current time. |utc|, when true, causes |string| to be +// interpreted as a UTC time rather than a local time when the time zone is +// ambiguous. +bool StringToTime(const char* string, time_t* out_time, bool utc) { if (strcasecmp(string, "never") == 0) { - *time = 0; + *out_time = 0; + return true; + } + + if (strcasecmp(string, "now") == 0) { + errno = 0; + PCHECK(time(out_time) != -1 || errno == 0); return true; } @@ -155,41 +163,53 @@ bool StringToTime(const char* string, time_t* time, bool utc) { tm time_tm; const char* strptime_result = strptime(string, kFormats[index], &time_tm); if (strptime_result == end) { + time_t test_out_time; if (utc) { - *time = timegm(&time_tm); + test_out_time = timegm(&time_tm); } else { - *time = mktime(&time_tm); + test_out_time = mktime(&time_tm); } - return true; + // mktime() is supposed to set errno in the event of an error, but support + // for this is spotty, so there’s no way to distinguish between a true + // time_t of -1 (1969-12-31 23:59:59 UTC) and an error. Assume error. + // + // See 10.11.5 Libc-1082.50.1/stdtime/FreeBSD/localtime.c and + // glibc-2.24/time/mktime.c, which don’t set errno or save and restore + // errno. Post-Android 7.1.0 Bionic is even more hopeless, setting errno + // whenever the time conversion returns -1, even for valid input. See + // libc/tzcode/localtime.c mktime(). Windows seems to get it right: see + // 10.0.14393 SDK Source/ucrt/time/mktime.cpp. + if (test_out_time != -1) { + *out_time = test_out_time; + return true; + } } } - char* end_result; - errno = 0; - long long strtoll_result = strtoll(string, &end_result, 0); - if (end_result == end && errno == 0 && - base::IsValueInRangeForNumericType(strtoll_result)) { - *time = strtoll_result; + int64_t int64_result; + if (StringToNumber(string, &int64_result) && + base::IsValueInRangeForNumericType(int64_result)) { + *out_time = int64_result; return true; } return false; } -// Converts |time_tt| to a string, and returns it. |utc| determines whether the -// converted time will reference local time or UTC. If |time_tt| is 0, the +// Converts |out_time| to a string, and returns it. |utc| determines whether the +// converted time will reference local time or UTC. If |out_time| is 0, the // string "never" will be returned as a special case. -std::string TimeToString(time_t time_tt, bool utc) { - if (time_tt == 0) { +std::string TimeToString(time_t out_time, bool utc) { + if (out_time == 0) { return std::string("never"); } tm time_tm; if (utc) { - gmtime_r(&time_tt, &time_tm); + PCHECK(gmtime_r(&out_time, &time_tm)); } else { - localtime_r(&time_tt, &time_tm); + PCHECK(localtime_r(&out_time, &time_tm)); } char string[64]; diff --git a/tools/crashpad_database_util.md b/tools/crashpad_database_util.md index 610731dd..a63d9a68 100644 --- a/tools/crashpad_database_util.md +++ b/tools/crashpad_database_util.md @@ -118,7 +118,8 @@ database. Set the last-upload-attempt time in the database’s settings. _TIME_ is a string representation of a time, which may be in _yyyy-mm-dd hh:mm:ss_ - format, a numeric `time_t` value, or the special string `"never"`. + format, a numeric `time_t` value, or the special strings `"never"` or + `"now"`. See also **--show-last-upload-attempt-time**. diff --git a/util/file/string_file.cc b/util/file/string_file.cc index 1c74b3f0..959b0a53 100644 --- a/util/file/string_file.cc +++ b/util/file/string_file.cc @@ -157,17 +157,16 @@ FileOffset StringFile::Seek(FileOffset offset, int whence) { LOG(ERROR) << "Seek(): new_offset invalid"; return -1; } - FileOffset new_offset_fileoffset = new_offset.ValueOrDie(); size_t new_offset_sizet; - if (!AssignIfInRange(&new_offset_sizet, new_offset_fileoffset)) { - LOG(ERROR) << "Seek(): new_offset " << new_offset_fileoffset + if (!new_offset.AssignIfValid(&new_offset_sizet)) { + LOG(ERROR) << "Seek(): new_offset " << new_offset.ValueOrDie() << " invalid for size_t"; return -1; } offset_ = new_offset_sizet; - return offset_.ValueOrDie(); + return base::ValueOrDieForType(offset_); } } // namespace crashpad diff --git a/util/mach/exception_types.cc b/util/mach/exception_types.cc index 09366b57..85bdf770 100644 --- a/util/mach/exception_types.cc +++ b/util/mach/exception_types.cc @@ -24,6 +24,8 @@ #include "base/logging.h" #include "base/mac/mach_logging.h" #include "util/mac/mac_util.h" +#include "util/mach/mach_extensions.h" +#include "util/numeric/in_range_cast.h" #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 @@ -128,6 +130,114 @@ exception_type_t ExcCrashRecoverOriginalException( return (code_0 >> 20) & 0xf; } +bool ExcCrashCouldContainException(exception_type_t exception) { + // EXC_CRASH should never be wrapped in another EXC_CRASH. + // + // EXC_RESOURCE and EXC_GUARD are software exceptions that are never wrapped + // in EXC_CRASH. The only time EXC_CRASH is generated is for processes exiting + // due to an unhandled core-generating signal or being killed by SIGKILL for + // code-signing reasons. Neither of these apply to EXC_RESOURCE or EXC_GUARD. + // See 10.10 xnu-2782.1.97/bsd/kern/kern_exit.c proc_prepareexit(). Receiving + // these exception types wrapped in EXC_CRASH would lose information because + // their code[0] uses all 64 bits (see ExceptionSnapshotMac::Initialize()) and + // the code[0] recovered from EXC_CRASH only contains 20 significant bits. + // + // EXC_CORPSE_NOTIFY may be generated from EXC_CRASH, but the opposite should + // never occur. + // + // kMachExceptionSimulated is a non-fatal Crashpad-specific pseudo-exception + // that never exists as an exception within the kernel and should thus never + // be wrapped in EXC_CRASH. + return exception != EXC_CRASH && + exception != EXC_RESOURCE && + exception != EXC_GUARD && + exception != EXC_CORPSE_NOTIFY && + exception != kMachExceptionSimulated; +} + +int32_t ExceptionCodeForMetrics(exception_type_t exception, + mach_exception_code_t code_0) { + if (exception == kMachExceptionSimulated) { + return exception; + } + + int signal = 0; + if (exception == EXC_CRASH) { + const exception_type_t original_exception = + ExcCrashRecoverOriginalException(code_0, &code_0, &signal); + if (!ExcCrashCouldContainException(original_exception)) { + LOG(WARNING) << "EXC_CRASH should not contain exception " + << original_exception; + return InRangeCast(original_exception, 0xffff) << 16; + } + exception = original_exception; + } + + uint16_t metrics_exception = InRangeCast(exception, 0xffff); + + uint16_t metrics_code_0; + switch (exception) { + case EXC_RESOURCE: + metrics_code_0 = (EXC_RESOURCE_DECODE_RESOURCE_TYPE(code_0) << 8) | + EXC_RESOURCE_DECODE_FLAVOR(code_0); + break; + + case EXC_GUARD: { + // This will be GUARD_TYPE_MACH_PORT (1) from or + // GUARD_TYPE_FD (2) from 10.12.2 xnu-3789.31.2/bsd/sys/guarded.h + const uint8_t guard_type = (code_0) >> 61; + + // These exceptions come through 10.12.2 + // xnu-3789.31.2/osfmk/ipc/mach_port.c mach_port_guard_exception() or + // xnu-3789.31.2/bsd/kern/kern_guarded.c fp_guard_exception(). In each + // case, bits 32-60 of code_0 encode the guard type-specific “flavor”. For + // Mach port guards, these flavor codes come from the + // mach_port_guard_exception_codes enum in . For file + // descriptor guards, they come from the guard_exception_codes enum in + // xnu-3789.31.2/bsd/sys/guarded.h. Both of these enums define shifted-bit + // values (1 << 0, 1 << 1, 1 << 2, etc.) In actual usage as determined by + // callers to these functions, these “flavor” codes are never ORed with + // one another. For the purposes of encoding these codes for metrics, + // convert the flavor codes to their corresponding bit shift values. + const uint32_t guard_flavor = (code_0 >> 32) & 0x1fffffff; + uint8_t metrics_guard_flavor = 0xff; + for (int bit = 0; bit < 29; ++bit) { + const uint32_t test_mask = 1 << bit; + if (guard_flavor & test_mask) { + // Make sure that no other bits are set. + DCHECK_EQ(guard_flavor, test_mask); + + metrics_guard_flavor = bit; + break; + } + } + + metrics_code_0 = (guard_type << 8) | metrics_guard_flavor; + break; + } + + case EXC_CORPSE_NOTIFY: + // code_0 may be a pointer. See 10.12.2 xnu-3789.31.2/osfmk/kern/task.c + // task_deliver_crash_notification(). Just encode 0 for metrics purposes. + metrics_code_0 = 0; + break; + + default: + metrics_code_0 = InRangeCast(code_0, 0xffff); + if (exception == 0 && metrics_code_0 == 0 && signal != 0) { + // This exception came from a signal that did not originate as another + // Mach exception. Encode the signal number, using EXC_CRASH as the + // top-level exception type. This is safe because EXC_CRASH will not + // otherwise appear as metrics_exception. + metrics_exception = EXC_CRASH; + metrics_code_0 = signal; + } + break; + } + + return (metrics_exception << 16) | metrics_code_0; +} + bool IsExceptionNonfatalResource(exception_type_t exception, mach_exception_code_t code_0, pid_t pid) { diff --git a/util/mach/exception_types.h b/util/mach/exception_types.h index 834f2123..5082ed19 100644 --- a/util/mach/exception_types.h +++ b/util/mach/exception_types.h @@ -15,6 +15,7 @@ #ifndef CRASHPAD_UTIL_MACH_EXCEPTION_TYPES_H_ #define CRASHPAD_UTIL_MACH_EXCEPTION_TYPES_H_ +#include #include #include @@ -54,6 +55,55 @@ exception_type_t ExcCrashRecoverOriginalException( mach_exception_code_t* original_code_0, int* signal); +//! \brief Determines whether a given exception type could plausibly be carried +//! within an `EXC_CRASH` exception. +//! +//! \param[in] exception The exception type to test. +//! +//! \return `true` if an `EXC_CRASH` exception could plausibly carry \a +//! exception. +//! +//! An `EXC_CRASH` exception can wrap exceptions that originate as hardware +//! faults, as well as exceptions that originate from certain software sources +//! such as POSIX signals. It cannot wrap another `EXC_CRASH` exception, nor can +//! it wrap `EXC_RESOURCE`, `EXC_GUARD`, or `EXC_CORPSE_NOTIFY` exceptions. It +//! also cannot wrap Crashpad-specific #kMachExceptionSimulated exceptions. +bool ExcCrashCouldContainException(exception_type_t exception); + +//! \brief Returns the exception code to report via a configured metrics system. +//! +//! \param[in] exception The exception type as received by a Mach exception +//! handler. +//! \param[in] code_0 The first exception code (`code[0]`) as received by a +//! Mach exception handler. +//! +//! \return An exception code that maps useful information from \a exception and +//! \a code_0 to the more limited data type available for metrics reporting. +//! +//! For classic Mach exceptions (including hardware faults reported as Mach +//! exceptions), the mapping is `(exception << 16) | code_0`. +//! +//! For `EXC_CRASH` exceptions that originate as Mach exceptions described +//! above, the mapping above is used, with the original exception’s values. For +//! `EXC_CRASH` exceptions that originate as POSIX signals without an underlying +//! Mach exception, the mapping is `(EXC_CRASH << 16) | code_0`. +//! +//! `EXC_RESOURCE` and `EXC_GUARD` exceptions both contain exception-specific +//! “type” values and type-specific “flavor” values. In these cases, the mapping +//! is `(exception << 16) | (type << 8) | flavor`. For `EXC_GUARD`, the “flavor” +//! value is rewritten to be more space-efficient by replacing the +//! kernel-supplied bitmask having exactly one bit set with the index of the set +//! bit. +//! +//! `EXC_CORPSE_NOTIFY` exceptions are reported as classic Mach exceptions with +//! the \a code_0 field set to `0`. +//! +//! If \a exception is #kMachExceptionSimulated, that value is returned as-is. +//! +//! Overflow conditions in any field are handled via saturation. +int32_t ExceptionCodeForMetrics(exception_type_t exception, + mach_exception_code_t code_0); + //! \brief Determines whether an exception is a non-fatal `EXC_RESOURCE`. //! //! \param[in] exception The exception type as received by a Mach exception diff --git a/util/mach/exception_types_test.cc b/util/mach/exception_types_test.cc index d82dcbb9..7ecdd252 100644 --- a/util/mach/exception_types_test.cc +++ b/util/mach/exception_types_test.cc @@ -41,9 +41,21 @@ TEST(ExceptionTypes, ExcCrashRecoverOriginalException) { {0xb100001, EXC_BAD_ACCESS, KERN_INVALID_ADDRESS, SIGSEGV}, {0xb100002, EXC_BAD_ACCESS, KERN_PROTECTION_FAILURE, SIGSEGV}, {0xa100002, EXC_BAD_ACCESS, KERN_PROTECTION_FAILURE, SIGBUS}, - {0x4200001, EXC_BAD_INSTRUCTION, 1, SIGILL}, - {0x8300001, EXC_ARITHMETIC, 1, SIGFPE}, - {0x5600002, EXC_BREAKPOINT, 2, SIGTRAP}, + {0xa100005, EXC_BAD_ACCESS, VM_PROT_READ | VM_PROT_EXECUTE, SIGBUS}, + {0xa10000d, EXC_BAD_ACCESS, EXC_I386_GPFLT, SIGBUS}, + {0x9100032, EXC_BAD_ACCESS, KERN_CODESIGN_ERROR, SIGKILL}, + {0x4200001, EXC_BAD_INSTRUCTION, EXC_I386_INVOP, SIGILL}, + {0x420000b, EXC_BAD_INSTRUCTION, EXC_I386_SEGNPFLT, SIGILL}, + {0x420000c, EXC_BAD_INSTRUCTION, EXC_I386_STKFLT, SIGILL}, + {0x8300001, EXC_ARITHMETIC, EXC_I386_DIV, SIGFPE}, + {0x8300002, EXC_ARITHMETIC, EXC_I386_INTO, SIGFPE}, + {0x8300005, EXC_ARITHMETIC, EXC_I386_EXTERR, SIGFPE}, + {0x8300008, EXC_ARITHMETIC, EXC_I386_SSEEXTERR, SIGFPE}, + {0x5500007, EXC_SOFTWARE, EXC_I386_BOUND, SIGTRAP}, + {0x5600001, EXC_BREAKPOINT, EXC_I386_SGL, SIGTRAP}, + {0x5600002, EXC_BREAKPOINT, EXC_I386_BPT, SIGTRAP}, + {0x0700080, EXC_SYSCALL, 128, 0}, + {0x0706000, EXC_SYSCALL, 0x6000, 0}, {0x3000000, 0, 0, SIGQUIT}, {0x6000000, 0, 0, SIGABRT}, {0xc000000, 0, 0, SIGSYS}, @@ -86,40 +98,173 @@ TEST(ExceptionTypes, ExcCrashRecoverOriginalException) { EXPECT_EQ(test_data.signal, signal); } -// These macros come from the #ifdef KERNEL section of : -// 10.10 xnu-2782.1.97/osfmk/kern/exc_resource.h. -#define EXC_RESOURCE_ENCODE_TYPE(code, type) \ - ((code) |= ((static_cast(type) & 0x7ull) << 61)) -#define EXC_RESOURCE_ENCODE_FLAVOR(code, flavor) \ - ((code) |= ((static_cast(flavor) & 0x7ull) << 58)) +TEST(ExceptionTypes, ExcCrashCouldContainException) { + // This seems wrong, but it happens when EXC_CRASH carries an exception not + // originally caused by a hardware fault, such as SIGABRT. + EXPECT_TRUE(ExcCrashCouldContainException(0)); + + EXPECT_TRUE(ExcCrashCouldContainException(EXC_BAD_ACCESS)); + EXPECT_TRUE(ExcCrashCouldContainException(EXC_BAD_INSTRUCTION)); + EXPECT_TRUE(ExcCrashCouldContainException(EXC_ARITHMETIC)); + EXPECT_TRUE(ExcCrashCouldContainException(EXC_EMULATION)); + EXPECT_TRUE(ExcCrashCouldContainException(EXC_SOFTWARE)); + EXPECT_TRUE(ExcCrashCouldContainException(EXC_BREAKPOINT)); + EXPECT_TRUE(ExcCrashCouldContainException(EXC_SYSCALL)); + EXPECT_TRUE(ExcCrashCouldContainException(EXC_MACH_SYSCALL)); + EXPECT_TRUE(ExcCrashCouldContainException(EXC_RPC_ALERT)); + EXPECT_FALSE(ExcCrashCouldContainException(EXC_CRASH)); + EXPECT_FALSE(ExcCrashCouldContainException(EXC_RESOURCE)); + EXPECT_FALSE(ExcCrashCouldContainException(EXC_GUARD)); + EXPECT_FALSE(ExcCrashCouldContainException(EXC_CORPSE_NOTIFY)); + EXPECT_FALSE(ExcCrashCouldContainException(kMachExceptionSimulated)); +} + +// This macro is adapted from those in the #ifdef KERNEL section of +// : 10.12.2 xnu-3789.31.2/osfmk/kern/exc_resource.h. +#define EXC_RESOURCE_ENCODE_TYPE_FLAVOR(type, flavor) \ + (static_cast( \ + (((static_cast(type) & 0x7ull) << 61) | \ + (static_cast(flavor) & 0x7ull) << 58))) + +TEST(ExceptionTypes, ExceptionCodeForMetrics) { + struct TestData { + exception_type_t exception; + mach_exception_code_t code_0; + int32_t metrics_code; + }; + const TestData kTestData[] = { +#define ENCODE_EXC(type, code_0) \ + { (type), (code_0), ((type) << 16) | (code_0) } + ENCODE_EXC(EXC_BAD_ACCESS, KERN_INVALID_ADDRESS), + ENCODE_EXC(EXC_BAD_ACCESS, KERN_PROTECTION_FAILURE), + ENCODE_EXC(EXC_BAD_ACCESS, VM_PROT_READ | VM_PROT_EXECUTE), + ENCODE_EXC(EXC_BAD_ACCESS, EXC_I386_GPFLT), + ENCODE_EXC(EXC_BAD_ACCESS, KERN_CODESIGN_ERROR), + ENCODE_EXC(EXC_BAD_INSTRUCTION, EXC_I386_INVOP), + ENCODE_EXC(EXC_BAD_INSTRUCTION, EXC_I386_SEGNPFLT), + ENCODE_EXC(EXC_BAD_INSTRUCTION, EXC_I386_STKFLT), + ENCODE_EXC(EXC_ARITHMETIC, EXC_I386_DIV), + ENCODE_EXC(EXC_ARITHMETIC, EXC_I386_INTO), + ENCODE_EXC(EXC_ARITHMETIC, EXC_I386_EXTERR), + ENCODE_EXC(EXC_ARITHMETIC, EXC_I386_SSEEXTERR), + ENCODE_EXC(EXC_SOFTWARE, EXC_I386_BOUND), + ENCODE_EXC(EXC_BREAKPOINT, EXC_I386_SGL), + ENCODE_EXC(EXC_BREAKPOINT, EXC_I386_BPT), + ENCODE_EXC(EXC_SYSCALL, 128), + ENCODE_EXC(EXC_SYSCALL, 0x6000), +#undef ENCODE_EXC + +#define ENCODE_EXC_CRASH(type, code_0) \ + { \ + EXC_CRASH, (((type) & 0xf) << 20) | ((code_0) & 0xfffff), \ + ((type) << 16) | (code_0) \ + } + ENCODE_EXC_CRASH(EXC_BAD_ACCESS, KERN_INVALID_ADDRESS), + ENCODE_EXC_CRASH(EXC_BAD_ACCESS, KERN_PROTECTION_FAILURE), + ENCODE_EXC_CRASH(EXC_BAD_ACCESS, VM_PROT_READ | VM_PROT_EXECUTE), + ENCODE_EXC_CRASH(EXC_BAD_ACCESS, EXC_I386_GPFLT), + ENCODE_EXC_CRASH(EXC_BAD_ACCESS, KERN_CODESIGN_ERROR), + ENCODE_EXC_CRASH(EXC_BAD_INSTRUCTION, EXC_I386_INVOP), + ENCODE_EXC_CRASH(EXC_BAD_INSTRUCTION, EXC_I386_SEGNPFLT), + ENCODE_EXC_CRASH(EXC_BAD_INSTRUCTION, EXC_I386_STKFLT), + ENCODE_EXC_CRASH(EXC_ARITHMETIC, EXC_I386_DIV), + ENCODE_EXC_CRASH(EXC_ARITHMETIC, EXC_I386_INTO), + ENCODE_EXC_CRASH(EXC_ARITHMETIC, EXC_I386_EXTERR), + ENCODE_EXC_CRASH(EXC_ARITHMETIC, EXC_I386_SSEEXTERR), + ENCODE_EXC_CRASH(EXC_SOFTWARE, EXC_I386_BOUND), + ENCODE_EXC_CRASH(EXC_BREAKPOINT, EXC_I386_SGL), + ENCODE_EXC_CRASH(EXC_BREAKPOINT, EXC_I386_BPT), + ENCODE_EXC_CRASH(EXC_SYSCALL, 128), + ENCODE_EXC_CRASH(EXC_SYSCALL, 0x6000), +#undef ENCODE_EXC_CRASH + +#define ENCODE_EXC_CRASH_SIGNAL(signal) \ + { EXC_CRASH, (((signal) & 0xff) << 24), (EXC_CRASH << 16) | (signal) } + ENCODE_EXC_CRASH_SIGNAL(SIGQUIT), + ENCODE_EXC_CRASH_SIGNAL(SIGABRT), + ENCODE_EXC_CRASH_SIGNAL(SIGSYS), +#undef ENCODE_EXC_CRASH_SIGNAL + +#define ENCODE_EXC_RESOURCE(type, flavor) \ + { \ + EXC_RESOURCE, EXC_RESOURCE_ENCODE_TYPE_FLAVOR((type), (flavor)), \ + (EXC_RESOURCE << 16) | ((type) << 8) | (flavor) \ + } + ENCODE_EXC_RESOURCE(RESOURCE_TYPE_CPU, FLAVOR_CPU_MONITOR), + ENCODE_EXC_RESOURCE(RESOURCE_TYPE_CPU, FLAVOR_CPU_MONITOR_FATAL), + ENCODE_EXC_RESOURCE(RESOURCE_TYPE_WAKEUPS, FLAVOR_WAKEUPS_MONITOR), + ENCODE_EXC_RESOURCE(RESOURCE_TYPE_MEMORY, FLAVOR_HIGH_WATERMARK), + ENCODE_EXC_RESOURCE(RESOURCE_TYPE_IO, FLAVOR_IO_PHYSICAL_WRITES), + ENCODE_EXC_RESOURCE(RESOURCE_TYPE_IO, FLAVOR_IO_LOGICAL_WRITES), +#undef ENCODE_EXC_RESOURCE + +#define ENCODE_EXC_GUARD(type, flavor) \ + { \ + EXC_GUARD, \ + static_cast(static_cast((type) & 0x7) \ + << 61) | \ + (static_cast((1 << (flavor)) & 0x1ffffffff) << 32), \ + (EXC_GUARD << 16) | ((type) << 8) | (flavor) \ + } + ENCODE_EXC_GUARD(GUARD_TYPE_MACH_PORT, 0), // kGUARD_EXC_DESTROY + ENCODE_EXC_GUARD(GUARD_TYPE_MACH_PORT, 1), // kGUARD_EXC_MOD_REFS + ENCODE_EXC_GUARD(GUARD_TYPE_MACH_PORT, 2), // kGUARD_EXC_SET_CONTEXT + ENCODE_EXC_GUARD(GUARD_TYPE_MACH_PORT, 3), // kGUARD_EXC_UNGUARDED + ENCODE_EXC_GUARD(GUARD_TYPE_MACH_PORT, 4), // kGUARD_EXC_INCORRECT_GUARD + + // 2 is GUARD_TYPE_FD from 10.12.2 xnu-3789.31.2/bsd/sys/guarded.h. + ENCODE_EXC_GUARD(2, 0), // kGUARD_EXC_CLOSE + ENCODE_EXC_GUARD(2, 1), // kGUARD_EXC_DUP + ENCODE_EXC_GUARD(2, 2), // kGUARD_EXC_NOCLOEXEC + ENCODE_EXC_GUARD(2, 3), // kGUARD_EXC_SOCKET_IPC + ENCODE_EXC_GUARD(2, 4), // kGUARD_EXC_FILEPORT + ENCODE_EXC_GUARD(2, 5), // kGUARD_EXC_MISMATCH + ENCODE_EXC_GUARD(2, 6), // kGUARD_EXC_WRITE +#undef ENCODE_EXC_GUARD + + // Test that overflow saturates. + {0x00010000, 0x00001000, static_cast(0xffff1000)}, + {0x00001000, 0x00010000, 0x1000ffff}, + {0x00010000, 0x00010000, static_cast(0xffffffff)}, + }; + + for (size_t index = 0; index < arraysize(kTestData); ++index) { + const TestData& test_data = kTestData[index]; + SCOPED_TRACE(base::StringPrintf("index %zu, exception 0x%x, code_0 0x%llx", + index, + test_data.exception, + test_data.code_0)); + + int32_t metrics_code = + ExceptionCodeForMetrics(test_data.exception, test_data.code_0); + + EXPECT_EQ(test_data.metrics_code, metrics_code); + } +} TEST(ExceptionTypes, IsExceptionNonfatalResource) { const pid_t pid = getpid(); - mach_exception_code_t code = 0; - EXC_RESOURCE_ENCODE_TYPE(code, RESOURCE_TYPE_CPU); - EXC_RESOURCE_ENCODE_FLAVOR(code, FLAVOR_CPU_MONITOR); + mach_exception_code_t code = + EXC_RESOURCE_ENCODE_TYPE_FLAVOR(RESOURCE_TYPE_CPU, FLAVOR_CPU_MONITOR); EXPECT_TRUE(IsExceptionNonfatalResource(EXC_RESOURCE, code, pid)); if (MacOSXMinorVersion() >= 10) { // FLAVOR_CPU_MONITOR_FATAL was introduced in OS X 10.10. - code = 0; - EXC_RESOURCE_ENCODE_TYPE(code, RESOURCE_TYPE_CPU); - EXC_RESOURCE_ENCODE_FLAVOR(code, FLAVOR_CPU_MONITOR_FATAL); + code = EXC_RESOURCE_ENCODE_TYPE_FLAVOR(RESOURCE_TYPE_CPU, + FLAVOR_CPU_MONITOR_FATAL); EXPECT_FALSE(IsExceptionNonfatalResource(EXC_RESOURCE, code, pid)); } // This assumes that WAKEMON_MAKE_FATAL is not set for this process. The // default is for WAKEMON_MAKE_FATAL to not be set, there’s no public API to // enable it, and nothing in this process should have enabled it. - code = 0; - EXC_RESOURCE_ENCODE_TYPE(code, RESOURCE_TYPE_WAKEUPS); - EXC_RESOURCE_ENCODE_FLAVOR(code, FLAVOR_WAKEUPS_MONITOR); + code = EXC_RESOURCE_ENCODE_TYPE_FLAVOR(RESOURCE_TYPE_WAKEUPS, + FLAVOR_WAKEUPS_MONITOR); EXPECT_TRUE(IsExceptionNonfatalResource(EXC_RESOURCE, code, pid)); - code = 0; - EXC_RESOURCE_ENCODE_TYPE(code, RESOURCE_TYPE_MEMORY); - EXC_RESOURCE_ENCODE_FLAVOR(code, FLAVOR_HIGH_WATERMARK); + code = EXC_RESOURCE_ENCODE_TYPE_FLAVOR(RESOURCE_TYPE_MEMORY, + FLAVOR_HIGH_WATERMARK); EXPECT_TRUE(IsExceptionNonfatalResource(EXC_RESOURCE, code, pid)); // Non-EXC_RESOURCE exceptions should never be considered nonfatal resource diff --git a/util/misc/address_sanitizer.h b/util/misc/address_sanitizer.h new file mode 100644 index 00000000..75c72399 --- /dev/null +++ b/util/misc/address_sanitizer.h @@ -0,0 +1,28 @@ +// Copyright 2016 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. + +#ifndef CRASHPAD_UTIL_MISC_ADDRESS_SANITIZER_H_ +#define CRASHPAD_UTIL_MISC_ADDRESS_SANITIZER_H_ + +#include "base/compiler_specific.h" +#include "build/build_config.h" + +#if !defined(ADDRESS_SANITIZER) +#if HAS_FEATURE(address_sanitizer) || \ + (defined(COMPILER_GCC) && defined(__SANITIZE_ADDRESS__)) +#define ADDRESS_SANITIZER 1 +#endif +#endif // !defined(ADDRESS_SANITIZER) + +#endif // CRASHPAD_UTIL_MISC_ADDRESS_SANITIZER_H_ diff --git a/util/misc/arraysize_unsafe_test.cc b/util/misc/arraysize_unsafe_test.cc index 1c638e02..a6660aee 100644 --- a/util/misc/arraysize_unsafe_test.cc +++ b/util/misc/arraysize_unsafe_test.cc @@ -14,6 +14,7 @@ #include "util/misc/arraysize_unsafe.h" +#include "base/compiler_specific.h" #include "gtest/gtest.h" namespace crashpad { @@ -21,35 +22,37 @@ namespace test { namespace { TEST(ArraySizeUnsafe, ArraySizeUnsafe) { - char c0[0]; - static_assert(ARRAYSIZE_UNSAFE(c0) == 0, "c0"); - char c1[1]; static_assert(ARRAYSIZE_UNSAFE(c1) == 1, "c1"); + ALLOW_UNUSED_LOCAL(c1); char c2[2]; static_assert(ARRAYSIZE_UNSAFE(c2) == 2, "c2"); + ALLOW_UNUSED_LOCAL(c2); char c4[4]; static_assert(ARRAYSIZE_UNSAFE(c4) == 4, "c4"); - - int i0[0]; - static_assert(ARRAYSIZE_UNSAFE(i0) == 0, "i0"); + ALLOW_UNUSED_LOCAL(c4); int i1[1]; static_assert(ARRAYSIZE_UNSAFE(i1) == 1, "i1"); + ALLOW_UNUSED_LOCAL(i1); int i2[2]; static_assert(ARRAYSIZE_UNSAFE(i2) == 2, "i2"); + ALLOW_UNUSED_LOCAL(i2); int i4[4]; static_assert(ARRAYSIZE_UNSAFE(i4) == 4, "i4"); + ALLOW_UNUSED_LOCAL(i4); long l8[8]; static_assert(ARRAYSIZE_UNSAFE(l8) == 8, "l8"); + ALLOW_UNUSED_LOCAL(l8); int l9[9]; static_assert(ARRAYSIZE_UNSAFE(l9) == 9, "l9"); + ALLOW_UNUSED_LOCAL(l9); struct S { char c; @@ -58,14 +61,13 @@ TEST(ArraySizeUnsafe, ArraySizeUnsafe) { bool b; }; - S s0[0]; - static_assert(ARRAYSIZE_UNSAFE(s0) == 0, "s0"); - S s1[1]; static_assert(ARRAYSIZE_UNSAFE(s1) == 1, "s1"); + ALLOW_UNUSED_LOCAL(s1); S s10[10]; static_assert(ARRAYSIZE_UNSAFE(s10) == 10, "s10"); + ALLOW_UNUSED_LOCAL(s10); } } // namespace diff --git a/util/misc/uuid.cc b/util/misc/uuid.cc index 4f1fbcbe..d4345a8a 100644 --- a/util/misc/uuid.cc +++ b/util/misc/uuid.cc @@ -95,17 +95,11 @@ bool UUID::InitializeWithNew() { uuid_generate(uuid); InitializeFromBytes(uuid); return true; -#elif defined(OS_WIN) - ::UUID system_uuid; - if (UuidCreate(&system_uuid) != RPC_S_OK) { - LOG(ERROR) << "UuidCreate"; - return false; - } - InitializeFromSystemUUID(&system_uuid); - return true; -#elif defined(OS_LINUX) || defined(OS_ANDROID) +#elif defined(OS_WIN) || defined(OS_LINUX) || defined(OS_ANDROID) // Linux does not provide a UUID generator in a widely-available system // library. uuid_generate() from libuuid is not available everywhere. + // On Windows, do not use UuidCreate() to avoid a dependency on rpcrt4, so + // that this function is usable early in DllMain(). base::RandBytes(this, sizeof(*this)); // Set six bits per RFC 4122 §4.4 to identify this as a pseudo-random UUID. diff --git a/util/stdlib/string_number_conversion.cc b/util/stdlib/string_number_conversion.cc index 28f6ea11..65d5a632 100644 --- a/util/stdlib/string_number_conversion.cc +++ b/util/stdlib/string_number_conversion.cc @@ -110,6 +110,13 @@ struct StringToUnsignedIntTraits } }; +struct StringToInt64Traits + : public StringToSignedIntegerTraits { + static LongType Convert(const char* str, char** end, int base) { + return strtoll(str, end, base); + } +}; + struct StringToUnsignedInt64Traits : public StringToUnsignedIntegerTraits { static LongType Convert(const char* str, char** end, int base) { @@ -166,6 +173,10 @@ bool StringToNumber(const base::StringPiece& string, unsigned int* number) { return StringToIntegerInternal(string, number); } +bool StringToNumber(const base::StringPiece& string, int64_t* number) { + return StringToIntegerInternal(string, number); +} + bool StringToNumber(const base::StringPiece& string, uint64_t* number) { return StringToIntegerInternal(string, number); } diff --git a/util/stdlib/string_number_conversion.h b/util/stdlib/string_number_conversion.h index 8641a690..b7bdcce8 100644 --- a/util/stdlib/string_number_conversion.h +++ b/util/stdlib/string_number_conversion.h @@ -56,6 +56,7 @@ namespace crashpad { //! where such prefix recognition is desirable. bool StringToNumber(const base::StringPiece& string, int* number); bool StringToNumber(const base::StringPiece& string, unsigned int* number); +bool StringToNumber(const base::StringPiece& string, int64_t* number); bool StringToNumber(const base::StringPiece& string, uint64_t* number); //! \} diff --git a/util/stdlib/string_number_conversion_test.cc b/util/stdlib/string_number_conversion_test.cc index 6fe6f813..c455a38d 100644 --- a/util/stdlib/string_number_conversion_test.cc +++ b/util/stdlib/string_number_conversion_test.cc @@ -221,6 +221,105 @@ TEST(StringNumberConversion, StringToUnsignedInt) { EXPECT_EQ(6u, output); } +TEST(StringNumberConversion, StringToInt64) { + const struct { + const char* string; + bool valid; + int64_t value; + } kTestData[] = { + {"", false, 0}, + {"0", true, 0}, + {"1", true, 1}, + {"2147483647", true, 2147483647}, + {"2147483648", true, 2147483648}, + {"4294967295", true, 4294967295}, + {"4294967296", true, 4294967296}, + {"9223372036854775807", true, std::numeric_limits::max()}, + {"9223372036854775808", false, 0}, + {"18446744073709551615", false, 0}, + {"18446744073709551616", false, 0}, + {"-1", true, -1}, + {"-2147483648", true, INT64_C(-2147483648)}, + {"-2147483649", true, INT64_C(-2147483649)}, + {"-9223372036854775808", true, std::numeric_limits::min()}, + {"-9223372036854775809", false, 0}, + {"0x7fffffffffffffff", true, std::numeric_limits::max()}, + {"0x8000000000000000", false, 0}, + {"0xffffffffffffffff", false, 0}, + {"0x10000000000000000", false, 0}, + {"-0x7fffffffffffffff", true, -9223372036854775807}, + {"-0x8000000000000000", true, std::numeric_limits::min()}, + {"-0x8000000000000001", false, 0}, + {"0x7Fffffffffffffff", true, std::numeric_limits::max()}, + }; + + for (size_t index = 0; index < arraysize(kTestData); ++index) { + int64_t value; + bool valid = StringToNumber(kTestData[index].string, &value); + if (kTestData[index].valid) { + EXPECT_TRUE(valid) << "index " << index << ", string " + << kTestData[index].string; + if (valid) { + EXPECT_EQ(kTestData[index].value, value) + << "index " << index << ", string " << kTestData[index].string; + } + } else { + EXPECT_FALSE(valid) << "index " << index << ", string " + << kTestData[index].string << ", value " << value; + } + } +} + +TEST(StringNumberConversion, StringToUnsignedInt64) { + const struct { + const char* string; + bool valid; + uint64_t value; + } kTestData[] = { + {"", false, 0}, + {"0", true, 0}, + {"1", true, 1}, + {"2147483647", true, 2147483647}, + {"2147483648", true, 2147483648}, + {"4294967295", true, 4294967295}, + {"4294967296", true, 4294967296}, + {"9223372036854775807", true, 9223372036854775807}, + {"9223372036854775808", true, 9223372036854775808u}, + {"18446744073709551615", true, std::numeric_limits::max()}, + {"18446744073709551616", false, 0}, + {"-1", false, 0}, + {"-2147483648", false, 0}, + {"-2147483649", false, 0}, + {"-2147483648", false, 0}, + {"-9223372036854775808", false, 0}, + {"-9223372036854775809", false, 0}, + {"0x7fffffffffffffff", true, 9223372036854775807}, + {"0x8000000000000000", true, 9223372036854775808u}, + {"0xffffffffffffffff", true, std::numeric_limits::max()}, + {"0x10000000000000000", false, 0}, + {"-0x7fffffffffffffff", false, 0}, + {"-0x8000000000000000", false, 0}, + {"-0x8000000000000001", false, 0}, + {"0xFfffffffffffffff", true, std::numeric_limits::max()}, + }; + + for (size_t index = 0; index < arraysize(kTestData); ++index) { + uint64_t value; + bool valid = StringToNumber(kTestData[index].string, &value); + if (kTestData[index].valid) { + EXPECT_TRUE(valid) << "index " << index << ", string " + << kTestData[index].string; + if (valid) { + EXPECT_EQ(kTestData[index].value, value) + << "index " << index << ", string " << kTestData[index].string; + } + } else { + EXPECT_FALSE(valid) << "index " << index << ", string " + << kTestData[index].string << ", value " << value; + } + } +} + } // namespace } // namespace test } // namespace crashpad diff --git a/util/util.gyp b/util/util.gyp index 5768788a..abf0bfdc 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -84,6 +84,7 @@ 'mach/task_for_pid.h', 'mach/task_memory.cc', 'mach/task_memory.h', + 'misc/address_sanitizer.h', 'misc/arraysize_unsafe.h', 'misc/clock.h', 'misc/clock_mac.cc', @@ -262,8 +263,6 @@ ['OS=="win"', { 'link_settings': { 'libraries': [ - '-ladvapi32.lib', - '-lrpcrt4.lib', '-lwinhttp.lib', ], }, diff --git a/util/util_test.gyp b/util/util_test.gyp index b5989de9..e6bf5635 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -91,6 +91,7 @@ 'win/handle_test.cc', 'win/initial_client_data_test.cc', 'win/process_info_test.cc', + 'win/registration_protocol_win_test.cc', 'win/scoped_process_suspend_test.cc', 'win/time_test.cc', ], @@ -108,6 +109,7 @@ ], 'link_settings': { 'libraries': [ + '-ladvapi32.lib', '-limagehlp.lib', '-lrpcrt4.lib', ], diff --git a/util/win/nt_internals.cc b/util/win/nt_internals.cc index 12201620..97eba68b 100644 --- a/util/win/nt_internals.cc +++ b/util/win/nt_internals.cc @@ -42,7 +42,9 @@ NTSTATUS NTAPI NtSuspendProcess(HANDLE); NTSTATUS NTAPI NtResumeProcess(HANDLE); -void* NTAPI RtlGetUnloadEventTrace(); +VOID NTAPI RtlGetUnloadEventTraceEx(PULONG* ElementSize, + PULONG* ElementCount, + PVOID* EventTrace); namespace crashpad { @@ -145,12 +147,12 @@ NTSTATUS NtResumeProcess(HANDLE handle) { return nt_resume_process(handle); } -template -RTL_UNLOAD_EVENT_TRACE* RtlGetUnloadEventTrace() { - static const auto rtl_get_unload_event_trace = - GET_FUNCTION_REQUIRED(L"ntdll.dll", ::RtlGetUnloadEventTrace); - return reinterpret_cast*>( - rtl_get_unload_event_trace()); +void RtlGetUnloadEventTraceEx(ULONG** element_size, + ULONG** element_count, + void** event_trace) { + static const auto rtl_get_unload_event_trace_ex = + GET_FUNCTION_REQUIRED(L"ntdll.dll", ::RtlGetUnloadEventTraceEx); + rtl_get_unload_event_trace_ex(element_size, element_count, event_trace); } // Explicit instantiations with the only 2 valid template arguments to avoid @@ -169,10 +171,4 @@ template NTSTATUS NtOpenThread( const process_types::CLIENT_ID* client_id); -template RTL_UNLOAD_EVENT_TRACE* -RtlGetUnloadEventTrace(); - -template RTL_UNLOAD_EVENT_TRACE* -RtlGetUnloadEventTrace(); - } // namespace crashpad diff --git a/util/win/nt_internals.h b/util/win/nt_internals.h index 0a80fd53..41e5fa44 100644 --- a/util/win/nt_internals.h +++ b/util/win/nt_internals.h @@ -75,10 +75,7 @@ NTSTATUS NtSuspendProcess(HANDLE handle); NTSTATUS NtResumeProcess(HANDLE handle); -// From https://msdn.microsoft.com/en-us/library/bb432428(VS.85).aspx and -// http://processhacker.sourceforge.net/doc/struct___r_t_l___u_n_l_o_a_d___e_v_e_n_t___t_r_a_c_e.html -#define RTL_UNLOAD_EVENT_TRACE_NUMBER 64 - +// From https://msdn.microsoft.com/en-us/library/cc678403(v=vs.85).aspx. template struct RTL_UNLOAD_EVENT_TRACE { typename Traits::Pointer BaseAddress; @@ -87,14 +84,10 @@ struct RTL_UNLOAD_EVENT_TRACE { ULONG TimeDateStamp; ULONG CheckSum; WCHAR ImageName[32]; - ULONG Version0; - union { - ULONG Version1; - typename Traits::Pad alignment_for_x64; - }; }; -template -RTL_UNLOAD_EVENT_TRACE* RtlGetUnloadEventTrace(); +void RtlGetUnloadEventTraceEx(ULONG** element_size, + ULONG** element_count, + void** event_trace); } // namespace crashpad diff --git a/util/win/registration_protocol_win.cc b/util/win/registration_protocol_win.cc index 17638415..e5cbd8d0 100644 --- a/util/win/registration_protocol_win.cc +++ b/util/win/registration_protocol_win.cc @@ -15,12 +15,11 @@ #include "util/win/registration_protocol_win.h" #include -#include #include "base/logging.h" +#include "base/macros.h" #include "util/win/exception_handler_server.h" #include "util/win/scoped_handle.h" -#include "util/win/scoped_local_alloc.h" namespace crashpad { @@ -97,7 +96,6 @@ HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name, bool first_instance) { SECURITY_ATTRIBUTES security_attributes; SECURITY_ATTRIBUTES* security_attributes_pointer = nullptr; - ScopedLocalAlloc scoped_sec_desc; if (first_instance) { // Pre-Vista does not have integrity levels. @@ -105,21 +103,10 @@ HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name, const DWORD major_version = LOBYTE(LOWORD(version)); const bool is_vista_or_later = major_version >= 6; if (is_vista_or_later) { - // Mandatory Label, no ACE flags, no ObjectType, integrity level - // untrusted. - const wchar_t kSddl[] = L"S:(ML;;;;;S-1-16-0)"; - - PSECURITY_DESCRIPTOR sec_desc; - PCHECK(ConvertStringSecurityDescriptorToSecurityDescriptor( - kSddl, SDDL_REVISION_1, &sec_desc, nullptr)) - << "ConvertStringSecurityDescriptorToSecurityDescriptor"; - - // Take ownership of the allocated SECURITY_DESCRIPTOR. - scoped_sec_desc.reset(sec_desc); - memset(&security_attributes, 0, sizeof(security_attributes)); security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES); - security_attributes.lpSecurityDescriptor = sec_desc; + security_attributes.lpSecurityDescriptor = + const_cast(GetSecurityDescriptorForNamedPipeInstance(nullptr)); security_attributes.bInheritHandle = TRUE; security_attributes_pointer = &security_attributes; } @@ -136,4 +123,85 @@ HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name, security_attributes_pointer); } +const void* GetSecurityDescriptorForNamedPipeInstance(size_t* size) { + // Mandatory Label, no ACE flags, no ObjectType, integrity level untrusted is + // "S:(ML;;;;;S-1-16-0)". Typically + // ConvertStringSecurityDescriptorToSecurityDescriptor() would be used to + // convert from a string representation. However, that function cannot be used + // because it is in advapi32.dll and CreateNamedPipeInstance() is called from + // within DllMain() where the loader lock is held. advapi32.dll is delay + // loaded in chrome_elf.dll because it must avoid loading user32.dll. If an + // advapi32.dll function were used, it would cause a load of the DLL, which + // would in turn cause deadlock. + +#pragma pack(push, 1) + static const struct SecurityDescriptorBlob { + // See https://msdn.microsoft.com/en-us/library/cc230366.aspx. + SECURITY_DESCRIPTOR_RELATIVE sd_rel; + struct { + ACL acl; + struct { + // This is equivalent to SYSTEM_MANDATORY_LABEL_ACE, but there's no + // DWORD offset to the SID, instead it's inline. + ACE_HEADER header; + ACCESS_MASK mask; + SID sid; + } ace[1]; + } sacl; + } kSecDescBlob = { + // sd_rel. + { + SECURITY_DESCRIPTOR_REVISION1, // Revision. + 0x00, // Sbz1. + SE_SELF_RELATIVE | SE_SACL_PRESENT, // Control. + 0, // OffsetOwner. + 0, // OffsetGroup. + offsetof(SecurityDescriptorBlob, sacl), // OffsetSacl. + 0, // OffsetDacl. + }, + + // sacl. + { + // acl. + { + ACL_REVISION, // AclRevision. + 0, // Sbz1. + sizeof(kSecDescBlob.sacl), // AclSize. + arraysize(kSecDescBlob.sacl.ace), // AceCount. + 0, // Sbz2. + }, + + // ace[0]. + { + { + // header. + { + SYSTEM_MANDATORY_LABEL_ACE_TYPE, // AceType. + 0, // AceFlags. + sizeof(kSecDescBlob.sacl.ace[0]), // AceSize. + }, + + // mask. + 0, + + // sid. + { + SID_REVISION, // Revision. + // SubAuthorityCount. + arraysize(kSecDescBlob.sacl.ace[0].sid.SubAuthority), + // IdentifierAuthority. + {SECURITY_MANDATORY_LABEL_AUTHORITY}, + {SECURITY_MANDATORY_UNTRUSTED_RID}, // SubAuthority. + }, + }, + }, + }, + }; +#pragma pack(pop) + + if (size) + *size = sizeof(kSecDescBlob); + return reinterpret_cast(&kSecDescBlob); +} + } // namespace crashpad diff --git a/util/win/registration_protocol_win.h b/util/win/registration_protocol_win.h index 12209835..5f04a466 100644 --- a/util/win/registration_protocol_win.h +++ b/util/win/registration_protocol_win.h @@ -145,6 +145,18 @@ bool SendToCrashHandlerServer(const base::string16& pipe_name, HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name, bool first_instance); +//! \brief Returns the SECURITY_DESCRIPTOR blob that will be used for creating +//! the connection pipe in CreateNamedPipeInstance(). +//! +//! This function is exposed for only for testing. +//! +//! \param[out] size The size of the returned blob. May be `nullptr` if not +//! required. +//! +//! \return A pointer to a self-relative `SECURITY_DESCRIPTOR`. Ownership is not +//! transferred to the caller. +const void* GetSecurityDescriptorForNamedPipeInstance(size_t* size); + } // namespace crashpad #endif // CRASHPAD_UTIL_WIN_REGISTRATION_PROTOCOL_WIN_H_ diff --git a/util/win/registration_protocol_win_test.cc b/util/win/registration_protocol_win_test.cc new file mode 100644 index 00000000..60e7c86c --- /dev/null +++ b/util/win/registration_protocol_win_test.cc @@ -0,0 +1,53 @@ +// Copyright 2016 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 "util/win/registration_protocol_win.h" + +#include +#include +#include + +#include "gtest/gtest.h" +#include "test/errors.h" +#include "util/win/scoped_local_alloc.h" + +namespace crashpad { +namespace test { +namespace { + +TEST(SecurityDescriptor, MatchesAdvapi32) { + // This security descriptor is built manually in the connection code to avoid + // calling the advapi32 functions. Verify that it returns the same thing as + // ConvertStringSecurityDescriptorToSecurityDescriptor() would. + + // Mandatory Label, no ACE flags, no ObjectType, integrity level + // untrusted. + const wchar_t kSddl[] = L"S:(ML;;;;;S-1-16-0)"; + PSECURITY_DESCRIPTOR sec_desc; + ULONG sec_desc_len; + ASSERT_TRUE(ConvertStringSecurityDescriptorToSecurityDescriptor( + kSddl, SDDL_REVISION_1, &sec_desc, &sec_desc_len)) + << ErrorMessage("ConvertStringSecurityDescriptorToSecurityDescriptor"); + ScopedLocalAlloc sec_desc_owner(sec_desc); + + size_t created_len; + const void* const created = + GetSecurityDescriptorForNamedPipeInstance(&created_len); + ASSERT_EQ(sec_desc_len, created_len); + EXPECT_EQ(0, memcmp(sec_desc, created, sec_desc_len)); +} + +} // namespace +} // namespace test +} // namespace crashpad