3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2025-01-14 09:17:57 +08:00
Code Issues Projects Releases Wiki Activity

win: Wrap TerminateProcess() to accept cdecl patches on x86

Browse Source 
TerminateProcess(), like most of the Windows API, is declared WINAPI,
which is __stdcall on 32-bit x86. That means that the callee,
TerminateProcess() itself, is responsible for cleaning up parameters on
the stack on return. In https://crashpad.chromium.org/bug/179, crashes
in ExceptionHandlerServer::OnNonCrashDumpEvent() were observed in ways
that make it evident that TerminateProcess() has been patched with a
__cdecl routine. The crucial difference between __stdcall and __cdecl is
that the caller is responsible for stack parameter cleanup in __cdecl.
The mismatch means that nobody cleans parameters from the stack, and the
stack pointer has an unexpected value, which in the case of the Crashpad
handler crash, results in TerminateProcess()’s second argument
erroneously being used as the lock address in the call to
ReleaseSRWLockExclusive() or LeaveCriticalSection().

As a workaround, on 32-bit x86, call through SafeTerminateProcess(), a
custom assembly routine that’s compatible with either __stdcall or
__cdecl implementations of TerminateProcess() by not trusting the value
of the stack pointer on return from that function. Instead, the stack
pointer is restored directly from the frame pointer.

Bug: crashpad:179
Test: crashpad_util_test SafeTerminateProcess.*, others
Change-Id: If9508f4eb7631020ea69ddbbe4a22eb335cdb325
Reviewed-on: https://chromium-review.googlesource.com/481180
Reviewed-by: Scott Graham <scottmg@chromium.org>
This commit is contained in:
Mark Mentovai 2017-04-19 13:22:08 -04:00
parent ffe4c1018c
commit e04194afd9
10 changed files with 360 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
client/crashpad_client_win.cc
View File

@ -42,6 +42,7 @@
#include "util/win/ntstatus_logging.h"
#include "util/win/process_info.h"
#include "util/win/registration_protocol_win.h"
#include "util/win/safe_terminate_process.h"
#include "util/win/scoped_process_suspend.h"
#include "util/win/termination_codes.h"
#include "util/win/xp_compat.h"
@ -126,7 +127,7 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) {
// here, rather than trying to signal to a handler that will never arrive,
// and then sleeping unnecessarily.
LOG(ERROR) << "crash server failed to launch, self-terminating";
TerminateProcess(GetCurrentProcess(), kTerminationCodeCrashNoDump);
SafeTerminateProcess(GetCurrentProcess(), kTerminationCodeCrashNoDump);
return EXCEPTION_CONTINUE_SEARCH;
}
@ -171,7 +172,7 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) {
LOG(ERROR) << "crash server did not respond, self-terminating";
TerminateProcess(GetCurrentProcess(), kTerminationCodeCrashNoDump);
SafeTerminateProcess(GetCurrentProcess(), kTerminationCodeCrashNoDump);
return EXCEPTION_CONTINUE_SEARCH;
}
@ -733,8 +734,8 @@ void CrashpadClient::DumpWithoutCrash(const CONTEXT& context) {
// Win32 APIs, so just use regular locking here in case of multiple threads
// calling this function. If a crash occurs while we're in here, the worst
// that can happen is that the server captures a partial dump for this path
// because on the other thread gathering a crash dump, it TerminateProcess()d,
// causing this one to abort.
// because another thread’s crash processing finished and the process was
// terminated before this thread’s non-crash processing could be completed.
base::AutoLock lock(*g_non_crash_dump_lock);
// Create a fake EXCEPTION_POINTERS to give the handler something to work
@ -777,8 +778,8 @@ void CrashpadClient::DumpWithoutCrash(const CONTEXT& context) {
void CrashpadClient::DumpAndCrash(EXCEPTION_POINTERS* exception_pointers) {
if (g_signal_exception == INVALID_HANDLE_VALUE) {
LOG(ERROR) << "not connected";
TerminateProcess(GetCurrentProcess(),
kTerminationCodeNotConnectedToHandler);
SafeTerminateProcess(GetCurrentProcess(),
kTerminationCodeNotConnectedToHandler);
return;
}

3
util/util.gyp
View File

@ -212,6 +212,8 @@
'win/process_structs.h',
'win/registration_protocol_win.cc',
'win/registration_protocol_win.h',
'win/safe_terminate_process.asm',
'win/safe_terminate_process.h',
'win/scoped_handle.cc',
'win/scoped_handle.h',
'win/scoped_local_alloc.cc',
@ -316,6 +318,7 @@
}, { # else: OS!="win"
'sources!': [
'win/capture_context.asm',
'win/safe_terminate_process.asm',
],
}],
['OS=="linux"', {

9
util/util_test.gyp
View File

@ -102,6 +102,7 @@
'win/initial_client_data_test.cc',
'win/process_info_test.cc',
'win/registration_protocol_win_test.cc',
'win/safe_terminate_process_test.cc',
'win/scoped_process_suspend_test.cc',
'win/session_end_watcher_test.cc',
'win/time_test.cc',
@ -117,6 +118,7 @@
['OS=="win"', {
'dependencies': [
'crashpad_util_test_process_info_test_child',
'crashpad_util_test_safe_terminate_process_test_child',
],
'link_settings': {
'libraries': [
@ -171,6 +173,13 @@
},
},
},
{
'target_name': 'crashpad_util_test_safe_terminate_process_test_child',
'type': 'executable',
'sources': [
'win/safe_terminate_process_test_child.cc',
],
},
]
}],
],

