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 <[email protected]> Reviewed-by: Mark Mentovai <[email protected]>
chromium · Sep 10, 2020 · 3e065b1 · 3e065b1
1 parent 9a5a789
commit 3e065b1
Show file tree

Hide file tree

Showing 16 changed files with 290 additions and 112 deletions.
diff --git 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 @@ __attribute__((visibility("default"))) int pthread_create(
     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
@@ -62,7 +62,7 @@ template("compat_target") {
   } 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 @@ compat_target("compat") {
 
   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 @@ compat_target("compat") {
     "..: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
diff --git a/compat/android/android/api-level.h b/compat/android/android/api-level.h
diff --git 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
@@ -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 @@ extern "C" {
 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
@@ -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
@@ -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
@@ -181,6 +181,8 @@ if (crashpad_is_android) {
 
     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
@@ -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 @@ int main(int argc, char* argv[]) {
   }
 
   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
@@ -575,6 +575,7 @@ static_library("util") {
   }
 
   public_deps = [
+    ":no_cfi_icall",
     "../compat",
     "../third_party/zlib",
   ]
@@ -668,6 +669,14 @@ if (!crashpad_is_android && !crashpad_is_ios) {
   }
 }
 
+# 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 @@ source_set("util_test") {
     "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
@@ -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 @@ namespace {
 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