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

This reverts commit e77256b1.

Reason for revert: Reverting while I investigate failures causing recursive calls to hang.

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}

TBR=jorgelo@chromium.org,palmer@chromium.org,pcc@chromium.org,vtsyrklevich@chromium.org

Change-Id: I77e142638d73bd53de4b6fc1b9db2ffc819f6459
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 771365
Reviewed-on: https://chromium-review.googlesource.com/775594Reviewed-by: Peter Collingbourne <pcc@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517218}

content/zygote/zygote_main_linux.cc

@@ -23,8 +23,6 @@
 #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"
@@ -216,32 +214,23 @@ typedef struct tm* (*LocaltimeFunction)(const time_t* timep);
 typedef struct tm* (*LocaltimeRFunction)(const time_t* timep,
                                          struct tm* result);
 
-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 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;
 
 static void InitLibcLocaltimeFunctions() {
-  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) {
+  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) {
     // http://code.google.com/p/chromium/issues/detail?id=16800
     //
     // Nvidia's libGL.so overrides dlsym for an unknown reason and replaces
@@ -253,14 +242,14 @@ static void InitLibcLocaltimeFunctions() {
                   "http://code.google.com/p/chromium/issues/detail?id=16800";
   }
 
-  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;
+  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;
 }
 
 // Define localtime_override() function with asm name "localtime", so that all
@@ -280,9 +269,9 @@ struct tm* localtime_override(const time_t* timep) {
     return &time_struct;
   }
 
-  CHECK_EQ(0, pthread_once(&g_libc_funcs_guard, InitLibcLocaltimeFunctions));
-  struct tm* res =
-      base::UnsanitizedCfiCall(g_libc_funcs, &LibcFunctions::localtime)(timep);
+  CHECK_EQ(0, pthread_once(&g_libc_localtime_funcs_guard,
+                           InitLibcLocaltimeFunctions));
+  struct tm* res = g_libc_localtime(timep);
 #if defined(MEMORY_SANITIZER)
   if (res) __msan_unpoison(res, sizeof(*res));
   if (res->tm_zone) __msan_unpoison_string(res->tm_zone);
@@ -304,9 +293,9 @@ struct tm* localtime64_override(const time_t* timep) {
     return &time_struct;
   }
 
-  CHECK_EQ(0, pthread_once(&g_libc_funcs_guard, InitLibcLocaltimeFunctions));
-  struct tm* res = base::UnsanitizedCfiCall(g_libc_funcs,
-                                            &LibcFunctions::localtime64)(timep);
+  CHECK_EQ(0, pthread_once(&g_libc_localtime_funcs_guard,
+                           InitLibcLocaltimeFunctions));
+  struct tm* res = g_libc_localtime64(timep);
 #if defined(MEMORY_SANITIZER)
   if (res) __msan_unpoison(res, sizeof(*res));
   if (res->tm_zone) __msan_unpoison_string(res->tm_zone);
@@ -325,9 +314,9 @@ struct tm* localtime_r_override(const time_t* timep, struct tm* result) {
     return result;
   }
 
-  CHECK_EQ(0, pthread_once(&g_libc_funcs_guard, InitLibcLocaltimeFunctions));
-  struct tm* res = base::UnsanitizedCfiCall(
-      g_libc_funcs, &LibcFunctions::localtime_r)(timep, result);
+  CHECK_EQ(0, pthread_once(&g_libc_localtime_funcs_guard,
+                           InitLibcLocaltimeFunctions));
+  struct tm* res = g_libc_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);
@@ -346,9 +335,9 @@ struct tm* localtime64_r_override(const time_t* timep, struct tm* result) {
     return result;
   }
 
-  CHECK_EQ(0, pthread_once(&g_libc_funcs_guard, InitLibcLocaltimeFunctions));
-  struct tm* res = base::UnsanitizedCfiCall(
-      g_libc_funcs, &LibcFunctions::localtime64_r)(timep, result);
+  CHECK_EQ(0, pthread_once(&g_libc_localtime_funcs_guard,
+                           InitLibcLocaltimeFunctions));
+  struct tm* res = g_libc_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

@@ -162,6 +162,7 @@ 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