4
util/win/capture_context.asm
View File

@ -21,7 +21,7 @@ endif
ifdef _M_IX86
.586
.XMM
.xmm
.model flat
endif
@ -212,7 +212,7 @@ CONTEXT ends
endif
; namespace crashpad {
; void CaptureContext(CONTEXT* context)
; void CaptureContext(CONTEXT* context);
; } // namespace crashpad
ifdef _M_IX86
CAPTURECONTEXT_SYMBOL equ ?CaptureContext@crashpad@@YAXPAU_CONTEXT@@@Z

3
util/win/exception_handler_server.cc
View File

@ -34,6 +34,7 @@
#include "util/win/get_function.h"
#include "util/win/handle.h"
#include "util/win/registration_protocol_win.h"
#include "util/win/safe_terminate_process.h"
#include "util/win/xp_compat.h"
namespace crashpad {
@ -546,7 +547,7 @@ void __stdcall ExceptionHandlerServer::OnCrashDumpEvent(void* ctx, BOOLEAN) {
client->crash_exception_information_address(),
client->debug_critical_section_address());
TerminateProcess(client->process(), exit_code);
SafeTerminateProcess(client->process(), exit_code);
}
// static

74
util/win/safe_terminate_process.asm Normal file
View File

@ -0,0 +1,74 @@
; Copyright 2017 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.
; Detect ml64 assembling for x86_64 by checking for rax.
ifdef rax
_M_X64 equ 1
else
_M_IX86 equ 1
endif
ifdef _M_IX86
.586
.xmm
.model flat
includelib kernel32.lib
extern __imp__TerminateProcess@8:proc
; namespace crashpad {
; bool SafeTerminateProcess(HANDLE process, UINT exit_code);
; } // namespace crashpad
SAFETERMINATEPROCESS_SYMBOL equ ?SafeTerminateProcess@crashpad@@YA_NPAXI@Z
_TEXT segment
public SAFETERMINATEPROCESS_SYMBOL
SAFETERMINATEPROCESS_SYMBOL proc
; This function is written in assembler source because it’s important for it
; to not be inlined, for it to allocate a stack frame, and most critically,
; for it to not trust esp on return from TerminateProcess().
; __declspec(noinline) can prevent inlining and #pragma optimize("y", off) can
; disable frame pointer omission, but there’s no way to force a C compiler to
; distrust esp, and even if there was a way, it’d probably be fragile.
push ebp
mov ebp, esp
push [ebp+12]
push [ebp+8]
call dword ptr [__imp__TerminateProcess@8]
; Convert from BOOL to bool.
test eax, eax
setne al
; TerminateProcess() is supposed to be stdcall (callee clean-up), and esp and
; ebp are expected to already be equal. But if it’s been patched badly by
; something that’s cdecl (caller clean-up), this next move will get things
; back on track.
mov esp, ebp
pop ebp
ret
SAFETERMINATEPROCESS_SYMBOL endp
_TEXT ends
endif
end

55
util/win/safe_terminate_process.h Normal file
View File

@ -0,0 +1,55 @@
// Copyright 2017 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_WIN_SAFE_TERMINATE_PROCESS_H_
#define CRASHPAD_UTIL_WIN_SAFE_TERMINATE_PROCESS_H_
#include <windows.h>
#include "build/build_config.h"
namespace crashpad {
//! \brief Calls `TerminateProcess()`.
//!
//! `TerminateProcess()` has been observed in the wild as being patched badly on
//! 32-bit x86: it’s patched with code adhering to the `cdecl` (caller clean-up)
//! convention, although it’s supposed to be `stdcall` (callee clean-up). The
//! mix-up means that neither caller nor callee perform parameter clean-up from
//! the stack, causing the stack pointer to have an unexpected value on return
//! from the patched function. This typically results in a crash shortly
//! thereafter. See <a href="https://crashpad.chromium.org/bug/179">Crashpad bug
//! 179</a>.
//!
//! On 32-bit x86, this replacement function calls `TerminateProcess()` without
//! making any assumptions about the stack pointer on its return. As such, it’s
//! compatible with the badly patched `cdecl` version as well as the native
//! `stdcall` version (and other less badly patched versions).
//!
//! Elsewhere, this function calls `TerminateProcess()` directly without any
//! additional fanfare.
//!
//! Call this function instead of `TerminateProcess()` anywhere that
//! `TerminateProcess()` would normally be called.
bool SafeTerminateProcess(HANDLE process, UINT exit_code);
#if !defined(ARCH_CPU_X86)
inline bool SafeTerminateProcess(HANDLE process, UINT exit_code) {
return TerminateProcess(process, exit_code) != FALSE;
}
#endif // !ARCH_CPU_X86
} // namespace crashpad
#endif // CRASHPAD_UTIL_WIN_SAFE_TERMINATE_PROCESS_H_

185
util/win/safe_terminate_process_test.cc Normal file
View File

