From 359fc4a1336db10c8c9aae86f495ae415289b5b1 Mon Sep 17 00:00:00 2001 From: James Forshaw Date: Tue, 3 Dec 2019 10:20:34 -0800 Subject: [PATCH] [Windows] Add checks for DLL loader lock. This CL adds code to check if the current thread holds the DLL loader lock. This code can be used to enforce the requirement that certain parts of crashpad, such as process creation are not done during calls to DllMain which can lead to deadlocks and crashes. Only one check is current enforced, in client process creation, and only in debug builds. Bug: crashpad: 316 Change-Id: I5757a264bbf28ce2ab88a0cd7ac9481e46428c17 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1945993 Reviewed-by: Mark Mentovai Reviewed-by: Scott Graham Commit-Queue: James Forshaw --- client/crashpad_client_win.cc | 3 ++ util/BUILD.gn | 14 +++++++++ util/win/loader_lock.cc | 52 ++++++++++++++++++++++++++++++++ util/win/loader_lock.h | 25 +++++++++++++++ util/win/loader_lock_test.cc | 36 ++++++++++++++++++++++ util/win/loader_lock_test_dll.cc | 41 +++++++++++++++++++++++++ 6 files changed, 171 insertions(+) create mode 100644 util/win/loader_lock.cc create mode 100644 util/win/loader_lock.h create mode 100644 util/win/loader_lock_test.cc create mode 100644 util/win/loader_lock_test_dll.cc diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index c1c3ff14..4963f24f 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -41,6 +41,7 @@ #include "util/win/get_function.h" #include "util/win/handle.h" #include "util/win/initial_client_data.h" +#include "util/win/loader_lock.h" #include "util/win/nt_internals.h" #include "util/win/ntstatus_logging.h" #include "util/win/process_info.h" @@ -346,6 +347,8 @@ class ScopedCallSetHandlerStartupState { bool StartHandlerProcess( std::unique_ptr data) { + CHECK(!IsThreadInLoaderLock()); + ScopedCallSetHandlerStartupState scoped_startup_state_caller; std::wstring command_line; diff --git a/util/BUILD.gn b/util/BUILD.gn index 02558909..fcf559be 100644 --- a/util/BUILD.gn +++ b/util/BUILD.gn @@ -371,6 +371,8 @@ static_library("util") { "win/handle.h", "win/initial_client_data.cc", "win/initial_client_data.h", + "win/loader_lock.cc", + "win/loader_lock.h", "win/module_version.cc", "win/module_version.h", "win/nt_internals.cc", @@ -666,6 +668,7 @@ source_set("util_test") { "win/get_function_test.cc", "win/handle_test.cc", "win/initial_client_data_test.cc", + "win/loader_lock_test.cc", "win/process_info_test.cc", "win/registration_protocol_win_test.cc", "win/safe_terminate_process_test.cc", @@ -718,6 +721,7 @@ source_set("util_test") { "dbghelp.lib", ] data_deps += [ + ":crashpad_util_test_loader_lock_test", ":crashpad_util_test_process_info_test_child", ":crashpad_util_test_safe_terminate_process_test_child", ] @@ -738,4 +742,14 @@ if (crashpad_is_win) { "win/safe_terminate_process_test_child.cc", ] } + + crashpad_loadable_module("crashpad_util_test_loader_lock_test") { + testonly = true + sources = [ + "win/loader_lock_test_dll.cc", + ] + deps = [ + ":util", + ] + } } diff --git a/util/win/loader_lock.cc b/util/win/loader_lock.cc new file mode 100644 index 00000000..0c9a3cc5 --- /dev/null +++ b/util/win/loader_lock.cc @@ -0,0 +1,52 @@ +// Copyright 2019 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/loader_lock.h" + +#include + +#include "build/build_config.h" +#include "util/win/process_structs.h" + +namespace crashpad { + +namespace { + +#ifdef ARCH_CPU_64_BITS +using NativeTraits = process_types::internal::Traits64; +#else +using NativeTraits = process_types::internal::Traits32; +#endif // ARCH_CPU_64_BITS + +using PEB = process_types::PEB; +using TEB = process_types::TEB; +using RTL_CRITICAL_SECTION = process_types::RTL_CRITICAL_SECTION; + +TEB* GetTeb() { + return reinterpret_cast(NtCurrentTeb()); +} + +PEB* GetPeb() { + return reinterpret_cast(GetTeb()->ProcessEnvironmentBlock); +} + +} // namespace + +bool IsThreadInLoaderLock() { + RTL_CRITICAL_SECTION* loader_lock = + reinterpret_cast(GetPeb()->LoaderLock); + return loader_lock->OwningThread == GetTeb()->ClientId.UniqueThread; +} + +} // namespace crashpad diff --git a/util/win/loader_lock.h b/util/win/loader_lock.h new file mode 100644 index 00000000..3ee34bb4 --- /dev/null +++ b/util/win/loader_lock.h @@ -0,0 +1,25 @@ +// Copyright 2019 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_LOADER_LOCK_H_ +#define CRASHPAD_UTIL_WIN_LOADER_LOCK_H_ + +namespace crashpad { + +//! \return `true` if the current thread holds the loader lock. +bool IsThreadInLoaderLock(); + +} // namespace crashpad + +#endif // CRASHPAD_UTIL_WIN_LOADER_LOCK_H_ diff --git a/util/win/loader_lock_test.cc b/util/win/loader_lock_test.cc new file mode 100644 index 00000000..9fa32f87 --- /dev/null +++ b/util/win/loader_lock_test.cc @@ -0,0 +1,36 @@ +// Copyright 2019 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/loader_lock.h" + +#include "gtest/gtest.h" +#include "util/win/get_function.h" + +extern "C" bool LoaderLockDetected(); + +namespace crashpad { +namespace test { +namespace { + +TEST(LoaderLock, Detected) { + EXPECT_FALSE(IsThreadInLoaderLock()); + auto* loader_lock_detected = GET_FUNCTION_REQUIRED( + L"crashpad_util_test_loader_lock_test.dll", LoaderLockDetected); + EXPECT_TRUE(loader_lock_detected()); + EXPECT_FALSE(IsThreadInLoaderLock()); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/util/win/loader_lock_test_dll.cc b/util/win/loader_lock_test_dll.cc new file mode 100644 index 00000000..fd1bfe03 --- /dev/null +++ b/util/win/loader_lock_test_dll.cc @@ -0,0 +1,41 @@ +// Copyright 2019 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 "util/win/loader_lock.h" + +namespace { + +bool g_loader_lock_detected = false; + +} // namespace + +extern "C" { + +__declspec(dllexport) bool LoaderLockDetected() { + return g_loader_lock_detected; +} + +} // extern "C" + +BOOL WINAPI DllMain(HINSTANCE, DWORD reason, LPVOID) { + switch (reason) { + case DLL_PROCESS_ATTACH: + g_loader_lock_detected = crashpad::IsThreadInLoaderLock(); + break; + } + + return TRUE; +}