LoadAllImportsForDll before patching a function using IATPatchFunction

If a delayloaded function is patched using IATPatchFunction and then
later there are method calls to the same DLL, the delayloadhelper will
fully realize the IAT for that DLL, overwriting the patch. This change
forces all of the imports to get loaded before the patch is created.

The primary scenario for this is the GDI patch for GetFontData done in
InitializePDF(). This presents a problem, however, because this method
is currently called during startup for every child process, thus
negating the work done to make GDI delayloaded in the first place. As
such, this change also makes two optimizations to prevent losing the
delayload perf benefits:
1. For PDF (ppapi) processes, ensures InitializePDF's patches only run
on Win7 and Win8. This is done because Win10 uses different font
retrieval mechanisms because of win32k process mitigations.
2a. Ensures InitializePDF runs for ppapi processes. Since this
code is only actually needed for PDF rendering, there's no reason to
do it for every child process. The LoadAllImports + patching then
becomes pay-for-play when a PDF is being viewed.
2b. Ensures InitializePDF runs for printing utility processes which use
the GDI-based font information on all Windows versions.

Bug: 988164
Change-Id: I4d3c560a7c022c09251773c2a0d1cb4585da2145
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1731175
Commit-Queue: Cliff Smolinsky <cliffsmo@microsoft.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686696}

base/debug/profiler.cc

@@ -158,7 +158,7 @@ FunctionType FindFunctionInImports(const char* function_name) {
   base::win::PEImage image(CURRENT_MODULE());
 
   FunctionSearchContext ctx = { function_name, NULL };
-  image.EnumImportChunks(FindResolutionFunctionInImports, &ctx);
+  image.EnumImportChunks(FindResolutionFunctionInImports, &ctx, nullptr);
 
   return reinterpret_cast<FunctionType>(ctx.function);
 }

base/win/BUILD.gn

