diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index fa87ef996..cd1b206b0 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -438,7 +438,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 8c3bb160d..d203f6fd7 100644 --- a/snapshot/win/process_reader_win.cc +++ b/snapshot/win/process_reader_win.cc @@ -229,6 +229,7 @@ bool FillThreadContextAndSuspendCount(HANDLE process, thread->id == reinterpret_cast*>(NtCurrentTeb()) ->ClientId.UniqueThread; + bool did_suspend_thread = true; if (is_current_thread) { DCHECK(suspension_state == ProcessSuspensionState::kRunning); @@ -243,7 +244,9 @@ bool FillThreadContextAndSuspendCount(HANDLE process, DWORD previous_suspend_count = SuspendThread(thread_handle); if (previous_suspend_count == static_cast(-1)) { PLOG(ERROR) << "SuspendThread"; - return false; + // Must assume thread was already suspended, so we can still try to read + did_suspend_thread = false; + previous_suspend_count = 1; } if (previous_suspend_count <= 0 && suspension_state == ProcessSuspensionState::kSuspended) { @@ -281,7 +284,7 @@ bool FillThreadContextAndSuspendCount(HANDLE process, DoStackWalk(thread, process, thread_handle, is_64_reading_32); #endif - 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 e641c7fb9..32d767df7 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -129,6 +129,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_( @@ -172,6 +173,29 @@ class ClientData { } HANDLE process() const { return process_.get(); } + // Promotes the process handle to full access if it hasn't already been done. + 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, @@ -232,6 +256,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_; @@ -459,14 +484,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; @@ -543,11 +568,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 @@ -558,7 +583,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 1499c08aa..314fd73bb 100644 --- a/util/win/xp_compat.h +++ b/util/win/xp_compat.h @@ -26,6 +26,10 @@ 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, + + // A limited access version, suitable for initial access to the process. + kXPProcessLimitedAccess = PROCESS_DUP_HANDLE | PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_TERMINATE | SYNCHRONIZE, + //! \brief This is the XP-suitable value of `THREAD_ALL_ACCESS`. //!