Pepper: Remove LOAD_MODULE SRPC call in SFI mode.

components/nacl/loader/nacl_listener.cc

 // Copyright 2013 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 "components/nacl/loader/nacl_listener.h"
 
 #include <errno.h>
+#include <fcntl.h>
 #include <stdlib.h>
 
 #if defined(OS_POSIX)
 #include <unistd.h>
 #endif
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/rand_util.h"
 #include "components/nacl/common/nacl_messages.h"
 #include "components/nacl/loader/nacl_ipc_adapter.h"
 #include "components/nacl/loader/nacl_validation_db.h"
 #include "components/nacl/loader/nacl_validation_query.h"
 #include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_switches.h"
 #include "ipc/ipc_sync_channel.h"
 #include "ipc/ipc_sync_message_filter.h"
 #include "native_client/src/public/chrome_main.h"
 #include "native_client/src/public/nacl_app.h"
 #include "native_client/src/public/nacl_file_info.h"
+#include "native_client/src/trusted/service_runtime/include/sys/fcntl.h"
 
 #if defined(OS_POSIX)
 #include "base/file_descriptor_posix.h"
 #endif
 
 #if defined(OS_LINUX)
 #include "components/nacl/loader/nonsfi/irt_random.h"
 #include "components/nacl/loader/nonsfi/nonsfi_main.h"
 #include "content/public/common/child_process_sandbox_support_linux.h"
 #include "native_client/src/trusted/desc/nacl_desc_io.h"
-#include "native_client/src/trusted/service_runtime/include/sys/fcntl.h"
 #include "ppapi/nacl_irt/plugin_startup.h"
 #endif
 
 #if defined(OS_WIN)
 #include <fcntl.h>
 #include <io.h>
 
 #include "content/public/common/sandbox_init.h"
 #endif
 
@@ skipping 332 matching lines @@
 #if defined(OS_WIN)
   args->broker_duplicate_handle_func = BrokerDuplicateHandle;
   args->attach_debug_exception_handler_func = AttachDebugExceptionHandler;
   args->debug_stub_server_port_selected_handler_func =
       DebugStubPortSelectedHandler;
 #endif
 #if defined(OS_LINUX)
   args->prereserved_sandbox_size = prereserved_sandbox_size_;
 #endif
 
+  NaClFileInfo nexe_file_info;
+  // FIXME: Fix this on windows so that we turn this handle into an int.

[bbudge, 2014/06/30 18:28:53]

FIXME->TODO (or remove, it seems like you address it below.)

[teravest, 2014/06/30 22:04:56]

Done.

+  base::PlatformFile nexe_file = IPC::PlatformFileForTransitToPlatformFile(
+      params.nexe_file);
+#if defined(OS_WIN)
+  nexe_file_info.desc =
+      _open_osfhandle(reinterpret_cast<intptr_t>(nexe_file),
+                      _O_RDONLY | _O_BINARY);
+#else

[bbudge, 2014/06/30 18:28:53]

nit: #elif defined(OS_POSIX)

[teravest, 2014/06/30 22:04:56]

Done.

+  nexe_file_info.desc = nexe_file;
+#endif
+  nexe_file_info.file_token.lo = params.nexe_token_lo;

[Mark Seaborn, 2014/06/30 20:01:03]

FYI, having the browser pass the token through to the NaCl loader process is
suboptimal, because the NaCl process will just turn around and send the token
values back to the browser (see NaClProcessMsg_ResolveFileToken) in order to get
a guaranteed-safe FD back.  It would be more efficient for the browser to send a
guaranteed-safe FD in the first place!  That would save an IPC round trip.

(That's what I commented on Hidehiko's earlier change,
https://codereview.chromium.org/337463002/#msg3)

I'm just mentioning this to make sure you're aware of it.  You don't have to
change this.

[teravest, 2014/06/30 22:04:56]

I see what you mean. I've added a TODO here to address this, but I'd like to do
it in a separate change to keep this one from getting too big.

+  nexe_file_info.file_token.hi = params.nexe_token_hi;
+  args->nexe_desc = NaClDescIoFromFileInfo(nexe_file_info, NACL_ABI_O_RDONLY);
+
   NaClChromeMainStartApp(nap, args);
   NOTREACHED();
 }
 
 void NaClListener::StartNonSfi(const nacl::NaClStartParams& params) {
 #if !defined(OS_LINUX)
   NOTREACHED() << "Non-SFI NaCl is only supported on Linux";
 #else
   // Random number source initialization.
   nacl::nonsfi::SetUrandomFd(base::GetUrandomFD());
@@ skipping 54 matching lines @@
       io_thread_.message_loop_proxy(), &shutdown_event_);
   if (!Send(new NaClProcessHostMsg_PpapiChannelsCreated(
           browser_handle, ppapi_renderer_handle,
           trusted_renderer_handle, manifest_service_handle)))
     LOG(ERROR) << "Failed to send IPC channel handle to NaClProcessHost.";
 
   // Ensure that the validation cache key (used as an extra input to the
   // validation cache's hashing) isn't exposed accidentally.
   CHECK(!params.validation_cache_enabled);
   CHECK(params.validation_cache_key.size() == 0);
   CHECK(params.version.size() == 0);

[Mark Seaborn, 2014/06/30 20:01:03]

You could also check that the token is zero, to ensure we're not leaking
uninitialised data here.

[teravest, 2014/06/30 22:04:56]

Done.

   // Ensure that a debug stub FD isn't passed through accidentally.
   CHECK(!params.enable_debug_stub);
   CHECK(params.debug_stub_server_bound_socket.fd == -1);
 
   CHECK(!params.uses_irt);
   CHECK(params.handles.empty());
 
   CHECK(params.nexe_file != IPC::InvalidPlatformFileForTransit());
   nacl::nonsfi::MainStart(
       NaClDescIoDescFromDescAllocCtor(
@@ skipping 11 matching lines @@
   IPC::ChannelHandle trusted_renderer_handle =
       IPC::Channel::GenerateVerifiedChannelID("nacl");
   trusted_listener_ = new NaClTrustedListener(
       trusted_renderer_handle, io_thread_.message_loop_proxy().get());
 #if defined(OS_POSIX)
   trusted_renderer_handle.socket = base::FileDescriptor(
       trusted_listener_->TakeClientFileDescriptor(), true);
 #endif
   return trusted_renderer_handle;
 }