Commit 2eb182fa authored by Cliff Smolinsky's avatar Cliff Smolinsky Committed by Commit Bot

Enable GDI patching only for processes which require it.

GDI patching (inconsistently) results in delayload crashes due to the
sandbox restrictions applied to various processes. In an effort to catch
all processes which need the patching, we applied it to all utility
processes. However, this has proven to be too inclusive, as many of
those processes have sandboxes which prevent this code from working
properly. This change detects the sandbox type and only executes the
patching if the sandbox type is ppapi, PrintCompositor, or PdfConversion.

Bug: 1028670
Change-Id: I651f50d073a94bf3a7c1cfa9f01aa21761c68132
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2083384Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Reviewed-by: default avatarWill Harris <wfh@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarGreg Thompson <grt@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748400}
parent b30e2d76
...@@ -1036,10 +1036,6 @@ void ChromeMainDelegate::PreSandboxStartup() { ...@@ -1036,10 +1036,6 @@ void ChromeMainDelegate::PreSandboxStartup() {
locale; locale;
} }
#if !defined(CHROME_MULTIPLE_DLL_BROWSER) && BUILDFLAG(ENABLE_PDF)
InitializePDF();
#endif
#if defined(OS_POSIX) && !defined(OS_MACOSX) #if defined(OS_POSIX) && !defined(OS_MACOSX)
// Zygote needs to call InitCrashReporter() in RunZygote(). // Zygote needs to call InitCrashReporter() in RunZygote().
if (process_type != service_manager::switches::kZygoteProcess) { if (process_type != service_manager::switches::kZygoteProcess) {
...@@ -1066,6 +1062,10 @@ void ChromeMainDelegate::PreSandboxStartup() { ...@@ -1066,6 +1062,10 @@ void ChromeMainDelegate::PreSandboxStartup() {
// After all the platform Breakpads have been initialized, store the command // After all the platform Breakpads have been initialized, store the command
// line for crash reporting. // line for crash reporting.
crash_keys::SetCrashKeysFromCommandLine(command_line); crash_keys::SetCrashKeysFromCommandLine(command_line);
#if !defined(CHROME_MULTIPLE_DLL_BROWSER) && BUILDFLAG(ENABLE_PDF)
MaybeInitializeGDI();
#endif
} }
void ChromeMainDelegate::SandboxInitialized(const std::string& process_type) { void ChromeMainDelegate::SandboxInitialized(const std::string& process_type) {
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/win/windows_version.h" #include "base/win/windows_version.h"
#include "content/public/child/child_thread.h" #include "content/public/child/child_thread.h"
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "services/service_manager/sandbox/sandbox_type.h"
#include "services/service_manager/sandbox/switches.h" #include "services/service_manager/sandbox/switches.h"
#endif #endif
...@@ -48,35 +49,23 @@ DWORD WINAPI GetFontDataPatch(HDC hdc, ...@@ -48,35 +49,23 @@ DWORD WINAPI GetFontDataPatch(HDC hdc,
} // namespace } // namespace
void InitializePDF() { void MaybeInitializeGDI() {
#if defined(OS_WIN) #if defined(OS_WIN)
const base::CommandLine& command_line = const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess(); *base::CommandLine::ForCurrentProcess();
const std::string process_type = const std::string process_type =
command_line.GetSwitchValueASCII(switches::kProcessType); command_line.GetSwitchValueASCII(switches::kProcessType);
// Patch utility processes, which includes ones that do PDF to EMF conversion. // Patch utility processes which explicitly need GDI. Anything else, just
// They are hard to differentiate because they can also be launched from // return.
// chrome/service/ in a different manner vs. from chrome/browser/. service_manager::SandboxType service_sandbox_type =
bool needs_gdi32_patching = process_type == switches::kUtilityProcess; service_manager::SandboxTypeFromCommandLine(command_line);
if (!(service_sandbox_type == service_manager::SandboxType::kPpapi ||
if (!needs_gdi32_patching) { service_sandbox_type ==
// Windows prior to Win10 use GDI fonts in the PDF PPAPI process. service_manager::SandboxType::kPrintCompositor ||
needs_gdi32_patching = process_type == switches::kPpapiPluginProcess && service_sandbox_type == service_manager::SandboxType::kPdfConversion)) {
base::win::GetVersion() < base::win::Version::WIN10;
}
if (!needs_gdi32_patching) {
// Printing uses GDI for fonts on all versions of Windows.
// TODO(thestig): Check and see if this is actually necessary.
std::string service_sandbox_type = command_line.GetSwitchValueASCII(
service_manager::switches::kServiceSandboxType);
needs_gdi32_patching = service_sandbox_type ==
service_manager::switches::kPrintCompositorSandbox;
}
if (!needs_gdi32_patching)
return; return;
}
#if defined(COMPONENT_BUILD) #if defined(COMPONENT_BUILD)
HMODULE module = ::GetModuleHandleA("pdfium.dll"); HMODULE module = ::GetModuleHandleA("pdfium.dll");
......
...@@ -12,6 +12,6 @@ ...@@ -12,6 +12,6 @@
#endif #endif
// Initializes child-process specific code for the PDF module. // Initializes child-process specific code for the PDF module.
void InitializePDF(); void MaybeInitializeGDI();
#endif // CHROME_CHILD_PDF_CHILD_INIT_H_ #endif // CHROME_CHILD_PDF_CHILD_INIT_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment