Revert 255151 "Make chrome_elf use thunks instead of function po..."

(Crashy on today's canary)
> Make chrome_elf use thunks instead of function pointers.
> 
> 1. Add functionality to ServiceResolverThunk to copy a thunk without patching.
> 2. Move chrome_elf thunk-handling code to a common location.
> 3. Use a thunk instead of a f'n ptr for redirects.
> 
> BUG=334379
> 
> Review URL: https://codereview.chromium.org/183833004

TBR=caitkp@chromium.org

Review URL: https://codereview.chromium.org/189803007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@255703 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 133 insertions, 288 deletions

diff --git a/chrome_elf/blacklist/blacklist.cc b/chrome_elf/blacklist/blacklist.cc
index 19c7641..23a06b4 100644
--- a/chrome_elf/blacklist/blacklist.cc
+++ b/chrome_elf/blacklist/blacklist.cc
@@ -11,9 +11,9 @@
 #include "chrome_elf/blacklist/blacklist_interceptions.h"
 #include "chrome_elf/chrome_elf_constants.h"
 #include "chrome_elf/chrome_elf_util.h"
-#include "chrome_elf/thunk_getter.h"
 #include "sandbox/win/src/interception_internal.h"
 #include "sandbox/win/src/internal_types.h"
+#include "sandbox/win/src/sandbox_utils.h"
 #include "sandbox/win/src/service_resolver.h"
 
 // http://blogs.msdn.com/oldnewthing/archive/2004/10/25/247180.aspx
@@ -42,10 +42,109 @@ __declspec(allocate(".crthunk")) sandbox::ThunkData g_thunk_storage;
 
 namespace {
 
+enum Version {
+  VERSION_PRE_XP_SP2 = 0,  // Not supported.
+  VERSION_XP_SP2,
+  VERSION_SERVER_2003, // Also includes XP Pro x64 and Server 2003 R2.
+  VERSION_VISTA,       // Also includes Windows Server 2008.
+  VERSION_WIN7,        // Also includes Windows Server 2008 R2.
+  VERSION_WIN8,        // Also includes Windows Server 2012.
+  VERSION_WIN8_1,
+  VERSION_WIN_LAST,    // Indicates error condition.
+};
+
+// Whether a process is running under WOW64 (the wrapper that allows 32-bit
+// processes to run on 64-bit versions of Windows).  This will return
+// WOW64_DISABLED for both "32-bit Chrome on 32-bit Windows" and "64-bit
+// Chrome on 64-bit Windows".  WOW64_UNKNOWN means "an error occurred", e.g.
+// the process does not have sufficient access rights to determine this.
+enum WOW64Status {
+  WOW64_DISABLED,
+  WOW64_ENABLED,
+  WOW64_UNKNOWN,
+};
+
 // Record if the blacklist was successfully initialized so processes can easily
 // determine if the blacklist is enabled for them.
 bool g_blacklist_initialized = false;
 
+WOW64Status GetWOW64StatusForCurrentProcess() {
+  typedef BOOL (WINAPI* IsWow64ProcessFunc)(HANDLE, PBOOL);
+  IsWow64ProcessFunc is_wow64_process = reinterpret_cast<IsWow64ProcessFunc>(
+      GetProcAddress(GetModuleHandle(L"kernel32.dll"), "IsWow64Process"));
+  if (!is_wow64_process)
+    return WOW64_DISABLED;
+  BOOL is_wow64 = FALSE;
+  if (!(*is_wow64_process)(GetCurrentProcess(), &is_wow64))
+    return WOW64_UNKNOWN;
+  return is_wow64 ? WOW64_ENABLED : WOW64_DISABLED;
+}
+
+class OSInfo {
+ public:
+  struct VersionNumber {
+    int major;
+    int minor;
+    int build;
+  };
+
+  struct ServicePack {
+    int major;
+    int minor;
+  };
+
+  OSInfo() {
+    OSVERSIONINFOEX version_info = { sizeof(version_info) };
+    GetVersionEx(reinterpret_cast<OSVERSIONINFO*>(&version_info));
+    version_number_.major = version_info.dwMajorVersion;
+    version_number_.minor = version_info.dwMinorVersion;
+    version_number_.build = version_info.dwBuildNumber;
+    if ((version_number_.major == 5) && (version_number_.minor > 0)) {
+      // Treat XP Pro x64, Home Server, and Server 2003 R2 as Server 2003.
+      version_ = (version_number_.minor == 1) ? VERSION_XP_SP2 :
+                                                VERSION_SERVER_2003;
+      if (version_ == VERSION_XP_SP2 && version_info.wServicePackMajor < 2)
+        version_ = VERSION_PRE_XP_SP2;
+    } else if (version_number_.major == 6) {
+      switch (version_number_.minor) {
+        case 0:
+          // Treat Windows Server 2008 the same as Windows Vista.
+          version_ = VERSION_VISTA;
+          break;
+        case 1:
+          // Treat Windows Server 2008 R2 the same as Windows 7.
+          version_ = VERSION_WIN7;
+          break;
+        case 2:
+          // Treat Windows Server 2012 the same as Windows 8.
+          version_ = VERSION_WIN8;
+          break;
+        default:
+          version_ = VERSION_WIN8_1;
+          break;
+      }
+    } else if (version_number_.major > 6) {
+      version_ = VERSION_WIN_LAST;
+    } else {
+      version_ = VERSION_PRE_XP_SP2;
+    }
+
+    service_pack_.major = version_info.wServicePackMajor;
+    service_pack_.minor = version_info.wServicePackMinor;
+  }
+
+  Version version() const { return version_; }
+  VersionNumber version_number() const { return version_number_; }
+  ServicePack service_pack() const { return service_pack_; }
+
+ private:
+  Version version_;
+  VersionNumber version_number_;
+  ServicePack service_pack_;
+
+  DISALLOW_COPY_AND_ASSIGN(OSInfo);
+};
+
 // Record that the thunk setup completed succesfully and close the registry
 // key handle since it is no longer needed.
 void RecordSuccessfulThunkSetup(HKEY* key) {
@@ -244,16 +343,17 @@ bool Initialize(bool force) {
   if (!force && !LeaveSetupBeacon())
     return false;
 
-  // Tells the resolver to patch already patched functions.
-  const bool kRelaxed = true;
-
-  // Create a thunk via the appropriate ServiceResolver instance.
-  sandbox::ServiceResolverThunk* thunk = GetThunk(kRelaxed);
-
   // Don't try blacklisting on unsupported OS versions.
-  if (!thunk)
+  OSInfo os_info;
+  if (os_info.version() <= VERSION_PRE_XP_SP2)
     return false;
 
+  // Pseudo-handle, no need to close.
+  HANDLE current_process = ::GetCurrentProcess();
+
+  // Tells the resolver to patch already patched functions.
+  const bool kRelaxed = true;
+
   // Record that we are starting the thunk setup code.
   HKEY key = NULL;
   DWORD disposition = 0;
@@ -278,6 +378,26 @@ bool Initialize(bool force) {
     key = NULL;
   }
 
+  // Create a thunk via the appropriate ServiceResolver instance.
+  sandbox::ServiceResolverThunk* thunk = NULL;
+#if defined(_WIN64)
+  // Because Windows 8 and 8.1 have different stubs in 64-bit,
+  // ServiceResolverThunk can handle all the formats in 64-bit (instead only
+  // handling 1 like it does in 32-bit versions).
+  thunk = new sandbox::ServiceResolverThunk(current_process, kRelaxed);
+#else
+  if (GetWOW64StatusForCurrentProcess() == WOW64_ENABLED) {
+    if (os_info.version() >= VERSION_WIN8)
+      thunk = new sandbox::Wow64W8ResolverThunk(current_process, kRelaxed);
+    else
+      thunk = new sandbox::Wow64ResolverThunk(current_process, kRelaxed);
+  } else if (os_info.version() >= VERSION_WIN8) {
+    thunk = new sandbox::Win8ResolverThunk(current_process, kRelaxed);
+  } else {
+    thunk = new sandbox::ServiceResolverThunk(current_process, kRelaxed);
+  }
+#endif
+
   // Record that we have initialized the blacklist.
   g_blacklist_initialized = true;
 
diff --git a/chrome_elf/chrome_elf.gyp b/chrome_elf/chrome_elf.gyp
index 7eeae2b..165507d 100644
--- a/chrome_elf/chrome_elf.gyp
+++ b/chrome_elf/chrome_elf.gyp
@@ -124,8 +124,6 @@
       ],
       'dependencies': [
         'chrome_elf_common',
-        '../base/base.gyp:base_static',
-        '../sandbox/sandbox.gyp:sandbox',
       ],
     },
     {
@@ -152,8 +150,6 @@
         'chrome_elf_types.h',
         'chrome_elf_util.cc',
         'chrome_elf_util.h',
-        'thunk_getter.cc',
-        'thunk_getter.h',
       ],
     },
     {
diff --git a/chrome_elf/create_file/chrome_create_file_unittest.cc b/chrome_elf/create_file/chrome_create_file_unittest.cc
index e958aa9..e25b159 100644
--- a/chrome_elf/create_file/chrome_create_file_unittest.cc
+++ b/chrome_elf/create_file/chrome_create_file_unittest.cc
@@ -377,7 +377,9 @@ TEST_F(ChromeCreateFileTest, BypassTest) {
 }
 
 TEST_F(ChromeCreateFileTest, NtCreateFileAddressCheck) {
-  EXPECT_EQ(&g_nt_thunk_storage, g_ntdll_lookup["NtCreateFile"]);
+  HMODULE ntdll_handle = ::GetModuleHandle(L"ntdll.dll");
+  EXPECT_EQ(::GetProcAddress(ntdll_handle, "NtCreateFile"),
+            g_ntdll_lookup["NtCreateFile"]);
 }
 
 TEST_F(ChromeCreateFileTest, ReadWriteFromNtDll) {
diff --git a/chrome_elf/ntdll_cache.cc b/chrome_elf/ntdll_cache.cc
index 73b0e11..e550442 100644
--- a/chrome_elf/ntdll_cache.cc
+++ b/chrome_elf/ntdll_cache.cc
@@ -2,26 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome_elf/ntdll_cache.h"
-
 #include <stdint.h>
 #include <windows.h>
 
-#include "base/basictypes.h"
-#include "chrome_elf/thunk_getter.h"
-#include "sandbox/win/src/interception_internal.h"
-#include "sandbox/win/src/internal_types.h"
-#include "sandbox/win/src/service_resolver.h"
-
-// Allocate storage for thunks in a page of this module to save on doing
-// an extra allocation at run time.
-#pragma section(".crthunk", read, execute)
-__declspec(allocate(".crthunk")) sandbox::ThunkData g_nt_thunk_storage;
+#include "chrome_elf/ntdll_cache.h"
 
 FunctionLookupTable g_ntdll_lookup;
 
 void InitCache() {
-  HMODULE ntdll_handle = ::GetModuleHandle(sandbox::kNtdllName);
+  HMODULE ntdll_handle = ::GetModuleHandle(L"ntdll.dll");
 
   // To find the Export Address Table address, we start from the DOS header.
   // The module handle is actually the address of the header.
@@ -59,43 +48,4 @@ void InitCache() {
     FARPROC func_addr = reinterpret_cast<FARPROC>(func + base_addr);
     g_ntdll_lookup[std::string(name)] = func_addr;
   }
-
-  const bool kRelaxed = true;
-
-  // Create a thunk via the appropriate ServiceResolver instance.
-  sandbox::ServiceResolverThunk* thunk = GetThunk(kRelaxed);
-
-  if (thunk) {
-    BYTE* thunk_storage = reinterpret_cast<BYTE*>(&g_nt_thunk_storage);
-
-    // Mark the thunk storage as readable and writeable, since we
-    // ready to write to it.
-    DWORD old_protect = 0;
-    if (!::VirtualProtect(&g_nt_thunk_storage,
-                          sizeof(g_nt_thunk_storage),
-                          PAGE_EXECUTE_READWRITE,
-                          &old_protect)) {
-      return;
-    }
-
-    size_t storage_used = 0;
-    NTSTATUS ret = thunk->CopyThunk(::GetModuleHandle(sandbox::kNtdllName),
-                                    "NtCreateFile",
-                                    thunk_storage,
-                                    sizeof(sandbox::ThunkData),
-                                    &storage_used);
-    delete thunk;
-
-    // Ensure that the pointer to the old function can't be changed.
-    ::VirtualProtect(&g_nt_thunk_storage,
-                     sizeof(g_nt_thunk_storage),
-                     PAGE_EXECUTE_READ,
-                     &old_protect);
-
-    if (NT_SUCCESS(ret)) {
-      // Add an entry in the lookup table for the thunk.
-      g_ntdll_lookup["NtCreateFile"] =
-          reinterpret_cast<FARPROC>(&g_nt_thunk_storage);
-    }
-  }
 }
diff --git a/chrome_elf/ntdll_cache.h b/chrome_elf/ntdll_cache.h
index 5e4fb2b..4608cf19 100644
--- a/chrome_elf/ntdll_cache.h
+++ b/chrome_elf/ntdll_cache.h
@@ -7,15 +7,9 @@
 
 #include "chrome_elf/chrome_elf_types.h"
 
-namespace sandbox {
-struct ThunkData;
-}
-
 // Caches the addresses of all functions exported by ntdll in  |g_ntdll_lookup|.
 void InitCache();
 
 extern FunctionLookupTable g_ntdll_lookup;
 
-extern sandbox::ThunkData g_nt_thunk_storage;
-
 #endif  // CHROME_ELF_NTDLL_CACHE_H_
diff --git a/chrome_elf/thunk_getter.cc b/chrome_elf/thunk_getter.cc
deleted file mode 100644
index 8421e5e..0000000
--- a/chrome_elf/thunk_getter.cc
+++ /dev/null
@@ -1,142 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <stdint.h>
-#include <windows.h>
-
-#include "base/basictypes.h"
-#include "sandbox/win/src/interception_internal.h"
-#include "sandbox/win/src/internal_types.h"
-#include "sandbox/win/src/sandbox_utils.h"
-#include "sandbox/win/src/service_resolver.h"
-
-namespace {
-enum Version {
-  VERSION_PRE_XP_SP2 = 0,  // Not supported.
-  VERSION_XP_SP2,
-  VERSION_SERVER_2003,  // Also includes XP Pro x64 and Server 2003 R2.
-  VERSION_VISTA,        // Also includes Windows Server 2008.
-  VERSION_WIN7,         // Also includes Windows Server 2008 R2.
-  VERSION_WIN8,         // Also includes Windows Server 2012.
-  VERSION_WIN8_1,
-  VERSION_WIN_LAST,  // Indicates error condition.
-};
-
-// Whether a process is running under WOW64 (the wrapper that allows 32-bit
-// processes to run on 64-bit versions of Windows).  This will return
-// WOW64_DISABLED for both "32-bit Chrome on 32-bit Windows" and "64-bit
-// Chrome on 64-bit Windows".  WOW64_UNKNOWN means "an error occurred", e.g.
-// the process does not have sufficient access rights to determine this.
-enum WOW64Status { WOW64_DISABLED, WOW64_ENABLED, WOW64_UNKNOWN, };
-
-WOW64Status GetWOW64StatusForCurrentProcess() {
-  typedef BOOL(WINAPI * IsWow64ProcessFunc)(HANDLE, PBOOL);
-  IsWow64ProcessFunc is_wow64_process = reinterpret_cast<IsWow64ProcessFunc>(
-      GetProcAddress(GetModuleHandle(L"kernel32.dll"), "IsWow64Process"));
-  if (!is_wow64_process)
-    return WOW64_DISABLED;
-  BOOL is_wow64 = FALSE;
-  if (!is_wow64_process(GetCurrentProcess(), &is_wow64))
-    return WOW64_UNKNOWN;
-  return is_wow64 ? WOW64_ENABLED : WOW64_DISABLED;
-}
-
-class OSInfo {
- public:
-  struct VersionNumber {
-    int major;
-    int minor;
-    int build;
-  };
-
-  struct ServicePack {
-    int major;
-    int minor;
-  };
-
-  OSInfo() {
-    OSVERSIONINFOEX version_info = {sizeof(version_info)};
-    GetVersionEx(reinterpret_cast<OSVERSIONINFO*>(&version_info));
-    version_number_.major = version_info.dwMajorVersion;
-    version_number_.minor = version_info.dwMinorVersion;
-    version_number_.build = version_info.dwBuildNumber;
-    if ((version_number_.major == 5) && (version_number_.minor > 0)) {
-      // Treat XP Pro x64, Home Server, and Server 2003 R2 as Server 2003.
-      version_ =
-          (version_number_.minor == 1) ? VERSION_XP_SP2 : VERSION_SERVER_2003;
-      if (version_ == VERSION_XP_SP2 && version_info.wServicePackMajor < 2)
-        version_ = VERSION_PRE_XP_SP2;
-    } else if (version_number_.major == 6) {
-      switch (version_number_.minor) {
-        case 0:
-          // Treat Windows Server 2008 the same as Windows Vista.
-          version_ = VERSION_VISTA;
-          break;
-        case 1:
-          // Treat Windows Server 2008 R2 the same as Windows 7.
-          version_ = VERSION_WIN7;
-          break;
-        case 2:
-          // Treat Windows Server 2012 the same as Windows 8.
-          version_ = VERSION_WIN8;
-          break;
-        default:
-          version_ = VERSION_WIN8_1;
-          break;
-      }
-    } else if (version_number_.major > 6) {
-      version_ = VERSION_WIN_LAST;
-    } else {
-      version_ = VERSION_PRE_XP_SP2;
-    }
-
-    service_pack_.major = version_info.wServicePackMajor;
-    service_pack_.minor = version_info.wServicePackMinor;
-  }
-
-  Version version() const { return version_; }
-  VersionNumber version_number() const { return version_number_; }
-  ServicePack service_pack() const { return service_pack_; }
-
- private:
-  Version version_;
-  VersionNumber version_number_;
-  ServicePack service_pack_;
-
-  DISALLOW_COPY_AND_ASSIGN(OSInfo);
-};
-
-}  // namespace
-
-sandbox::ServiceResolverThunk* GetThunk(bool relaxed) {
-  // Create a thunk via the appropriate ServiceResolver instance.
-  sandbox::ServiceResolverThunk* thunk = NULL;
-
-  // No thunks for unsupported OS versions.
-  OSInfo os_info;
-  if (os_info.version() <= VERSION_PRE_XP_SP2)
-    return thunk;
-
-  // Pseudo-handle, no need to close.
-  HANDLE current_process = ::GetCurrentProcess();
-
-#if defined(_WIN64)
-  // ServiceResolverThunk can handle all the formats in 64-bit (instead only
-  // handling one like it does in 32-bit versions).
-  thunk = new sandbox::ServiceResolverThunk(current_process, relaxed);
-#else
-  if (GetWOW64StatusForCurrentProcess() == WOW64_ENABLED) {
-    if (os_info.version() >= VERSION_WIN8)
-      thunk = new sandbox::Wow64W8ResolverThunk(current_process, relaxed);
-    else
-      thunk = new sandbox::Wow64ResolverThunk(current_process, relaxed);
-  } else if (os_info.version() >= VERSION_WIN8) {
-    thunk = new sandbox::Win8ResolverThunk(current_process, relaxed);
-  } else {
-    thunk = new sandbox::ServiceResolverThunk(current_process, relaxed);
-  }
-#endif
-
-  return thunk;
-}
diff --git a/chrome_elf/thunk_getter.h b/chrome_elf/thunk_getter.h
deleted file mode 100644
index 5bc20fe..0000000
--- a/chrome_elf/thunk_getter.h
+++ /dev/null
@@ -1,16 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROME_ELF_THUNK_GETTER_H_
-#define CHROME_ELF_THUNK_GETTER_H_
-
-namespace sandbox {
-class ServiceResolverThunk;
-}
-
-// Creates a |ServiceResolverThunk| based on the OS version. Ownership of the
-// resulting thunk is passed to the caller.
-sandbox::ServiceResolverThunk* GetThunk(bool relaxed);
-
-#endif  // CHROME_ELF_THUNK_GETTER_H_
diff --git a/sandbox/win/src/service_resolver.h b/sandbox/win/src/service_resolver.h
index 20486c9..0089692 100644
--- a/sandbox/win/src/service_resolver.h
+++ b/sandbox/win/src/service_resolver.h
@@ -46,15 +46,6 @@ class ServiceResolverThunk : public ResolverThunk {
   // Call this to set up ntdll_base_ which will allow for local patches.
   virtual void AllowLocalPatches();
 
-  // Verifies that the function specified by |target_name| in |target_module| is
-  // a service and copies the data from that function into |thunk_storage|. If
-  // |storage_bytes| is too small, then the method fails.
-  virtual NTSTATUS CopyThunk(const void* target_module,
-                             const char* target_name,
-                             BYTE* thunk_storage,
-                             size_t storage_bytes,
-                             size_t* storage_used);
-
  protected:
   // The unit test will use this member to allow local patch on a buffer.
   HMODULE ntdll_base_;
diff --git a/sandbox/win/src/service_resolver_32.cc b/sandbox/win/src/service_resolver_32.cc
index 9b8bbf9..2e69dbc 100644
--- a/sandbox/win/src/service_resolver_32.cc
+++ b/sandbox/win/src/service_resolver_32.cc
@@ -179,32 +179,6 @@ size_t ServiceResolverThunk::GetThunkSize() const {
   return offsetof(ServiceFullThunk, internal_thunk) + GetInternalThunkSize();
 }
 
-NTSTATUS ServiceResolverThunk::CopyThunk(const void* target_module,
-                                         const char* target_name,
-                                         BYTE* thunk_storage,
-                                         size_t storage_bytes,
-                                         size_t* storage_used) {
-  NTSTATUS ret = ResolveTarget(target_module, target_name, &target_);
-  if (!NT_SUCCESS(ret))
-    return ret;
-
-  size_t thunk_bytes = GetThunkSize();
-  if (storage_bytes < thunk_bytes)
-    return STATUS_UNSUCCESSFUL;
-
-  ServiceFullThunk* thunk = reinterpret_cast<ServiceFullThunk*>(thunk_storage);
-
-  if (!IsFunctionAService(&thunk->original) &&
-      (!relaxed_ || !SaveOriginalFunction(&thunk->original, thunk_storage))) {
-    return STATUS_UNSUCCESSFUL;
-  }
-
-  if (NULL != storage_used)
-    *storage_used = thunk_bytes;
-
-  return ret;
-}
-
 bool ServiceResolverThunk::IsFunctionAService(void* local_thunk) const {
   ServiceEntry function_code;
   SIZE_T read;
diff --git a/sandbox/win/src/service_resolver_64.cc b/sandbox/win/src/service_resolver_64.cc
index 79c6b16..9612418 100644
--- a/sandbox/win/src/service_resolver_64.cc
+++ b/sandbox/win/src/service_resolver_64.cc
@@ -116,30 +116,6 @@ size_t ServiceResolverThunk::GetThunkSize() const {
   return sizeof(ServiceFullThunk);
 }
 
-NTSTATUS ServiceResolverThunk::CopyThunk(const void* target_module,
-                                         const char* target_name,
-                                         BYTE* thunk_storage,
-                                         size_t storage_bytes,
-                                         size_t* storage_used) {
-  NTSTATUS ret = ResolveTarget(target_module, target_name, &target_);
-  if (!NT_SUCCESS(ret))
-    return ret;
-
-  size_t thunk_bytes = GetThunkSize();
-  if (storage_bytes < thunk_bytes)
-    return STATUS_UNSUCCESSFUL;
-
-  ServiceFullThunk* thunk = reinterpret_cast<ServiceFullThunk*>(thunk_storage);
-
-  if (!IsFunctionAService(&thunk->original))
-    return STATUS_UNSUCCESSFUL;
-
-  if (NULL != storage_used)
-    *storage_used = thunk_bytes;
-
-  return ret;
-}
-
 bool ServiceResolverThunk::IsFunctionAService(void* local_thunk) const {
   ServiceFullThunk function_code;
   SIZE_T read;