SFI NaCl: Batch-open resource files

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>
 #include <string.h>
@@ skipping 257 matching lines @@
 
 bool NaClListener::OnMessageReceived(const IPC::Message& msg) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(NaClListener, msg)
       IPC_MESSAGE_HANDLER(NaClProcessMsg_Start, OnStart)
       IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
+bool NaClListener::OnOpenResource(
+    const IPC::Message& msg,
+    const std::string& key,
+    NaClIPCAdapter::OpenResourceReplyCallback cb) {
+  DCHECK(!cb.is_null());
+  std::map<std::string, std::pair<

[Mark Seaborn, 2015/03/20 00:40:52]

Nit: Define a typedef for this std::map?

[Yusuke Sato, 2015/04/16 02:38:45]

Done.

+    IPC::PlatformFileForTransit, base::FilePath> >::iterator it;
+
+  const std::string files_prefix = "files/";
+  if (key.find(files_prefix) == 0)

[Mark Seaborn, 2015/03/20 00:40:52]

You're stripping off the "files/" prefix which is added by untrusted code (in
ppapi/nacl_irt/manifest_service.cc).

It looks like you don't need a fallback because the "files/" prefix should
always be there.

How about changing manifest_service.cc not to add the prefix?  AFAICT,
json_manifest.cc requires it, and then strips it off, so that check can be
removed too.  This can be a standalone cleanup change.

[Yusuke Sato, 2015/04/16 02:38:45]

Sure, let me add a TODO for now.

[Yusuke Sato, 2015/04/16 03:51:14]

https://codereview.chromium.org/1070233007/

+    it = prefetched_resource_files_.find(key.substr(files_prefix.length()));

[Mark Seaborn, 2015/03/20 00:40:52]

Can you comment this as a fast path, like how it's commented in the Non-SFI
version?

[Yusuke Sato, 2015/04/16 02:38:45]

Done.

+  else
+    it = prefetched_resource_files_.find(key);
+
+  if (it != prefetched_resource_files_.end()) {
+    IPC::PlatformFileForTransit file = it->second.first;
+    base::FilePath path = it->second.second;
+    prefetched_resource_files_.erase(it);

[Mark Seaborn, 2015/03/20 00:40:52]

Can you add a comment saying why this is thread-safe: which threads it runs on,
and which lock it holds.

[Yusuke Sato, 2015/04/16 02:38:45]

Done.

+    // A pre-opened resource descriptor is available. Run the reply callback
+    // and return true.
+    cb.Run(msg, file, path);
+    return true;
+  }
+
+  // Return false to let the IPC adapter issue an IPC to the renderer.

[Mark Seaborn, 2015/03/20 00:40:52]

...and fall back to the slow path.

[Yusuke Sato, 2015/04/16 02:38:45]

Done.