@@ -26,6 +26,7 @@ buildflag_header("base_win_buildflags") {
 
 static_library("pe_image") {
   sources = [
+    "current_module.h",
     "pe_image.cc",
     "pe_image.h",
   ]

base/win/iat_patch_function.cc

@@ -24,9 +24,9 @@ struct InterceptFunctionInformation {
 };
 
 void* GetIATFunction(IMAGE_THUNK_DATA* iat_thunk) {
-  if (NULL == iat_thunk) {
+  if (!iat_thunk) {
     NOTREACHED();
-    return NULL;
+    return nullptr;
   }
 
   // Works around the 64 bit portability warning:
@@ -48,22 +48,20 @@ bool InterceptEnumCallback(const base::win::PEImage& image, const char* module,
   InterceptFunctionInformation* intercept_information =
     reinterpret_cast<InterceptFunctionInformation*>(cookie);
 
-  if (NULL == intercept_information) {
+  if (!intercept_information) {
     NOTREACHED();
     return false;
   }
 
   DCHECK(module);
 
-  if ((0 == lstrcmpiA(module, intercept_information->imported_from_module)) &&
-     (NULL != name) &&
-     (0 == lstrcmpiA(name, intercept_information->function_name))) {
+  if (name && (0 == lstrcmpiA(name, intercept_information->function_name))) {
     // Save the old pointer.
-    if (NULL != intercept_information->old_function) {
+    if (intercept_information->old_function) {
       *(intercept_information->old_function) = GetIATFunction(iat);
     }
 
-    if (NULL != intercept_information->iat_thunk) {
+    if (intercept_information->iat_thunk) {
       *(intercept_information->iat_thunk) = iat;
     }
 
@@ -104,8 +102,8 @@ DWORD InterceptImportedFunction(HMODULE module_handle,
                                 const char* function_name, void* new_function,
                                 void** old_function,
                                 IMAGE_THUNK_DATA** iat_thunk) {
-  if ((NULL == module_handle) || (NULL == imported_from_module) ||
-     (NULL == function_name) || (NULL == new_function)) {
+  if (!module_handle || !imported_from_module || !function_name ||
+      !new_function) {
     NOTREACHED();
     return ERROR_INVALID_PARAMETER;
   }
@@ -127,10 +125,11 @@ DWORD InterceptImportedFunction(HMODULE module_handle,
 
   // First go through the IAT. If we don't find the import we are looking
   // for in IAT, search delay import table.
-  target_image.EnumAllImports(InterceptEnumCallback, &intercept_information);
+  target_image.EnumAllImports(InterceptEnumCallback, &intercept_information,
+                              imported_from_module);
   if (!intercept_information.finished_operation) {
-    target_image.EnumAllDelayImports(InterceptEnumCallback,
-                                     &intercept_information);
+    target_image.EnumAllDelayImports(
+        InterceptEnumCallback, &intercept_information, imported_from_module);
   }
 
   return intercept_information.return_code;
@@ -147,8 +146,7 @@ DWORD InterceptImportedFunction(HMODULE module_handle,
 DWORD RestoreImportedFunction(void* intercept_function,
                               void* original_function,
                               IMAGE_THUNK_DATA* iat_thunk) {
-  if ((NULL == intercept_function) || (NULL == original_function) ||
-      (NULL == iat_thunk)) {
+  if (!intercept_function || !original_function || !iat_thunk) {
     NOTREACHED();
     return ERROR_INVALID_PARAMETER;
   }
@@ -166,15 +164,10 @@ DWORD RestoreImportedFunction(void* intercept_function,
 
 }  // namespace
 
-IATPatchFunction::IATPatchFunction()
-    : module_handle_(NULL),
-      intercept_function_(NULL),
-      original_function_(NULL),
-      iat_thunk_(NULL) {
-}
+IATPatchFunction::IATPatchFunction() = default;
 
 IATPatchFunction::~IATPatchFunction() {
-  if (NULL != intercept_function_) {
+  if (intercept_function_) {
     DWORD error = Unpatch();
     DCHECK_EQ(static_cast<DWORD>(NO_ERROR), error);
   }
@@ -185,7 +178,7 @@ DWORD IATPatchFunction::Patch(const wchar_t* module,
                               const char* function_name,
                               void* new_function) {
   HMODULE module_handle = LoadLibraryW(module);
-  if (module_handle == NULL) {
+  if (!module_handle) {
     NOTREACHED();
     return GetLastError();
   }
@@ -205,9 +198,9 @@ DWORD IATPatchFunction::PatchFromModule(HMODULE module,
                                         const char* imported_from_module,
                                         const char* function_name,
                                         void* new_function) {
-  DCHECK_EQ(static_cast<void*>(NULL), original_function_);
-  DCHECK_EQ(static_cast<IMAGE_THUNK_DATA*>(NULL), iat_thunk_);
-  DCHECK_EQ(static_cast<void*>(NULL), intercept_function_);
+  DCHECK_EQ(nullptr, original_function_);
+  DCHECK_EQ(nullptr, iat_thunk_);
+  DCHECK_EQ(nullptr, intercept_function_);
   DCHECK(module);
 
   DWORD error = InterceptImportedFunction(module,
@@ -239,10 +232,10 @@ DWORD IATPatchFunction::Unpatch() {
   // not going to be any safer
   if (module_handle_)
     FreeLibrary(module_handle_);
-  module_handle_ = NULL;
-  intercept_function_ = NULL;
-  original_function_ = NULL;
-  iat_thunk_ = NULL;
+  module_handle_ = nullptr;
+  intercept_function_ = nullptr;
+  original_function_ = nullptr;
+  iat_thunk_ = nullptr;
 
   return error;
 }

base/win/iat_patch_function.h

@@ -67,10 +67,10 @@ class BASE_EXPORT IATPatchFunction {
 
 
  private:
-  HMODULE module_handle_;
-  void* intercept_function_;
-  void* original_function_;
-  IMAGE_THUNK_DATA* iat_thunk_;
+  HMODULE module_handle_ = nullptr;
+  void* intercept_function_ = nullptr;
+  void* original_function_ = nullptr;
+  IMAGE_THUNK_DATA* iat_thunk_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(IATPatchFunction);
 };

base/win/pe_image.cc

@@ -5,9 +5,12 @@
 // This file implements PEImage, a generic class to manipulate PE files.
 // This file was adapted from GreenBorder's Code.
 
+#include "base/win/pe_image.h"
+
+#include <delayimp.h>
 #include <stddef.h>
 
-#include "base/win/pe_image.h"
+#include "base/win/current_module.h"
 
 namespace base {
 namespace win {
@@ -139,8 +142,8 @@ PIMAGE_SECTION_HEADER PEImage::GetImageSectionHeaderByName(
 
   for (int i = 0; i < num_sections; i++) {
     PIMAGE_SECTION_HEADER section = GetSectionHeader(i);
-    if (0 == _strnicmp(reinterpret_cast<LPCSTR>(section->Name), section_name,
-                       sizeof(section->Name))) {
+    if (_strnicmp(reinterpret_cast<LPCSTR>(section->Name), section_name,
+                  sizeof(section->Name)) == 0) {
       ret = section;
       break;
     }
@@ -382,7 +385,8 @@ bool PEImage::EnumRelocs(EnumRelocsFunction callback, PVOID cookie) const {
 }
 
 bool PEImage::EnumImportChunks(EnumImportChunksFunction callback,
-                               PVOID cookie) const {
+                               PVOID cookie,
+                               LPCSTR target_module_name) const {
   DWORD size = GetImageDirectoryEntrySize(IMAGE_DIRECTORY_ENTRY_IMPORT);
   PIMAGE_IMPORT_DESCRIPTOR import = GetFirstImportChunk();
 
@@ -396,8 +400,11 @@ bool PEImage::EnumImportChunks(EnumImportChunksFunction callback,
     PIMAGE_THUNK_DATA iat = reinterpret_cast<PIMAGE_THUNK_DATA>(
                                 RVAToAddr(import->FirstThunk));
 
-    if (!callback(*this, module_name, name_table, iat, cookie))
-      return false;
+    if (target_module_name == nullptr ||
+        (lstrcmpiA(module_name, target_module_name) == 0)) {
+      if (!callback(*this, module_name, name_table, iat, cookie))
+        return false;
+    }
   }
 
   return true;
@@ -432,13 +439,16 @@ bool PEImage::EnumOneImportChunk(EnumImportsFunction callback,
   return true;
 }
 
-bool PEImage::EnumAllImports(EnumImportsFunction callback, PVOID cookie) const {
+bool PEImage::EnumAllImports(EnumImportsFunction callback,
+                             PVOID cookie,
+                             LPCSTR target_module_name) const {
   EnumAllImportsStorage temp = { callback, cookie };
-  return EnumImportChunks(ProcessImportChunk, &temp);
+  return EnumImportChunks(ProcessImportChunk, &temp, target_module_name);
 }
 
 bool PEImage::EnumDelayImportChunks(EnumDelayImportChunksFunction callback,
-                                    PVOID cookie) const {
+                                    PVOID cookie,
+                                    LPCSTR target_module_name) const {
   PVOID directory = GetImageDirectoryEntryAddr(
                         IMAGE_DIRECTORY_ENTRY_DELAY_IMPORT);
   DWORD size = GetImageDirectoryEntrySize(IMAGE_DIRECTORY_ENTRY_DELAY_IMPORT);
@@ -474,9 +484,24 @@ bool PEImage::EnumDelayImportChunks(EnumDelayImportChunksFunction callback,
           static_cast<uintptr_t>(delay_descriptor->rvaIAT));
     }
 
-    if (!callback(*this, delay_descriptor, module_name, name_table, iat,
-                  cookie))
-      return false;
+    if (target_module_name == nullptr ||
+        (lstrcmpiA(module_name, target_module_name) == 0)) {
+      if (target_module_name) {
+        // Ensure all imports are properly loaded for the target module so that
+        // the callback is operating on a fully-realized set of imports.
+        // This call only loads the imports for the module where this code is
+        // executing, so it is only helpful or meaningful to do this if the
+        // current module is the module whose IAT we are enumerating.
+        // Use the module_name as retrieved from the IAT because this method
+        // is case sensitive.
+        if (module_ == CURRENT_MODULE())
+          ::__HrLoadAllImportsForDll(module_name);
+      }
+
+      if (!callback(*this, delay_descriptor, module_name, name_table, iat,
+                    cookie))
+        return false;
+    }
   }
 
   return true;
@@ -519,9 +544,11 @@ bool PEImage::EnumOneDelayImportChunk(EnumImportsFunction callback,
 }
 
 bool PEImage::EnumAllDelayImports(EnumImportsFunction callback,
-                                  PVOID cookie) const {
+                                  PVOID cookie,
+                                  LPCSTR target_module_name) const {
   EnumAllImportsStorage temp = { callback, cookie };
-  return EnumDelayImportChunks(ProcessDelayImportChunk, &temp);
+  return EnumDelayImportChunks(ProcessDelayImportChunk, &temp,
+                               target_module_name);
 }
 
 bool PEImage::VerifyMagic() const {

base/win/pe_image.h

@@ -187,12 +187,20 @@ class PEImage {
   // Enumerates PE imports.
   // cookie is a generic cookie to pass to the callback.
   // Returns true on success.
-  bool EnumAllImports(EnumImportsFunction callback, PVOID cookie) const;
+  // Use |target_module_name| to ensure the callback is only invoked for the
+  // specified module.
+  bool EnumAllImports(EnumImportsFunction callback,
+                      PVOID cookie,
+                      LPCSTR target_module_name) const;
 
   // Enumerates PE import blocks.
   // cookie is a generic cookie to pass to the callback.
   // Returns true on success.
-  bool EnumImportChunks(EnumImportChunksFunction callback, PVOID cookie) const;
+  // Use |target_module_name| to ensure the callback is only invoked for the
+  // specified module.
+  bool EnumImportChunks(EnumImportChunksFunction callback,
+                        PVOID cookie,
+                        LPCSTR target_module_name) const;
 
   // Enumerates the imports from a single PE import block.
   // cookie is a generic cookie to pass to the callback.
@@ -201,17 +209,25 @@ class PEImage {
                           PIMAGE_THUNK_DATA name_table, PIMAGE_THUNK_DATA iat,
                           PVOID cookie) const;
 
-
   // Enumerates PE delay imports.
   // cookie is a generic cookie to pass to the callback.
   // Returns true on success.
-  bool EnumAllDelayImports(EnumImportsFunction callback, PVOID cookie) const;
+  // Use |target_module_name| to ensure the callback is only invoked for the
+  // specified module. If this parameter is non-null then all delayloaded
+  // imports are resolved when the target module is found.
+  bool EnumAllDelayImports(EnumImportsFunction callback,
+                           PVOID cookie,
+                           LPCSTR target_module_name) const;
 
   // Enumerates PE delay import blocks.
   // cookie is a generic cookie to pass to the callback.
   // Returns true on success.
+  // Use |target_module_name| to ensure the callback is only invoked for the
+  // specified module. If this parameter is non-null then all delayloaded
+  // imports are resolved when the target module is found.
   bool EnumDelayImportChunks(EnumDelayImportChunksFunction callback,
-                             PVOID cookie) const;
+                             PVOID cookie,
+                             LPCSTR target_module_name) const;
 
   // Enumerates imports from a single PE delay import block.
   // cookie is a generic cookie to pass to the callback.

base/win/pe_image_unittest.cc

@@ -115,29 +115,29 @@ TEST(PEImageTest, EnumeratesPE) {
   base::FilePath pe_image_test_path = GetPEImageTestPath();
 
 #if defined(ARCH_CPU_ARM64)
-  const int sections = 7;
-  const int imports_dlls = 3;
-  const int delay_dlls = 2;
-  const int exports = 3;
-  const int imports = 72;
-  const int delay_imports = 2;
-  const int relocs = 740;
+  const int kSections = 7;
+  const int kImportsDlls = 3;
+  const int kDelayDlls = 2;
+  const int kExports = 3;
+  const int kImports = 72;
+  const int kDelayImports = 2;
+  const int kRelocs = 740;
 #elif defined(ARCH_CPU_64_BITS)
-  const int sections = 6;
-  const int imports_dlls = 2;
-  const int delay_dlls = 2;
-  const int exports = 3;
-  const int imports = 70;
-  const int delay_imports = 2;
-  const int relocs = 976;
+  const int kSections = 6;
+  const int kImportsDlls = 2;
+  const int kDelayDlls = 2;
+  const int kExports = 3;
+  const int kImports = 70;
+  const int kDelayImports = 2;
+  const int kRelocs = 976;
 #else
-  const int sections = 5;
-  const int imports_dlls = 2;
-  const int delay_dlls = 2;
-  const int exports = 3;
-  const int imports = 66;
-  const int delay_imports = 2;
-  const int relocs = 2114;
+  const int kSections = 5;
+  const int kImportsDlls = 2;
+  const int kDelayDlls = 2;
+  const int kExports = 3;
+  const int kImports = 66;
+  const int kDelayImports = 2;
+  const int kRelocs = 2114;
 #endif
 
   ScopedNativeLibrary module(pe_image_test_path);
@@ -148,31 +148,100 @@ TEST(PEImageTest, EnumeratesPE) {
   EXPECT_TRUE(pe.VerifyMagic());
 
   pe.EnumSections(SectionsCallback, &count);
-  EXPECT_EQ(sections, count);
+  EXPECT_EQ(kSections, count);
 
   count = 0;
-  pe.EnumImportChunks(ImportChunksCallback, &count);
-  EXPECT_EQ(imports_dlls, count);
+  pe.EnumImportChunks(ImportChunksCallback, &count, nullptr);
+  EXPECT_EQ(kImportsDlls, count);
 
   count = 0;
-  pe.EnumDelayImportChunks(DelayImportChunksCallback, &count);
-  EXPECT_EQ(delay_dlls, count);
+  pe.EnumDelayImportChunks(DelayImportChunksCallback, &count, nullptr);
+  EXPECT_EQ(kDelayDlls, count);
 
   count = 0;
   pe.EnumExports(ExportsCallback, &count);
-  EXPECT_EQ(exports, count);
+  EXPECT_EQ(kExports, count);
 
   count = 0;
-  pe.EnumAllImports(ImportsCallback, &count);
-  EXPECT_EQ(imports, count);
+  pe.EnumAllImports(ImportsCallback, &count, nullptr);
+  EXPECT_EQ(kImports, count);
 
   count = 0;
-  pe.EnumAllDelayImports(ImportsCallback, &count);
-  EXPECT_EQ(delay_imports, count);
+  pe.EnumAllDelayImports(ImportsCallback, &count, nullptr);
+  EXPECT_EQ(kDelayImports, count);
 
   count = 0;
   pe.EnumRelocs(RelocsCallback, &count);
-  EXPECT_EQ(relocs, count);
+  EXPECT_EQ(kRelocs, count);
+}
+
+// Tests that we are able to enumerate stuff from a PE file, and that
+// the actual number of items found matches an expected value.
+TEST(PEImageTest, EnumeratesPEWithTargetModule) {
+  base::FilePath pe_image_test_path = GetPEImageTestPath();
+  const char kTargetModuleStatic[] = "user32.dll";
+  const char kTargetModuleDelay[] = "cfgmgr32.dll";
+
+#if defined(ARCH_CPU_ARM64)
+  const int kSections = 7;
+  const int kImportsDlls = 2;
+  const int kDelayDlls = 1;
+  const int kExports = 3;
+  const int kImports = 2;
+  const int kDelayImports = 1;
+  const int kRelocs = 740;
+#elif defined(ARCH_CPU_64_BITS)
+  const int kSections = 6;
+  const int kImportsDlls = 1;
+  const int kDelayDlls = 1;
+  const int kExports = 3;
+  const int kImports = 2;
+  const int kDelayImports = 1;
+  const int kRelocs = 976;
+#else
+  const int kSections = 5;
+  const int kImportsDlls = 1;
+  const int kDelayDlls = 1;
+  const int kExports = 3;
+  const int kImports = 2;
+  const int kDelayImports = 1;
+  const int kRelocs = 2114;
+#endif
+
+  ScopedNativeLibrary module(pe_image_test_path);
+  ASSERT_TRUE(module.is_valid());
+
+  PEImage pe(module.get());
+  int count = 0;
+  EXPECT_TRUE(pe.VerifyMagic());
+
+  pe.EnumSections(SectionsCallback, &count);
+  EXPECT_EQ(kSections, count);
+
+  count = 0;
+  pe.EnumImportChunks(ImportChunksCallback, &count, kTargetModuleStatic);
+  EXPECT_EQ(kImportsDlls, count);
+
+  count = 0;
+  pe.EnumDelayImportChunks(DelayImportChunksCallback, &count,
+                           kTargetModuleDelay);
+  EXPECT_EQ(kDelayDlls, count);
+
+  count = 0;
+  pe.EnumExports(ExportsCallback, &count);
+  EXPECT_EQ(kExports, count);
+
+  count = 0;
+  pe.EnumAllImports(ImportsCallback, &count, kTargetModuleStatic);
+  EXPECT_EQ(kImports, count);
+
+  count = 0;
+  pe.EnumAllDelayImports(ImportsCallback, &count, kTargetModuleDelay);
+  EXPECT_EQ(kDelayImports, count);
+
+  count = 0;
+  pe.EnumRelocs(RelocsCallback, &count);
+  EXPECT_EQ(kRelocs, count);
 }
 
 // Tests that we can locate an specific exported symbol, by name and by ordinal.

chrome/child/pdf_child_init.cc

@@ -7,34 +7,20 @@
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
+#include "base/command_line.h"
 #include "base/no_destructor.h"
 #include "base/win/current_module.h"
 #include "base/win/iat_patch_function.h"
 #include "base/win/windows_version.h"
 #include "content/public/child/child_thread.h"
+#include "content/public/common/content_switches.h"
+#include "services/service_manager/sandbox/switches.h"
 #endif
 
 namespace {
 
 #if defined(OS_WIN)
-HDC WINAPI CreateDCAPatch(LPCSTR driver_name,
-                          LPCSTR device_name,
-                          LPCSTR output,
-                          const void* init_data) {
-  DCHECK(std::string("DISPLAY") == std::string(driver_name));
-  DCHECK(!device_name);
-  DCHECK(!output);
-  DCHECK(!init_data);
-
-  // CreateDC fails behind the sandbox, but not CreateCompatibleDC.
-  return CreateCompatibleDC(NULL);
-}
-
-typedef DWORD (WINAPI* GetFontDataPtr) (HDC hdc,
-                                        DWORD table,
-                                        DWORD offset,
-                                        LPVOID buffer,
-                                        DWORD length);
+typedef decltype(::GetFontData)* GetFontDataPtr;
 GetFontDataPtr g_original_get_font_data = nullptr;
 
 
@@ -64,39 +50,41 @@ DWORD WINAPI GetFontDataPatch(HDC hdc,
 
 void InitializePDF() {
 #if defined(OS_WIN)
-  // Need to patch a few functions for font loading to work correctly. This can
-  // be removed once we switch PDF to use Skia
-  // (https://bugs.chromium.org/p/pdfium/issues/detail?id=11).
+  const base::CommandLine& command_line =
+      *base::CommandLine::ForCurrentProcess();
+  std::string process_type =
+      command_line.GetSwitchValueASCII(switches::kProcessType);
+  bool is_pdf_utility_process = false;
+  if (command_line.HasSwitch(service_manager::switches::kServiceSandboxType)) {
+    std::string service_sandbox_type = command_line.GetSwitchValueASCII(
+        service_manager::switches::kServiceSandboxType);
+    if (service_sandbox_type ==
+        service_manager::switches::kPdfCompositorSandbox) {
+      is_pdf_utility_process = true;
+    }
+  }
+  // On Win10, pdf does not use GDI fonts and does not need to run this
+  // initialization for the ppapi process. Printing does still use GDI for
+  // fonts on Win10 though.
+  if (is_pdf_utility_process ||
+      (process_type == switches::kPpapiPluginProcess &&
+       base::win::GetVersion() < base::win::Version::WIN10)) {
+    // Need to patch a few functions for font loading to work correctly. This
+    // can be removed once we switch PDF to use Skia
+    // (https://bugs.chromium.org/p/pdfium/issues/detail?id=11).
 #if defined(COMPONENT_BUILD)
-  HMODULE module = ::GetModuleHandleA("pdfium.dll");
-  DCHECK(module);
+    HMODULE module = ::GetModuleHandleA("pdfium.dll");
+    DCHECK(module);
 #else
-  HMODULE module = CURRENT_MODULE();
+    HMODULE module = CURRENT_MODULE();
 #endif  // defined(COMPONENT_BUILD)
 
-  // When GDI is delayloaded, calls into real GDI after these patches are
-  // created result in the delayload handler overwriting the patches when it
-  // resolves the delayed imports with the full import information. To
-  // workaround this, we first call into GDI so that the delayed imports get
-  // resolved, THEN we apply the patches.
-  // On Win10 this isn't necessary because these methods are not used.
-  // TODO(crbug.com/988164): Implement a generic IATPatchFunction fix to handle
-  // these situations.
-  // TODO(crbug.com/988169): This workaround shouldn't be necessary on Win8
-  // either, but is because the win32k process mitigations aren't working
-  // properly.
-  if (base::win::GetVersion() < base::win::Version::WIN10)
-    GetFontData(nullptr, 0, 0, nullptr, 0);
-
-  static base::NoDestructor<base::win::IATPatchFunction> patch_createdca;
-  patch_createdca->PatchFromModule(module, "gdi32.dll", "CreateDCA",
-                                   reinterpret_cast<void*>(CreateDCAPatch));
-
-  static base::NoDestructor<base::win::IATPatchFunction> patch_get_font_data;
-  patch_get_font_data->PatchFromModule(
-      module, "gdi32.dll", "GetFontData",
-      reinterpret_cast<void*>(GetFontDataPatch));
-  g_original_get_font_data = reinterpret_cast<GetFontDataPtr>(
-      patch_get_font_data->original_function());
+    static base::NoDestructor<base::win::IATPatchFunction> patch_get_font_data;
+    patch_get_font_data->PatchFromModule(
+        module, "gdi32.dll", "GetFontData",
+        reinterpret_cast<void*>(GetFontDataPatch));
+    g_original_get_font_data = reinterpret_cast<GetFontDataPtr>(
+        patch_get_font_data->original_function());
+  }
 #endif  // defined(OS_WIN)
 }

chrome/chrome_elf/hook_util/hook_util.cc

@@ -105,12 +105,6 @@ bool IATFindHookFuncCallback(const base::win::PEImage& image,
   if (hook_func_info == nullptr)
     return false;
 
-  // Check for the right module.
-  if (module == nullptr ||
-      ::strnicmp(module, hook_func_info->imported_from_module,
-                 ::strlen(module)) != 0)
-    return true;
-
   // Check for the right function.
   if (import_name == nullptr ||
       ::strnicmp(import_name, hook_func_info->function_name,
@@ -178,9 +172,11 @@ DWORD ApplyIATHook(HMODULE module_handle,
 
   // First go through the IAT. If we don't find the import we are looking
   // for in IAT, search delay import table.
-  target_image.EnumAllImports(IATFindHookFuncCallback, &hook_info);
+  target_image.EnumAllImports(IATFindHookFuncCallback, &hook_info,
+                              imported_from_module);
   if (!hook_info.finished_operation) {
-    target_image.EnumAllDelayImports(IATFindHookFuncCallback, &hook_info);
+    target_image.EnumAllDelayImports(IATFindHookFuncCallback, &hook_info,
+                                     imported_from_module);
   }
 
   return hook_info.return_code;

chrome/test/delayload/delayloads_unittest.cc

@@ -51,7 +51,8 @@ class DelayloadsTest : public testing::Test {
     ASSERT_TRUE(module_mmap.Initialize(module_path));
     base::win::PEImageAsData pe_image_data(
         reinterpret_cast<HMODULE>(const_cast<uint8_t*>(module_mmap.data())));
-    pe_image_data.EnumImportChunks(DelayloadsTest::ImportsCallback, imports);
+    pe_image_data.EnumImportChunks(DelayloadsTest::ImportsCallback, imports,
+                                   nullptr);
   }
 };
 
@@ -324,9 +325,9 @@ TEST_F(DelayloadsTest, DISABLED_ChromeElfDllLoadSanityTestImpl) {
   ASSERT_TRUE(base::PathService::Get(base::DIR_EXE, &dll));
   dll = dll.Append(L"chrome_elf.dll");
 
-  // We don't expect user32 to be loaded in chrome_elf_import_unittests. If this
+  // We don't expect user32 to be loaded in delayloads_unittests. If this
   // test case fails, then it means that a dependency on user32 has crept into
-  // the chrome_elf_imports_unittests executable, which needs to be removed.
+  // the delayloads_unittests executable, which needs to be removed.
   // NOTE: it may be a secondary dependency of another system DLL.  If so,
   // try adding a "/DELAYLOAD:<blah>.dll" to the build.gn file.
   ASSERT_EQ(nullptr, ::GetModuleHandle(L"user32.dll"));