Reland "[cfi-icall] Use ProtectedMemory for localtime ptrs"

This is a reland of e77256b1 now that
crrev.com/00fb4b7e landed fixing the lock-up
issue we previously saw.

Original change's description:
> [cfi-icall] Use ProtectedMemory for localtime ptrs
>
> Control Flow Integrity [1] indirect call (cfi-icall) checking can not
> verify that dynamically resolved function pointers call their intended
> function. Instead we place the LibcFunctions pointers in
> ProtectedMemory, a wrapper for keeping variables in read-only memory
> except for when they are initialized.  After setting the pointers in
> protected memory we can use the UnsanitizedCfiCall wrapper to disable
> cfi-icall checking when calling them since we know they can not be
> tampered with.
>
> [1] https://www.chromium.org/developers/testing/control-flow-integrity
>
> Bug: 771365
> Change-Id: Ib74faff066e1107293b67d11f2a1a054bbff08b5
> Reviewed-on: https://chromium-review.googlesource.com/769853
> Reviewed-by: Chris Palmer <palmer@chromium.org>
> Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
> Reviewed-by: Peter Collingbourne <pcc@chromium.org>
> Commit-Queue: Peter Collingbourne <pcc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#517152}

Bug: 771365
Change-Id: Iaba5de8e37332878cd0b85c9dbdafc08e2d9a6ba
Reviewed-on: https://chromium-review.googlesource.com/782739Reviewed-by: Chris Palmer <palmer@chromium.org>
Reviewed-by: Peter Collingbourne <pcc@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518373}

content/zygote/zygote_main_linux.cc

@@ -23,6 +23,8 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/lazy_instance.h"
+#include "base/memory/protected_memory.h"
+#include "base/memory/protected_memory_cfi.h"
 #include "base/native_library.h"
 #include "base/pickle.h"
 #include "base/posix/eintr_wrapper.h"
@@ -214,23 +216,32 @@ typedef struct tm* (*LocaltimeFunction)(const time_t* timep);
 typedef struct tm* (*LocaltimeRFunction)(const time_t* timep,
                                          struct tm* result);
 
