diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 4ae8da03..bc0f32c0 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -391,7 +391,7 @@ bool StartHandlerProcess( } ScopedKernelHANDLE this_process( - OpenProcess(kXPProcessAllAccess, true, GetCurrentProcessId())); + OpenProcess(kXPProcessLimitedAccess, true, GetCurrentProcessId())); if (!this_process.is_valid()) { PLOG(ERROR) << "OpenProcess"; return false; diff --git a/snapshot/win/process_reader_win.cc b/snapshot/win/process_reader_win.cc index e3784cae..56203b02 100644 --- a/snapshot/win/process_reader_win.cc +++ b/snapshot/win/process_reader_win.cc @@ -138,6 +138,7 @@ bool FillThreadContextAndSuspendCount(HANDLE thread_handle, bool is_current_thread = thread->id == reinterpret_cast*>( NtCurrentTeb())->ClientId.UniqueThread; + bool did_suspend_thread = true; if (is_current_thread) { DCHECK(suspension_state == ProcessSuspensionState::kRunning); @@ -147,8 +148,9 @@ bool FillThreadContextAndSuspendCount(HANDLE thread_handle, } else { DWORD previous_suspend_count = SuspendThread(thread_handle); if (previous_suspend_count == static_cast(-1)) { - PLOG(ERROR) << "SuspendThread"; - return false; + PLOG(WARNING) << "SuspendThread"; + did_suspend_thread = false; + previous_suspend_count = 1; } if (previous_suspend_count <= 0 && suspension_state == ProcessSuspensionState::kSuspended) { @@ -183,7 +185,7 @@ bool FillThreadContextAndSuspendCount(HANDLE thread_handle, } } - if (!ResumeThread(thread_handle)) { + if (did_suspend_thread && !ResumeThread(thread_handle)) { PLOG(ERROR) << "ResumeThread"; return false; } diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index 9394256e..4bbcf9ab 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -127,6 +127,7 @@ class ClientData { non_crash_dump_completed_event_( std::move(non_crash_dump_completed_event)), process_(std::move(process)), + process_promoted_(false), crash_exception_information_address_( crash_exception_information_address), non_crash_exception_information_address_( @@ -167,6 +168,28 @@ class ClientData { } HANDLE process() const { return process_.get(); } + HANDLE process_promoted() + { + if (!process_promoted_) + { + // Duplicate restricted process handle for a full memory access handle. + HANDLE hAllAccessHandle = nullptr; + if (DuplicateHandle(GetCurrentProcess(), + process_.get(), + GetCurrentProcess(), + &hAllAccessHandle, + kXPProcessAllAccess, + FALSE, + 0)) + { + ScopedKernelHANDLE ScopedAllAccessHandle(hAllAccessHandle); + process_.swap(ScopedAllAccessHandle); + process_promoted_ = true; + } + } + return process_.get(); + } + private: void RegisterThreadPoolWaits( WAITORTIMERCALLBACK crash_dump_request_callback, @@ -227,6 +250,7 @@ class ClientData { ScopedKernelHANDLE non_crash_dump_requested_event_; ScopedKernelHANDLE non_crash_dump_completed_event_; ScopedKernelHANDLE process_; + bool process_promoted_; WinVMAddress crash_exception_information_address_; WinVMAddress non_crash_exception_information_address_; WinVMAddress debug_critical_section_address_; @@ -456,14 +480,14 @@ bool ExceptionHandlerServer::ServiceClientConnection( // the process, but the client will be able to, so we make a second attempt // having impersonated the client. HANDLE client_process = OpenProcess( - kXPProcessAllAccess, false, message.registration.client_process_id); + kXPProcessLimitedAccess, false, message.registration.client_process_id); if (!client_process) { if (!ImpersonateNamedPipeClient(service_context.pipe())) { PLOG(ERROR) << "ImpersonateNamedPipeClient"; return false; } client_process = OpenProcess( - kXPProcessAllAccess, false, message.registration.client_process_id); + kXPProcessLimitedAccess, false, message.registration.client_process_id); PCHECK(RevertToSelf()); if (!client_process) { LOG(ERROR) << "failed to open " << message.registration.client_process_id; @@ -540,11 +564,11 @@ void __stdcall ExceptionHandlerServer::OnCrashDumpEvent(void* ctx, BOOLEAN) { // Capture the exception. unsigned int exit_code = client->delegate()->ExceptionHandlerServerException( - client->process(), + client->process_promoted(), client->crash_exception_information_address(), client->debug_critical_section_address()); - SafeTerminateProcess(client->process(), exit_code); + SafeTerminateProcess(client->process_promoted(), exit_code); } // static @@ -555,7 +579,7 @@ void __stdcall ExceptionHandlerServer::OnNonCrashDumpEvent(void* ctx, BOOLEAN) { // Capture the exception. client->delegate()->ExceptionHandlerServerException( - client->process(), + client->process_promoted(), client->non_crash_exception_information_address(), client->debug_critical_section_address()); diff --git a/util/win/xp_compat.h b/util/win/xp_compat.h index 1d4d4a0b..45789f56 100644 --- a/util/win/xp_compat.h +++ b/util/win/xp_compat.h @@ -26,6 +26,7 @@ enum { //! against a Vista+ SDK results in `ERROR_ACCESS_DENIED` when running on XP. //! See https://msdn.microsoft.com/library/ms684880.aspx. kXPProcessAllAccess = STANDARD_RIGHTS_REQUIRED | SYNCHRONIZE | 0xFFF, + kXPProcessLimitedAccess = PROCESS_DUP_HANDLE | PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_TERMINATE | SYNCHRONIZE, //! \brief This is the XP-suitable value of `THREAD_ALL_ACCESS`. //!