Don't set shared memory hooks for the Cloud Print service process.

Bug: 1061138
Change-Id: Ie4e452dd78b207dfd86fb8da2cc7f0575c907f43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2121470
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754072}

base/memory/shared_memory_hooks.h

@@ -9,11 +9,16 @@
 #include "base/memory/unsafe_shared_memory_region.h"
 #include "base/memory/writable_shared_memory_region.h"
 
-namespace service_manager {
+// TODO(https://crbug.com/1062136): This can be removed when Cloud Print support
+// is dropped.
+namespace content {
+struct MainFunctionParams;
+}  // namespace content
+int CloudPrintServiceProcessMain(const content::MainFunctionParams& parameters);
 
+namespace service_manager {
 struct MainParams;
 int Main(const MainParams&);
-
 }  // namespace service_manager
 
 namespace base {
@@ -24,6 +29,8 @@ class SharedMemoryHooks {
 
  private:
   friend class SharedMemoryHooksTest;
+  friend int ::CloudPrintServiceProcessMain(
+      const content::MainFunctionParams& parameters);
   friend int service_manager::Main(const service_manager::MainParams&);
 
   // Allows shared memory region creation to be hooked. Useful for sandboxed

chrome/service/service_main.cc

@@ -4,6 +4,7 @@
 
 #include "base/base_switches.h"
 #include "base/debug/debugger.h"
+#include "base/memory/shared_memory_hooks.h"
 #include "base/message_loop/message_pump_type.h"
 #include "base/run_loop.h"
 #include "base/task/single_thread_task_executor.h"
@@ -16,6 +17,17 @@
 // Mainline routine for running as the Cloud Print service process.
 int CloudPrintServiceProcessMain(
     const content::MainFunctionParams& parameters) {
+  // This is a hack: the Cloud Print service doesn't actually set up a sandbox,
+  // but service_manager::SandboxTypeFromCommandLine(command_line)) doesn't know
+  // about it, so it's considered sandboxed, causing shared memory hooks to be
+  // installed above. The Cloud Print service *also* doesn't set
+  // is_broker_process when initializing Mojo, so that bit also can't be used to
+  // determine whether or not to install the shared memory hooks.
+  //
+  // Since the Cloud Print service is supposed to go away at some point soon,
+  // just remove the hooks here.
+  base::SharedMemoryHooks::SetCreateHooks(nullptr, nullptr, nullptr);
+
   // Chrome disallows cookies by default. All code paths that want to use
   // cookies should go through the browser process.
   net::URLRequest::SetDefaultCookiePolicyToBlock();

services/service_manager/embedder/main.cc

@@ -388,11 +388,7 @@ int Main(const MainParams& params) {
     // sandboxed process. The defines below must be in sync with the
     // implementation of mojo::NodeController::CreateSharedBuffer().
 #if !defined(OS_MACOSX) && !defined(OS_NACL_SFI) && !defined(OS_FUCHSIA)
-    // TODO(dcheng): The separate check for |is_broker_process| should not be
-    // required, but avoid changing the behavior of IsUnsandboxedSandboxType()
-    // for now.
-    if (mojo_config.is_broker_process ||
-        service_manager::IsUnsandboxedSandboxType(
+    if (service_manager::IsUnsandboxedSandboxType(
             service_manager::SandboxTypeFromCommandLine(command_line))) {
       // Unsandboxed processes don't need shared memory brokering... because
       // they're not sandboxed.
@@ -400,6 +396,9 @@ int Main(const MainParams& params) {
       // Don't bother with hooks if direct shared memory allocation has been
       // requested.
     } else {
+      // Sanity check, since installing the shared memory hooks in a broker
+      // process will lead to infinite recursion.
+      DCHECK(!mojo_config.is_broker_process);
       // Otherwise, this is a sandboxed process that will need brokering to
       // allocate shared memory.
       base::SharedMemoryHooks::SetCreateHooks(