-static pthread_once_t g_libc_localtime_funcs_guard = PTHREAD_ONCE_INIT;
-static LocaltimeFunction g_libc_localtime;
-static LocaltimeFunction g_libc_localtime64;
-static LocaltimeRFunction g_libc_localtime_r;
-static LocaltimeRFunction g_libc_localtime64_r;
+struct LibcFunctions {
+  LocaltimeFunction localtime;
+  LocaltimeFunction localtime64;
+  LocaltimeRFunction localtime_r;
+  LocaltimeRFunction localtime64_r;
+};
+
+static pthread_once_t g_libc_funcs_guard = PTHREAD_ONCE_INIT;
+// The libc function pointers are stored in read-only memory after being
+// dynamically resolved as a security mitigation to prevent the pointer from
+// being tampered with. See crbug.com/771365 for details.
+static PROTECTED_MEMORY_SECTION base::ProtectedMemory<LibcFunctions>
+    g_libc_funcs;
 
 static void InitLibcLocaltimeFunctions() {
-  g_libc_localtime = reinterpret_cast<LocaltimeFunction>(
-      dlsym(RTLD_NEXT, "localtime"));
-  g_libc_localtime64 = reinterpret_cast<LocaltimeFunction>(
-      dlsym(RTLD_NEXT, "localtime64"));
-  g_libc_localtime_r = reinterpret_cast<LocaltimeRFunction>(
-      dlsym(RTLD_NEXT, "localtime_r"));
-  g_libc_localtime64_r = reinterpret_cast<LocaltimeRFunction>(
-      dlsym(RTLD_NEXT, "localtime64_r"));
-
-  if (!g_libc_localtime || !g_libc_localtime_r) {
+  auto writer = base::AutoWritableMemory::Create(g_libc_funcs);
+  g_libc_funcs->localtime =
+      reinterpret_cast<LocaltimeFunction>(dlsym(RTLD_NEXT, "localtime"));
+  g_libc_funcs->localtime64 =
+      reinterpret_cast<LocaltimeFunction>(dlsym(RTLD_NEXT, "localtime64"));
+  g_libc_funcs->localtime_r =
+      reinterpret_cast<LocaltimeRFunction>(dlsym(RTLD_NEXT, "localtime_r"));
+  g_libc_funcs->localtime64_r =
+      reinterpret_cast<LocaltimeRFunction>(dlsym(RTLD_NEXT, "localtime64_r"));
+
+  if (!g_libc_funcs->localtime || !g_libc_funcs->localtime_r) {
     // http://code.google.com/p/chromium/issues/detail?id=16800
     //
     // Nvidia's libGL.so overrides dlsym for an unknown reason and replaces
@@ -242,14 +253,14 @@ static void InitLibcLocaltimeFunctions() {
                   "http://code.google.com/p/chromium/issues/detail?id=16800";
   }
 
-  if (!g_libc_localtime)
-    g_libc_localtime = gmtime;
-  if (!g_libc_localtime64)
-    g_libc_localtime64 = g_libc_localtime;
-  if (!g_libc_localtime_r)
-    g_libc_localtime_r = gmtime_r;
-  if (!g_libc_localtime64_r)
-    g_libc_localtime64_r = g_libc_localtime_r;
+  if (!g_libc_funcs->localtime)
+    g_libc_funcs->localtime = gmtime;
+  if (!g_libc_funcs->localtime64)
+    g_libc_funcs->localtime64 = g_libc_funcs->localtime;
+  if (!g_libc_funcs->localtime_r)
+    g_libc_funcs->localtime_r = gmtime_r;
+  if (!g_libc_funcs->localtime64_r)
+    g_libc_funcs->localtime64_r = g_libc_funcs->localtime_r;
 }
 
 // Define localtime_override() function with asm name "localtime", so that all
@@ -269,9 +280,9 @@ struct tm* localtime_override(const time_t* timep) {
     return &time_struct;
   }
 
-  CHECK_EQ(0, pthread_once(&g_libc_localtime_funcs_guard,
-                           InitLibcLocaltimeFunctions));
-  struct tm* res = g_libc_localtime(timep);
+  CHECK_EQ(0, pthread_once(&g_libc_funcs_guard, InitLibcLocaltimeFunctions));
+  struct tm* res =
+      base::UnsanitizedCfiCall(g_libc_funcs, &LibcFunctions::localtime)(timep);
 #if defined(MEMORY_SANITIZER)
   if (res) __msan_unpoison(res, sizeof(*res));
   if (res->tm_zone) __msan_unpoison_string(res->tm_zone);
@@ -293,9 +304,9 @@ struct tm* localtime64_override(const time_t* timep) {
     return &time_struct;
   }
 
-  CHECK_EQ(0, pthread_once(&g_libc_localtime_funcs_guard,
-                           InitLibcLocaltimeFunctions));
-  struct tm* res = g_libc_localtime64(timep);
+  CHECK_EQ(0, pthread_once(&g_libc_funcs_guard, InitLibcLocaltimeFunctions));
+  struct tm* res = base::UnsanitizedCfiCall(g_libc_funcs,
+                                            &LibcFunctions::localtime64)(timep);
 #if defined(MEMORY_SANITIZER)
   if (res) __msan_unpoison(res, sizeof(*res));
   if (res->tm_zone) __msan_unpoison_string(res->tm_zone);
@@ -314,9 +325,9 @@ struct tm* localtime_r_override(const time_t* timep, struct tm* result) {
     return result;
   }
 
-  CHECK_EQ(0, pthread_once(&g_libc_localtime_funcs_guard,
-                           InitLibcLocaltimeFunctions));
-  struct tm* res = g_libc_localtime_r(timep, result);
+  CHECK_EQ(0, pthread_once(&g_libc_funcs_guard, InitLibcLocaltimeFunctions));
+  struct tm* res = base::UnsanitizedCfiCall(
+      g_libc_funcs, &LibcFunctions::localtime_r)(timep, result);
 #if defined(MEMORY_SANITIZER)
   if (res) __msan_unpoison(res, sizeof(*res));
   if (res->tm_zone) __msan_unpoison_string(res->tm_zone);
@@ -335,9 +346,9 @@ struct tm* localtime64_r_override(const time_t* timep, struct tm* result) {
     return result;
   }
 
-  CHECK_EQ(0, pthread_once(&g_libc_localtime_funcs_guard,
-                           InitLibcLocaltimeFunctions));
-  struct tm* res = g_libc_localtime64_r(timep, result);
+  CHECK_EQ(0, pthread_once(&g_libc_funcs_guard, InitLibcLocaltimeFunctions));
+  struct tm* res = base::UnsanitizedCfiCall(
+      g_libc_funcs, &LibcFunctions::localtime64_r)(timep, result);
 #if defined(MEMORY_SANITIZER)
   if (res) __msan_unpoison(res, sizeof(*res));
   if (res->tm_zone) __msan_unpoison_string(res->tm_zone);

tools/cfi/blacklist.txt

@@ -164,7 +164,6 @@ src:*ui/accessibility/platform/ax_platform_node_auralinux.cc
 
 src:*chrome/browser/ui/zoom/chrome_zoom_level_prefs.cc
 src:*third_party/webrtc/modules/desktop_capture/x11/x_server_pixel_buffer.cc
-src:*content/zygote/zygote_main_linux.cc
 src:*media/cdm/*
 src:*third_party/swiftshader/*
 src:*base/native_library_unittest.cc