linux, mac: disable cfi-icall for cross-dso calls
CFI attempts to verify that the dynamic type of a function object
matches the static type of the function pointer used to call it.
https://clang.llvm.org/docs/ControlFlowIntegrity.html#indirect-function-call-checking
However, the analyzer does not have enough information to check
cross-dso calls. In these instances, CFI crashes upon calling the
function with an error like:
pthread_create_linux.cc:60:16: runtime error:
control flow integrity check for type
'int (unsigned long *, const pthread_attr_t *, void *(*)(void *), void *)'
failed during indirect function call
(/lib/x86_64-linux-gnu/libpthread.so.0+0x9200):
note: (unknown) defined here pthread_create_linux.cc:60:16:
note: check failed in crashpad_handler,
destination function located in /lib/x86_64-linux-gnu/libpthread.so.0
Change-Id: Ib29dabfe714f2ee9cc06a5d17e6899ff81a06df4
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2339332
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
diff --git a/client/pthread_create_linux.cc b/client/pthread_create_linux.cc
index d5a60fe..df7aac4 100644
--- a/client/pthread_create_linux.cc
+++ b/client/pthread_create_linux.cc
@@ -17,6 +17,7 @@
#include "base/logging.h"
#include "client/crashpad_client.h"
+#include "util/misc/no_cfi_icall.h"
namespace {
@@ -45,13 +46,12 @@
const pthread_attr_t* attr,
StartRoutineType start_routine,
void* arg) {
- static const auto next_pthread_create = []() {
- const auto next_pthread_create =
- reinterpret_cast<decltype(pthread_create)*>(
- dlsym(RTLD_NEXT, "pthread_create"));
- CHECK(next_pthread_create) << "dlsym: " << dlerror();
- return next_pthread_create;
- }();
+ static const crashpad::NoCfiIcall<decltype(pthread_create)*>
+ next_pthread_create([]() {
+ const auto next_pthread_create = dlsym(RTLD_NEXT, "pthread_create");
+ CHECK(next_pthread_create) << "dlsym: " << dlerror();
+ return next_pthread_create;
+ }());
StartParams* params = new StartParams;
params->start_routine = start_routine;
diff --git a/compat/BUILD.gn b/compat/BUILD.gn
index d98a6a6..6154f8f 100644
--- a/compat/BUILD.gn
+++ b/compat/BUILD.gn
@@ -62,7 +62,7 @@
} else {
static_library(target_name) {
forward_variables_from(invoker, "*", [ "configs" ])
- if (!(defined(configs))) {
+ if (!defined(configs)) {
configs = []
}
if (defined(invoker.configs)) {
@@ -116,8 +116,6 @@
if (crashpad_is_android) {
sources += [
- "android/android/api-level.cc",
- "android/android/api-level.h",
"android/dlfcn_internal.cc",
"android/dlfcn_internal.h",
"android/elf.h",
@@ -166,7 +164,7 @@
"..:crashpad_config",
]
- deps = []
+ deps = [ "../util:no_cfi_icall" ]
if (!crashpad_is_mac) {
deps += [ "../third_party/xnu" ]
diff --git a/compat/android/android/api-level.cc b/compat/android/android/api-level.cc
deleted file mode 100644
index c29ec1a..0000000
--- a/compat/android/android/api-level.cc
+++ /dev/null
@@ -1,50 +0,0 @@
-// Copyright 2018 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 <android/api-level.h>
-
-#include <dlfcn.h>
-#include <limits.h>
-#include <stdlib.h>
-#include <sys/system_properties.h>
-
-#include "dlfcn_internal.h"
-
-#if __NDK_MAJOR__ < 20
-
-extern "C" {
-
-int android_get_device_api_level() {
- using FuncType = int (*)();
- static const FuncType bionic_get_device_api_level =
- reinterpret_cast<FuncType>(
- crashpad::internal::Dlsym(RTLD_NEXT, "android_get_device_api_level"));
-
- if (bionic_get_device_api_level) {
- return bionic_get_device_api_level();
- }
-
- char api_string[PROP_VALUE_MAX];
- int length = __system_property_get("ro.build.version.sdk", api_string);
- if (length <= 0) {
- return -1;
- }
-
- int api_level = atoi(api_string);
- return api_level > 0 ? api_level : -1;
-}
-
-} // extern "C"
-
-#endif // __NDK_MAJOR__ < 20
diff --git a/compat/android/android/api-level.h b/compat/android/android/api-level.h
deleted file mode 100644
index bfff9af..0000000
--- a/compat/android/android/api-level.h
+++ /dev/null
@@ -1,39 +0,0 @@
-// Copyright 2018 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_COMPAT_ANDROID_ANDROID_API_LEVEL_H_
-#define CRASHPAD_COMPAT_ANDROID_ANDROID_API_LEVEL_H_
-
-#include_next <android/api-level.h>
-#include <android/ndk-version.h>
-
-#include <sys/cdefs.h>
-
-#if __NDK_MAJOR__ < 20
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-// Returns the API level of the device or -1 if it can't be determined. This
-// function is provided by NDK r20.
-int android_get_device_api_level();
-
-#ifdef __cplusplus
-} // extern "C"
-#endif
-
-#endif // __NDK_MAJOR__ < 20
-
-#endif // CRASHPAD_COMPAT_ANDROID_ANDROID_API_LEVEL_H_
diff --git a/compat/android/sys/epoll.cc b/compat/android/sys/epoll.cc
index 7fd3bb3..6c1a476 100644
--- a/compat/android/sys/epoll.cc
+++ b/compat/android/sys/epoll.cc
@@ -19,13 +19,14 @@
#include <unistd.h>
#include "dlfcn_internal.h"
+#include "util/misc/no_cfi_icall.h"
#if __ANDROID_API__ < 21
extern "C" {
int epoll_create1(int flags) {
- static const auto epoll_create1_p = reinterpret_cast<int (*)(int)>(
+ static const crashpad::NoCfiIcall<decltype(epoll_create1)*> epoll_create1_p(
crashpad::internal::Dlsym(RTLD_DEFAULT, "epoll_create1"));
return epoll_create1_p ? epoll_create1_p(flags)
: syscall(SYS_epoll_create1, flags);
diff --git a/compat/android/sys/mman_mmap.cc b/compat/android/sys/mman_mmap.cc
index d890d84..c929cfe 100644
--- a/compat/android/sys/mman_mmap.cc
+++ b/compat/android/sys/mman_mmap.cc
@@ -20,6 +20,7 @@
#include <unistd.h>
#include "dlfcn_internal.h"
+#include "util/misc/no_cfi_icall.h"
#if defined(__USE_FILE_OFFSET64) && __ANDROID_API__ < 21
@@ -88,8 +89,7 @@
void* mmap(void* addr, size_t size, int prot, int flags, int fd, off_t offset) {
// Use the system’s mmap64() wrapper if available. It will be available on
// Android 5.0 (“Lollipop”) and later.
- using Mmap64Type = void* (*)(void*, size_t, int, int, int, off64_t);
- static const Mmap64Type mmap64 = reinterpret_cast<Mmap64Type>(
+ static const crashpad::NoCfiIcall<decltype(LocalMmap64)*> mmap64(
crashpad::internal::Dlsym(RTLD_DEFAULT, "mmap64"));
if (mmap64) {
return mmap64(addr, size, prot, flags, fd, offset);
diff --git a/compat/compat.gyp b/compat/compat.gyp
index 1c2eeaf..693f2c6 100644
--- a/compat/compat.gyp
+++ b/compat/compat.gyp
@@ -19,9 +19,13 @@
'targets': [
{
'target_name': 'crashpad_compat',
+ 'dependencies': [
+ '../util/no_cfi_icall.gyp:no_cfi_icall',
+ ],
+ 'include_dirs': [
+ '..',
+ ],
'sources': [
- 'android/android/api-level.cc',
- 'android/android/api-level.h',
'android/dlfcn_internal.cc',
'android/dlfcn_internal.h',
'android/elf.h',
diff --git a/compat/linux/sys/mman_memfd_create.cc b/compat/linux/sys/mman_memfd_create.cc
index 12aaa2c..e0ee5a4 100644
--- a/compat/linux/sys/mman_memfd_create.cc
+++ b/compat/linux/sys/mman_memfd_create.cc
@@ -18,14 +18,15 @@
#include <sys/syscall.h>
#include <unistd.h>
+#include "util/misc/no_cfi_icall.h"
+
#if defined(__GLIBC__)
extern "C" {
int memfd_create(const char* name, unsigned int flags) {
- using MemfdCreateType = int (*)(const char*, int);
- static const MemfdCreateType next_memfd_create =
- reinterpret_cast<MemfdCreateType>(dlsym(RTLD_NEXT, "memfd_create"));
+ static const crashpad::NoCfiIcall<decltype(memfd_create)*> next_memfd_create(
+ dlsym(RTLD_NEXT, "memfd_create"));
return next_memfd_create ? next_memfd_create(name, flags)
: syscall(SYS_memfd_create, name, flags);
}
diff --git a/handler/BUILD.gn b/handler/BUILD.gn
index dc48178..332117d 100644
--- a/handler/BUILD.gn
+++ b/handler/BUILD.gn
@@ -181,6 +181,8 @@
sources = [ "linux/handler_trampoline.cc" ]
+ deps = [ "../util:no_cfi_icall" ]
+
ldflags = [ "-llog" ]
if (crashpad_is_in_chromium) {
diff --git a/handler/linux/handler_trampoline.cc b/handler/linux/handler_trampoline.cc
index 3453563..1596d77 100644
--- a/handler/linux/handler_trampoline.cc
+++ b/handler/linux/handler_trampoline.cc
@@ -16,6 +16,8 @@
#include <dlfcn.h>
#include <stdlib.h>
+#include "util/misc/no_cfi_icall.h"
+
// The first argument passed to the trampoline is the name of the native library
// exporting the symbol `CrashpadHandlerMain`. The remaining arguments are the
// same as for `HandlerMain()`.
@@ -34,8 +36,8 @@
}
using MainType = int (*)(int, char*[]);
- MainType crashpad_main =
- reinterpret_cast<MainType>(dlsym(handle, "CrashpadHandlerMain"));
+ const crashpad::NoCfiIcall<MainType> crashpad_main(
+ dlsym(handle, "CrashpadHandlerMain"));
if (!crashpad_main) {
__android_log_print(ANDROID_LOG_FATAL, kTag, "dlsym: %s", dlerror());
return EXIT_FAILURE;
diff --git a/util/BUILD.gn b/util/BUILD.gn
index 2585caa..81e6205 100644
--- a/util/BUILD.gn
+++ b/util/BUILD.gn
@@ -575,6 +575,7 @@
}
public_deps = [
+ ":no_cfi_icall",
"../compat",
"../third_party/zlib",
]
@@ -668,6 +669,14 @@
}
}
+# This exists as a separate target from util so that compat may depend on it
+# without cycles.
+source_set("no_cfi_icall") {
+ sources = [ "misc/no_cfi_icall.h" ]
+ public_deps = [ "../third_party/mini_chromium:base" ]
+ public_configs = [ "..:crashpad_config" ]
+}
+
source_set("util_test") {
testonly = true
@@ -685,6 +694,7 @@
"misc/from_pointer_cast_test.cc",
"misc/initialization_state_dcheck_test.cc",
"misc/initialization_state_test.cc",
+ "misc/no_cfi_icall_test.cc",
"misc/paths_test.cc",
"misc/random_string_test.cc",
"misc/range_set_test.cc",
diff --git a/util/mach/exception_types.cc b/util/mach/exception_types.cc
index 7fc33f0..9c9bbd4 100644
--- a/util/mach/exception_types.cc
+++ b/util/mach/exception_types.cc
@@ -26,6 +26,7 @@
#include "base/mac/mach_logging.h"
#include "util/mac/mac_util.h"
#include "util/mach/mach_extensions.h"
+#include "util/misc/no_cfi_icall.h"
#include "util/numeric/in_range_cast.h"
#if __MAC_OS_X_VERSION_MAX_ALLOWED >= __MAC_10_9
@@ -84,8 +85,8 @@
int ProcGetWakemonParams(pid_t pid, int* rate_hz, int* flags) {
#if __MAC_OS_X_VERSION_MAX_ALLOWED < __MAC_10_9
// proc_get_wakemon_params() isn’t in the SDK. Look it up dynamically.
- static ProcGetWakemonParamsType proc_get_wakemon_params =
- GetProcGetWakemonParams();
+ static crashpad::NoCfiIcall<ProcGetWakemonParamsType> proc_get_wakemon_params(
+ GetProcGetWakemonParams());
#endif
#if __MAC_OS_X_VERSION_MIN_REQUIRED < __MAC_10_9
diff --git a/util/misc/no_cfi_icall.h b/util/misc/no_cfi_icall.h
new file mode 100644
index 0000000..ebc632c
--- /dev/null
+++ b/util/misc/no_cfi_icall.h
@@ -0,0 +1,125 @@
+// Copyright 2020 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_NO_CFI_ICALL_H_
+#define CRASHPAD_UTIL_MISC_NO_CFI_ICALL_H_
+
+#include <type_traits>
+#include <utility>
+
+#include "base/compiler_specific.h"
+#include "base/macros.h"
+#include "build/build_config.h"
+
+#if defined(OS_WIN)
+#include <windows.h>
+#endif // OS_WIN
+
+namespace crashpad {
+
+namespace {
+
+template <typename Functor>
+struct FunctorTraits;
+
+template <typename R, typename... Args>
+struct FunctorTraits<R (*)(Args...)> {
+ template <typename Function, typename... RunArgs>
+ DISABLE_CFI_ICALL static R Invoke(Function&& function, RunArgs&&... args) {
+ return std::forward<Function>(function)(std::forward<RunArgs>(args)...);
+ }
+};
+
+template <typename R, typename... Args>
+struct FunctorTraits<R (*)(Args..., ...)> {
+ template <typename Function, typename... RunArgs>
+ DISABLE_CFI_ICALL static R Invoke(Function&& function, RunArgs&&... args) {
+ return std::forward<Function>(function)(std::forward<RunArgs>(args)...);
+ }
+};
+
+#if defined(OS_WIN) && defined(ARCH_CPU_X86)
+template <typename R, typename... Args>
+struct FunctorTraits<R(__stdcall*)(Args...)> {
+ template <typename... RunArgs>
+ DISABLE_CFI_ICALL static R Invoke(R(__stdcall* function)(Args...),
+ RunArgs&&... args) {
+ return function(std::forward<RunArgs>(args)...);
+ }
+};
+#endif // OS_WIN && ARCH_CPU_X86
+
+} // namespace
+
+//! \brief Disables cfi-icall for calls made through a function pointer.
+//!
+//! Clang provides several Control-Flow-Integrity (CFI) sanitizers, among them,
+//! cfi-icall, which attempts to verify that the dynamic type of a function
+//! matches the static type of the function pointer used to call it.
+//!
+//! https://clang.llvm.org/docs/ControlFlowIntegrity.html#indirect-function-call-checking
+//!
+//! However, cfi-icall does not have enough information to check indirect calls
+//! to functions in other modules, such as through the pointers returned by
+//! `dlsym()`. In these cases, CFI aborts the program upon executing the
+//! indirect call.
+//!
+//! This class encapsulates cross-DSO function pointers to disable cfi-icall
+//! precisely when calling these pointers.
+template <typename Functor>
+class NoCfiIcall {
+ public:
+ //! \brief Constructs this object.
+ //!
+ //! \param function A pointer to the function to be called.
+ explicit NoCfiIcall(Functor function) : function_(function) {}
+
+ //! \see NoCfiIcall
+ template <typename PointerType,
+ typename = std::enable_if_t<
+ std::is_same<typename std::remove_cv<PointerType>::type,
+ void*>::value>>
+ explicit NoCfiIcall(PointerType function)
+ : function_(reinterpret_cast<Functor>(function)) {}
+
+#if defined(OS_WIN)
+ //! \see NoCfiIcall
+ template <typename = std::enable_if_t<
+ !std::is_same<typename std::remove_cv<Functor>::type,
+ FARPROC>::value>>
+ explicit NoCfiIcall(FARPROC function)
+ : function_(reinterpret_cast<Functor>(function)) {}
+#endif // OS_WIN
+
+ ~NoCfiIcall() = default;
+
+ //! \brief Calls the function without sanitization by cfi-icall.
+ template <typename... RunArgs>
+ decltype(auto) operator()(RunArgs&&... args) const {
+ return FunctorTraits<Functor>::Invoke(function_,
+ std::forward<RunArgs>(args)...);
+ }
+
+ //! \brief Returns `true` if not `nullptr`.
+ operator bool() const { return function_ != nullptr; }
+
+ private:
+ Functor function_;
+
+ DISALLOW_COPY_AND_ASSIGN(NoCfiIcall);
+};
+
+} // namespace crashpad
+
+#endif // CRASHPAD_UTIL_MISC_NO_CFI_ICALL_H_
diff --git a/util/misc/no_cfi_icall_test.cc b/util/misc/no_cfi_icall_test.cc
new file mode 100644
index 0000000..680edbb
--- /dev/null
+++ b/util/misc/no_cfi_icall_test.cc
@@ -0,0 +1,83 @@
+// Copyright 2020 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/misc/no_cfi_icall.h"
+
+#include <stdio.h>
+
+#include "build/build_config.h"
+#include "gtest/gtest.h"
+
+#if defined(OS_WIN)
+#include <windows.h>
+
+#include "util/win/get_function.h"
+#else
+#include <dlfcn.h>
+#include <sys/types.h>
+#include <unistd.h>
+#endif
+
+namespace crashpad {
+namespace test {
+namespace {
+
+TEST(NoCfiIcall, NullptrIsFalse) {
+ NoCfiIcall<void (*)(void)> call(nullptr);
+ ASSERT_FALSE(call);
+}
+
+TEST(NoCfiIcall, SameDSOICall) {
+ static int (*func)() = []() { return 42; };
+ NoCfiIcall<decltype(func)> call(func);
+ ASSERT_TRUE(call);
+ ASSERT_EQ(call(), 42);
+}
+
+TEST(NoCfiIcall, CrossDSOICall) {
+#if defined(OS_WIN)
+ static const NoCfiIcall<decltype(GetCurrentProcessId)*> call(
+ GET_FUNCTION_REQUIRED(L"kernel32.dll", GetCurrentProcessId));
+ ASSERT_TRUE(call);
+ EXPECT_EQ(call(), GetCurrentProcessId());
+#else
+ static const NoCfiIcall<decltype(getpid)*> call(dlsym(RTLD_NEXT, "getpid"));
+ ASSERT_TRUE(call);
+ EXPECT_EQ(call(), getpid());
+#endif
+}
+
+TEST(NoCfiIcall, Args) {
+#if !defined(OS_WIN)
+ static const NoCfiIcall<decltype(snprintf)*> call(
+ dlsym(RTLD_NEXT, "snprintf"));
+ ASSERT_TRUE(call);
+
+ char buf[1024];
+
+ // Regular args.
+ memset(buf, 0, sizeof(buf));
+ ASSERT_GT(call(buf, sizeof(buf), "Hello!"), 0);
+ EXPECT_STREQ(buf, "Hello!");
+
+ // Variadic args.
+ memset(buf, 0, sizeof(buf));
+ ASSERT_GT(call(buf, sizeof(buf), "%s, %s!", "Hello", "World"), 0);
+ EXPECT_STREQ(buf, "Hello, World!");
+#endif
+}
+
+} // namespace
+} // namespace test
+} // namespace crashpad
diff --git a/util/no_cfi_icall.gyp b/util/no_cfi_icall.gyp
new file mode 100644
index 0000000..cfb53a7
--- /dev/null
+++ b/util/no_cfi_icall.gyp
@@ -0,0 +1,38 @@
+# Copyright 2020 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.
+
+{
+ 'includes': [
+ '../build/crashpad.gypi',
+ ],
+ 'targets': [
+ {
+ 'target_name': 'no_cfi_icall',
+ 'type': 'static_library',
+ 'dependencies': [
+ '../third_party/mini_chromium/mini_chromium.gyp:base',
+ ],
+ 'include_dirs': [
+ '..',
+ '<(INTERMEDIATE_DIR)',
+ ],
+ 'sources': [
+ 'misc/no_cfi_icall.h',
+ ],
+ 'export_dependent_settings': [
+ '../third_party/mini_chromium/mini_chromium.gyp:base',
+ ],
+ },
+ ],
+}
diff --git a/util/util_test.gyp b/util/util_test.gyp
index 749c532..339bb4a 100644
--- a/util/util_test.gyp
+++ b/util/util_test.gyp
@@ -21,6 +21,7 @@
'target_name': 'crashpad_util_test',
'type': 'executable',
'dependencies': [
+ 'no_cfi_icall.gyp:no_cfi_icall',
'util.gyp:crashpad_util',
'../client/client.gyp:crashpad_client',
'../compat/compat.gyp:crashpad_compat',
@@ -79,6 +80,7 @@
'misc/from_pointer_cast_test.cc',
'misc/initialization_state_dcheck_test.cc',
'misc/initialization_state_test.cc',
+ 'misc/no_cfi_icall_test.cc',
'misc/paths_test.cc',
'misc/scoped_forbid_return_test.cc',
'misc/random_string_test.cc',