@ -0,0 +1,185 @@
// Copyright 2017 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/safe_terminate_process.h"
#include <string.h>
#include <string>
#include <memory>
#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/macros.h"
#include "build/build_config.h"
#include "gtest/gtest.h"
#include "test/errors.h"
#include "test/test_paths.h"
#include "test/win/child_launcher.h"
#include "util/win/scoped_handle.h"
namespace crashpad {
namespace test {
namespace {
// Patches executable code, saving a copy of the original code so that it can be
// restored on destruction.
class ScopedExecutablePatch {
public:
ScopedExecutablePatch(void* target, const void* source, size_t size)
: original_(new uint8_t[size]), target_(target), size_(size) {
memcpy(original_.get(), target_, size_);
ScopedVirtualProtectRWX protect_rwx(target_, size_);
memcpy(target_, source, size_);
}
~ScopedExecutablePatch() {
ScopedVirtualProtectRWX protect_rwx(target_, size_);
memcpy(target_, original_.get(), size_);
}
private:
// Sets the protection on (address, size) to PAGE_EXECUTE_READWRITE by calling
// VirtualProtect(), and restores the original protection on destruction. Note
// that the region may span multiple pages, but the first page’s original
// protection will be applied to the entire region on destruction. This
// shouldn’t be a problem in practice for patching a function for this test’s
// purposes.
class ScopedVirtualProtectRWX {
public:
// If either the constructor or destructor fails, PCHECK() to terminate
// immediately, because the process will be in a weird and untrustworthy
// state, and gtest error handling isn’t worthwhile at that point.
ScopedVirtualProtectRWX(void* address, size_t size)
: address_(address), size_(size) {
PCHECK(VirtualProtect(
address_, size_, PAGE_EXECUTE_READWRITE, &old_protect_))
<< "VirtualProtect";
}
~ScopedVirtualProtectRWX() {
DWORD last_protect_;
PCHECK(VirtualProtect(address_, size_, old_protect_, &last_protect_))
<< "VirtualProtect";
}
private:
void* address_;
size_t size_;
DWORD old_protect_;
DISALLOW_COPY_AND_ASSIGN(ScopedVirtualProtectRWX);
};
std::unique_ptr<uint8_t[]> original_;
void* target_;
size_t size_;
DISALLOW_COPY_AND_ASSIGN(ScopedExecutablePatch);
};
TEST(SafeTerminateProcess, PatchBadly) {
// This is a test of SafeTerminateProcess(), but it doesn’t actually terminate
// anything. Instead, it works with a process handle for the current process
// that doesn’t have PROCESS_TERMINATE access. The whole point of this test is
// to patch the real TerminateProcess() badly with a cdecl implementation to
// ensure that SafeTerminateProcess() can recover from such gross misconduct.
// The actual termination isn’t relevant to this test.
//
// Notably, don’t duplicate the process handle with PROCESS_TERMINATE access
// or with the DUPLICATE_SAME_ACCESS option. The SafeTerminateProcess() calls
// that follow operate on a duplicate of the current process’ process handle,
// and they’re supposed to fail, not terminate this process.
HANDLE process;
ASSERT_TRUE(DuplicateHandle(GetCurrentProcess(),
GetCurrentProcess(),
GetCurrentProcess(),
&process,
PROCESS_QUERY_INFORMATION,
false,
0))
<< ErrorMessage("DuplicateHandle");
ScopedKernelHANDLE process_owner(process);
// Make sure that TerminateProcess() works as a baseline.
SetLastError(ERROR_SUCCESS);
EXPECT_FALSE(TerminateProcess(process, 0));
EXPECT_EQ(GetLastError(), ERROR_ACCESS_DENIED);
// Make sure that SafeTerminateProcess() works, calling through to
// TerminateProcess() properly.
SetLastError(ERROR_SUCCESS);
EXPECT_FALSE(SafeTerminateProcess(process, 0));
EXPECT_EQ(GetLastError(), ERROR_ACCESS_DENIED);
{
// Patch TerminateProcess() badly. This turns it into a no-op that returns 0
// without cleaning up arguments from the stack, as a stdcall function is
// expected to do.
//
// This simulates the unexpected cdecl-patched TerminateProcess() as seen at
// https://crashpad.chromium.org/bug/179. In reality, this only affects
// 32-bit x86, as there’s no calling convention confusion on x86_64. It
// doesn’t hurt to run this test in the 64-bit environment, though.
const uint8_t patch[] = {
#if defined(ARCH_CPU_X86)
0x31, 0xc0, // xor eax, eax
#elif defined(ARCH_CPU_X86_64)
0x48, 0x31, 0xc0, // xor rax, rax
#else
#error Port
#endif
0xc3, // ret
};
void* target = reinterpret_cast<void*>(TerminateProcess);
ScopedExecutablePatch executable_patch(target, patch, arraysize(patch));
// Make sure that SafeTerminateProcess() can be called. Since it’s been
// patched with a no-op stub, GetLastError() shouldn’t be modified.
SetLastError(ERROR_SUCCESS);
EXPECT_FALSE(SafeTerminateProcess(process, 0));
EXPECT_EQ(GetLastError(), ERROR_SUCCESS);
}
// Now that the real TerminateProcess() has been restored, verify that it
// still works properly.
SetLastError(ERROR_SUCCESS);
EXPECT_FALSE(SafeTerminateProcess(process, 0));
EXPECT_EQ(GetLastError(), ERROR_ACCESS_DENIED);
}
TEST(SafeTerminateProcess, TerminateChild) {
base::FilePath test_executable = TestPaths::Executable();
std::wstring child_executable =
test_executable.DirName()
.Append(test_executable.BaseName().RemoveFinalExtension().value() +
L"_safe_terminate_process_test_child.exe")
.value();
ChildLauncher child(child_executable, std::wstring());
ASSERT_NO_FATAL_FAILURE(child.Start());
constexpr DWORD kExitCode = 0x51ee9d1e; // Sort of like “sleep and die.”
ASSERT_TRUE(SafeTerminateProcess(child.process_handle(), kExitCode))
<< ErrorMessage("TerminateProcess");
EXPECT_EQ(child.WaitForExit(), kExitCode);
}
} // namespace
} // namespace test
} // namespace crashpad

22
util/win/safe_terminate_process_test_child.cc Normal file
View File

@ -0,0 +1,22 @@
// Copyright 2017 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 <stdlib.h>
#include <wchar.h>
#include <windows.h>
int wmain(int argc, wchar_t** argv) {
Sleep(INFINITE);
return EXIT_FAILURE;
}

2
util/win/termination_codes.h
View File

@ -18,7 +18,7 @@
namespace crashpad {
//! \brief Crashpad-specific codes that are used as arguments to
//! `TerminateProcess()` in unusual circumstances.
//! SafeTerminateProcess() or `TerminateProcess()` in unusual circumstances.
enum TerminationCodes : unsigned int {
//! \brief The crash handler did not respond, and the client self-terminated.
kTerminationCodeCrashNoDump = 0xffff7001,