+  return false;
+}
+
 void NaClListener::OnStart(const nacl::NaClStartParams& params) {
 #if defined(OS_LINUX) || defined(OS_MACOSX)
   int urandom_fd = dup(base::GetUrandomFD());
   if (urandom_fd < 0) {
     LOG(ERROR) << "Failed to dup() the urandom FD";
     return;
   }
   NaClChromeMainSetUrandomFd(urandom_fd);
 #endif
   struct NaClApp* nap = NULL;
   NaClChromeMainInit();
 
   crash_info_shmem_.reset(new base::SharedMemory(params.crash_info_shmem_handle,
                                                  false));
   CHECK(crash_info_shmem_->Map(nacl::kNaClCrashInfoShmemSize));
   NaClSetFatalErrorCallback(&FatalLogHandler);
 
   nap = NaClAppCreate();
   if (nap == NULL) {
     LOG(ERROR) << "NaClAppCreate() failed";
     return;
   }
 
   IPC::ChannelHandle browser_handle;
   IPC::ChannelHandle ppapi_renderer_handle;
   IPC::ChannelHandle manifest_service_handle;
 
+  for (size_t i = 0; i < params.prefetched_resource_files.size(); ++i) {
+    bool result = prefetched_resource_files_.insert(std::make_pair(
+        params.prefetched_resource_files[i].file_key,
+        std::make_pair(
+            params.prefetched_resource_files[i].file,
+            params.prefetched_resource_files[i].file_path_metadata))).second;
+    if (!result) {
+      DLOG(ERROR) << "Duplicated open_resource key: "
+                  << params.prefetched_resource_files[i].file_key;
+    }
+  }
+
   if (params.enable_ipc_proxy) {
     browser_handle = IPC::Channel::GenerateVerifiedChannelID("nacl");
     ppapi_renderer_handle = IPC::Channel::GenerateVerifiedChannelID("nacl");
     manifest_service_handle = IPC::Channel::GenerateVerifiedChannelID("nacl");
 
     // Create the PPAPI IPC channels between the NaCl IRT and the host
     // (browser/renderer) processes. The IRT uses these channels to
     // communicate with the host and to initialize the IPC dispatchers.
     SetUpIPCAdapter(&browser_handle, io_thread_.message_loop_proxy(),
                     nap, NACL_CHROME_DESC_BASE);
     SetUpIPCAdapter(&ppapi_renderer_handle, io_thread_.message_loop_proxy(),
                     nap, NACL_CHROME_DESC_BASE + 1);
 
     scoped_refptr<NaClIPCAdapter> manifest_ipc_adapter =
         SetUpIPCAdapter(&manifest_service_handle,
                         io_thread_.message_loop_proxy(),
                         nap,
                         NACL_CHROME_DESC_BASE + 2);
     manifest_ipc_adapter->set_resolve_file_token_callback(
         base::Bind(&NaClListener::ResolveFileToken, base::Unretained(this)));
+    manifest_ipc_adapter->set_open_resource_callback(
+        base::Bind(&NaClListener::OnOpenResource, base::Unretained(this)));
   }
 
   trusted_listener_ = new NaClTrustedListener(
       IPC::Channel::GenerateVerifiedChannelID("nacl"),
       io_thread_.message_loop_proxy().get(),
       &shutdown_event_);
   if (!Send(new NaClProcessHostMsg_PpapiChannelsCreated(
           browser_handle,
           ppapi_renderer_handle,
           trusted_listener_->TakeClientChannelHandle(),
@@ skipping 106 matching lines @@
     // //mojo/nacl for implementation details.
     InjectMojo(nap);
   } else {
     // When Mojo isn't enabled, we inject a file descriptor that intentionally
     // fails on any imc_sendmsg() call to make debugging easier.
     InjectDisabledMojo(nap);
   }
 #else
   InjectDisabledMojo(nap);
 #endif
-  // TODO(yusukes): Support pre-opening resource files.
-  CHECK(params.prefetched_resource_files.empty());
 
   int exit_status;
   if (!NaClChromeMainStart(nap, args, &exit_status))
     NaClExit(1);
 
   // Report the plugin's exit status if the application started successfully.
   trusted_listener_->Send(new NaClRendererMsg_ReportExitStatus(exit_status));
   NaClExit(exit_status);
 }
 
 void NaClListener::ResolveFileToken(
     uint64_t token_lo,
     uint64_t token_hi,
     base::Callback<void(IPC::PlatformFileForTransit, base::FilePath)> cb) {
   if (!Send(new NaClProcessMsg_ResolveFileToken(token_lo, token_hi))) {
     cb.Run(IPC::PlatformFileForTransit(), base::FilePath());
     return;
   }
   resolved_cb_ = cb;
 }
 
 void NaClListener::OnFileTokenResolved(
     uint64_t token_lo,
     uint64_t token_hi,
     IPC::PlatformFileForTransit ipc_fd,
     base::FilePath file_path) {
   resolved_cb_.Run(ipc_fd, file_path);
   resolved_cb_.Reset();
 }