Non-SFI NaCl: Batch-open resource files

components/nacl/renderer/ppb_nacl_private_impl.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/renderer/ppb_nacl_private_impl.h"
 
 #include <numeric>
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/containers/scoped_ptr_hash_map.h"
 #include "base/cpu.h"
 #include "base/files/file.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
+#include "base/macros.h"
 #include "base/rand_util.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "components/nacl/common/nacl_host_messages.h"
 #include "components/nacl/common/nacl_messages.h"
 #include "components/nacl/common/nacl_nonsfi_util.h"
 #include "components/nacl/common/nacl_switches.h"
 #include "components/nacl/common/nacl_types.h"
 #include "components/nacl/renderer/file_downloader.h"
 #include "components/nacl/renderer/histogram.h"
 #include "components/nacl/renderer/json_manifest.h"
 #include "components/nacl/renderer/manifest_downloader.h"
 #include "components/nacl/renderer/manifest_service_channel.h"
 #include "components/nacl/renderer/nexe_load_manager.h"
 #include "components/nacl/renderer/platform_info.h"
 #include "components/nacl/renderer/pnacl_translation_resource_host.h"
 #include "components/nacl/renderer/progress_event.h"
 #include "components/nacl/renderer/trusted_plugin_channel.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/sandbox_init.h"
 #include "content/public/renderer/pepper_plugin_instance.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
 #include "content/public/renderer/renderer_ppapi_host.h"
+#include "ipc/ipc_message_attachment_set.h"
 #include "native_client/src/public/imc_types.h"
 #include "net/base/data_url.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_util.h"
 #include "ppapi/c/pp_bool.h"
 #include "ppapi/c/private/pp_file_handle.h"
 #include "ppapi/shared_impl/ppapi_globals.h"
 #include "ppapi/shared_impl/ppapi_permissions.h"
 #include "ppapi/shared_impl/ppapi_preferences.h"
 #include "ppapi/shared_impl/var.h"
@@ skipping 12 matching lines @@
 
 namespace nacl {
 namespace {
 
 // The pseudo-architecture used to indicate portable native client.
 const char* const kPortableArch = "portable";
 
 // The base URL for resources used by the PNaCl translator processes.
 const char* kPNaClTranslatorBaseUrl = "chrome://pnacl-translator/";
 
+// The maximum number of resource files DownloadNexe() can open.
+const size_t kMaxPreOpenResourceFiles = 2;

[Mark Seaborn, 2015/02/09 04:48:34]

Why so low?  I thought your aim was to pass more than this.

With this set low, I think the fast path won't be covered by the tests you added
earlier, will it?

[Yusuke Sato, 2015/02/11 05:54:21]

This is currently 2 because kMaxDescriptorsPerMessage in ipc/ is still 7.

The test I added is to make sure that the renderer process can start a NaCl
plugin whose nmf has more than kMaxDescriptorsPerMessage resource files. I think
the test is useful regardless of the kMaxPreOpenResourceFiles value.

I will increase kMaxDescriptorsPerMessage in ipc/ as well as
kMaxPreOpenResourceFiles to something like ~80 or ~128 once this CL is
submitted. ipc/ changes are not part of this CL following your earlier
suggestion.

[Mark Seaborn, 2015/02/12 03:57:33]

OK, so the test is still covering the fast path.  Can you add a TEST= line to
the commit message, please, to give the name of the test that's covering this?

[Yusuke Sato, 2015/02/13 23:01:16]

Done.

+
+#if defined(OS_POSIX)
+// On POSIX platforms, the maximum number of files a process can pass to
+// another is limited to kMaxDescriptorsPerMessage. Since the browser process
+// may use up to 5 descriptors for passing non-resource files when creating
+// the NaCl plugin process, kMaxPreOpenResourceFiles must be smaller than or
+// equal to (IPC::MessageAttachmentSet::kMaxDescriptorsPerMessage - 5).
+COMPILE_ASSERT(kMaxPreOpenResourceFiles <=
+               IPC::MessageAttachmentSet::kMaxDescriptorsPerMessage - 5,
+               k_max_pre_open_resource_files_too_large);
+#endif
+
 base::LazyInstance<scoped_refptr<PnaclTranslationResourceHost> >
     g_pnacl_resource_host = LAZY_INSTANCE_INITIALIZER;
 
 bool InitializePnaclResourceHost() {
   // Must run on the main thread.
   content::RenderThread* render_thread = content::RenderThread::Get();
   if (!render_thread)
     return false;
   if (!g_pnacl_resource_host.Get().get()) {
     g_pnacl_resource_host.Get() = new PnaclTranslationResourceHost(
@@ skipping 15 matching lines @@
 };
 
 class NaClPluginInstance {
  public:
   NaClPluginInstance(PP_Instance instance):
       nexe_load_manager(instance), pexe_size(0) {}
 
   NexeLoadManager nexe_load_manager;
   scoped_ptr<JsonManifest> json_manifest;
   scoped_ptr<InstanceInfo> instance_info;
+  std::vector<NaClLaunchParams::ResourceFileInfo> resource_files_info;

[Mark Seaborn, 2015/02/09 04:48:34]

If I read this right, NaClLaunchParams::ResourceFileInfo contains an
IPC::PlatformFileForTransit, which is a non-owning reference.  That means this
would leak FDs if the NaCl instance failed to start up.

[Yusuke Sato, 2015/02/11 05:54:21]

You're right. The IPC::PlatformFileForTransit handle here is a
base::FileDescriptor with auto_close=true, so once it's passed to a IPC message,
it's always auto-closed, but if something fails before creating the IPC message,
the FDs will leak. I've added ~NaClPluginInstance() to handle such a case. Done.

 
   // When translation is complete, this records the size of the pexe in
   // bytes so that it can be reported in a later load event.
   uint64_t pexe_size;
 };
 
 typedef base::ScopedPtrHashMap<PP_Instance, NaClPluginInstance> InstanceMap;
 base::LazyInstance<InstanceMap> g_instance_map = LAZY_INSTANCE_INITIALIZER;
 
 NaClPluginInstance* GetNaClPluginInstance(PP_Instance instance) {
@@ skipping 273 matching lines @@
   // must also check on the trusted side of the proxy.
   if (load_manager->DevInterfacesEnabled())
     perm_bits |= ppapi::PERMISSION_DEV;
   instance_info.permissions =
       ppapi::PpapiPermissions::GetForCommandLine(perm_bits);
   std::string error_message_string;
   NaClLaunchResult launch_result;
 
   IPC::PlatformFileForTransit nexe_for_transit =
       IPC::InvalidPlatformFileForTransit();
+
+  NaClPluginInstance* nacl_plugin_instance = GetNaClPluginInstance(instance);
 #if defined(OS_POSIX)
   if (nexe_file_info->handle != PP_kInvalidFileHandle)
     nexe_for_transit = base::FileDescriptor(nexe_file_info->handle, true);
+  const std::vector<NaClLaunchParams::ResourceFileInfo>& resource_files_info =
+      nacl_plugin_instance->resource_files_info;

[Mark Seaborn, 2015/02/09 04:48:34]

Shouldn't nacl_plugin_instance->resource_files_info be cleared out after this? 
(At least this would reclaim some storage.  Not sure if it would leak FDs for
the <embed>'s lifetime without this.)

[Yusuke Sato, 2015/02/11 05:54:21]

Done.

 #elif defined(OS_WIN)
   // Duplicate the handle on the browser side instead of the renderer.
   // This is because BrokerGetFileForProcess isn't part of content/public, and
   // it's simpler to do the duplication in the browser anyway.
   nexe_for_transit = nexe_file_info->handle;
+  // TODO(yusukes): Support pre-opening resource files.

[Mark Seaborn, 2015/02/09 04:48:34]

Add "on Windows"?

[Yusuke Sato, 2015/02/11 05:54:21]

Done.

+  std::vector<NaClLaunchParams::ResourceFileInfo> resource_files_info;  // empty
 #else
 #error Unsupported target platform.
 #endif
   if (!sender->Send(new NaClHostMsg_LaunchNaCl(
           NaClLaunchParams(
               instance_info.url.spec(),
               nexe_for_transit,
               nexe_file_info->token_lo,
               nexe_file_info->token_hi,
+              resource_files_info,
               routing_id,
               perm_bits,
               PP_ToBool(uses_nonsfi_mode),
               process_type),
           &launch_result,
           &error_message_string))) {
     ppapi::PpapiGlobals::Get()->GetMainThreadMessageLoop()->PostTask(
         FROM_HERE,
         base::Bind(callback.func, callback.user_data,
                    static_cast<int32_t>(PP_ERROR_FAILED)));
@@ skipping 13 matching lines @@
         base::Bind(callback.func, callback.user_data,
                    static_cast<int32_t>(PP_ERROR_FAILED)));
     return;
   }
   result_socket = launch_result.imc_channel_handle;
   instance_info.channel_handle = launch_result.ppapi_ipc_channel_handle;
   instance_info.plugin_pid = launch_result.plugin_pid;
   instance_info.plugin_child_id = launch_result.plugin_child_id;
 
   // Don't save instance_info if channel handle is invalid.
-  if (IsValidChannelHandle(instance_info.channel_handle)) {
-    NaClPluginInstance* nacl_plugin_instance = GetNaClPluginInstance(instance);
+  if (IsValidChannelHandle(instance_info.channel_handle))
     nacl_plugin_instance->instance_info.reset(new InstanceInfo(instance_info));
-  }
 
   *(static_cast<NaClHandle*>(imc_handle)) = ToNativeHandle(result_socket);
 
   // Store the crash information shared memory handle.
   load_manager->set_crash_info_shmem_handle(
       launch_result.crash_info_shmem_handle);
 
   // Create the trusted plugin channel.
   if (IsValidChannelHandle(launch_result.trusted_ipc_channel_handle)) {
     bool report_exit_status = PP_ToBool(main_service_runtime);
@@ skipping 108 matching lines @@
     replace_pos = r.find_first_not_of(white_list);
   }
   return r;
 }
 
 PP_FileHandle GetReadonlyPnaclFd(const char* url,
                                  bool is_executable,
                                  uint64_t* nonce_lo,
                                  uint64_t* nonce_hi) {
   std::string filename = PnaclComponentURLToFilename(url);
-  IPC::PlatformFileForTransit out_fd = IPC::InvalidPlatformFileForTransit();
   IPC::Sender* sender = content::RenderThread::Get();
+  NaClFileInfo file_info;
   DCHECK(sender);
   if (!sender->Send(new NaClHostMsg_GetReadonlyPnaclFD(
-          std::string(filename), is_executable,
-          &out_fd, nonce_lo, nonce_hi))) {
+          std::string(filename), is_executable, &file_info))) {
     return PP_kInvalidFileHandle;
   }
-  if (out_fd == IPC::InvalidPlatformFileForTransit()) {
+  if (file_info.file == IPC::InvalidPlatformFileForTransit()) {
     return PP_kInvalidFileHandle;
   }
-  return IPC::PlatformFileForTransitToPlatformFile(out_fd);
+  *nonce_lo = file_info.file_token_lo;
+  *nonce_hi = file_info.file_token_hi;
+  return IPC::PlatformFileForTransitToPlatformFile(file_info.file);
 }
 
 void GetReadExecPnaclFd(const char* url,
                         PP_NaClFileInfo* out_file_info) {
   *out_file_info = kInvalidNaClFileInfo;
   out_file_info->handle = GetReadonlyPnaclFd(url, true /* is_executable */,
                                              &out_file_info->token_lo,
                                              &out_file_info->token_hi);
 }
 
@@ skipping 92 matching lines @@
     return;
   g_pnacl_resource_host.Get()->ReportTranslationFinished(instance, success);
 
   // Record the pexe size for reporting in a later load event.
   NaClPluginInstance* nacl_plugin_instance = GetNaClPluginInstance(instance);
   if (nacl_plugin_instance) {
     nacl_plugin_instance->pexe_size = pexe_size;
   }
 }
 
-PP_FileHandle OpenNaClExecutable(PP_Instance instance,
-                                 const char* file_url,
-                                 uint64_t* nonce_lo,
-                                 uint64_t* nonce_hi) {
-  // Fast path only works for installed file URLs.
-  GURL gurl(file_url);
-  if (!gurl.SchemeIs("chrome-extension"))
-    return PP_kInvalidFileHandle;
+bool OpenNaClResources(
+    PP_Instance instance,
+    const std::vector<std::string>& resource_file_urls,
+    std::vector<NaClFileInfo>* out_resource_file_handles) {
+  DCHECK(out_resource_file_handles);
+  if (resource_file_urls.empty())
+    return false;
 
   NexeLoadManager* load_manager = GetNexeLoadManager(instance);
   DCHECK(load_manager);
   if (!load_manager)
     return PP_kInvalidFileHandle;
 
   content::PepperPluginInstance* plugin_instance =
       content::PepperPluginInstance::Get(instance);
   if (!plugin_instance)
-    return PP_kInvalidFileHandle;
-  // IMPORTANT: Make sure the document can request the given URL. If we don't
-  // check, a malicious app could probe the extension system. This enforces a
-  // same-origin policy which prevents the app from requesting resources from
-  // another app.
+    return false;
   blink::WebSecurityOrigin security_origin =
       plugin_instance->GetContainer()->element().document().securityOrigin();
-  if (!security_origin.canRequest(gurl))
-    return PP_kInvalidFileHandle;
-
-  IPC::PlatformFileForTransit out_fd = IPC::InvalidPlatformFileForTransit();
   IPC::Sender* sender = content::RenderThread::Get();
   DCHECK(sender);
-  *nonce_lo = 0;
-  *nonce_hi = 0;
-  base::FilePath file_path;
-  if (!sender->Send(
-      new NaClHostMsg_OpenNaClExecutable(GetRoutingID(instance),
-                                         GURL(file_url),
-                                         !load_manager->nonsfi(),
-                                         &out_fd,
-                                         nonce_lo,
-                                         nonce_hi))) {
-    return PP_kInvalidFileHandle;
+
+  std::vector<GURL> resource_gurls;
+  for (size_t i = 0; i < resource_file_urls.size(); ++i) {
+    resource_gurls.push_back(GURL(resource_file_urls[i]));
   }
 
-  if (out_fd == IPC::InvalidPlatformFileForTransit())
-    return PP_kInvalidFileHandle;
+  for (size_t i = 0; i < resource_gurls.size(); ++i) {
+    const GURL& gurl = resource_gurls[i];
+    // Fast path only works for installed file URLs.
+    if (!gurl.SchemeIs("chrome-extension"))
+      return false;
+    // IMPORTANT: Make sure the document can request the given URL. If we don't
+    // check, a malicious app could probe the extension system. This enforces a
+    // same-origin policy which prevents the app from requesting resources from
+    // another app.
+    if (!security_origin.canRequest(gurl))
+      return false;
+  }
 
-  return IPC::PlatformFileForTransitToPlatformFile(out_fd);
+  if (!sender->Send(new NaClHostMsg_OpenNaClResources(
+          GetRoutingID(instance),
+          resource_gurls,
+          !load_manager->nonsfi(),
+          out_resource_file_handles))) {
+    return false;
+  }
+
+  if (out_resource_file_handles->empty() ||
+      (out_resource_file_handles->at(0).file ==
+       IPC::InvalidPlatformFileForTransit())) {
+    DLOG(ERROR) << "Could not open the main nexe: " << resource_gurls[0];
+    return false;
+  }
+  return true;
 }
 
 void DispatchEvent(PP_Instance instance,
                    PP_NaClEventType event_type,
                    const char* resource_url,
                    PP_Bool length_is_computable,
                    uint64_t loaded_bytes,
                    uint64_t total_bytes) {
   ProgressEvent event(event_type,
                       resource_url,
@@ skipping 437 matching lines @@
 };
 
 void DownloadNexeCompletion(const DownloadNexeRequest& request,
                             PP_NaClFileInfo* out_file_info,
                             FileDownloader::Status status,
                             base::File target_file,
                             int http_status);
 
 void DownloadNexe(PP_Instance instance,
                   const char* url,
+                  PP_Bool download_resource_files,
                   PP_NaClFileInfo* out_file_info,
                   PP_CompletionCallback callback) {
   CHECK(url);
   CHECK(out_file_info);
   DownloadNexeRequest request;
   request.instance = instance;
   request.url = url;
   request.callback = callback;
   request.start_time = base::Time::Now();
 
+  std::vector<std::string> file_urls;
+  file_urls.push_back(url);  // the main nexe must be the first element.
+
+  std::vector<
+    std::pair<std::string /*url*/, std::string /*key*/> > resource_files;
+  if (download_resource_files) {
+    JsonManifest* manifest = GetJsonManifest(instance);
+    if (manifest)
+      manifest->GetFiles(&resource_files);
+    if (resource_files.size() > kMaxPreOpenResourceFiles)
+      resource_files.resize(kMaxPreOpenResourceFiles);
+    for (size_t i = 0; i < resource_files.size(); ++i) {
+      file_urls.push_back(resource_files[i].first);
+    }
+  }
+
   // Try the fast path for retrieving the file first.
-  PP_FileHandle handle = OpenNaClExecutable(instance,
-                                            url,
-                                            &out_file_info->token_lo,
-                                            &out_file_info->token_hi);
-  if (handle != PP_kInvalidFileHandle) {
+  std::vector<NaClFileInfo> file_handles;
+  if (OpenNaClResources(instance, file_urls, &file_handles)) {
+    DCHECK(!file_handles.empty());
+    out_file_info->handle =
+        IPC::PlatformFileForTransitToPlatformFile(file_handles[0].file);
+    out_file_info->token_lo = file_handles[0].file_token_lo;
+    out_file_info->token_hi = file_handles[0].file_token_hi;
+    NaClPluginInstance* nacl_plugin_instance = GetNaClPluginInstance(instance);
+    for (size_t i = 1; i < file_handles.size(); ++i) {
+      std::string key = resource_files[i - 1].second;
+      nacl_plugin_instance->resource_files_info.push_back(
+          NaClLaunchParams::ResourceFileInfo(file_handles[i], key));
+    }
     DownloadNexeCompletion(request,
                            out_file_info,
                            FileDownloader::SUCCESS,
-                           base::File(handle),
+                           base::File(out_file_info->handle),
                            200);
     return;
   }
 
   // The fast path didn't work, we'll fetch the file using URLLoader and write
   // it to local storage.
   base::File target_file(CreateTemporaryFile(instance));
   GURL gurl(url);
 
+  // On the slow path, do not retry to pre-open the resource files.
   content::PepperPluginInstance* plugin_instance =
       content::PepperPluginInstance::Get(instance);
   if (!plugin_instance) {
     ppapi::PpapiGlobals::Get()->GetMainThreadMessageLoop()->PostTask(
         FROM_HERE,
         base::Bind(callback.func, callback.user_data,
                    static_cast<int32_t>(PP_ERROR_FAILED)));
   }
   const blink::WebDocument& document =
       plugin_instance->GetContainer()->element().document();
@@ skipping 115 matching lines @@
   if (!test_gurl.is_valid()) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(callback,
                    static_cast<int32_t>(PP_ERROR_FAILED),
                    kInvalidNaClFileInfo));
     return;
   }
 
   // Try the fast path for retrieving the file first.
-  uint64_t file_token_lo = 0;
-  uint64_t file_token_hi = 0;
-  PP_FileHandle file_handle = OpenNaClExecutable(instance,
-                                                 url.c_str(),
-                                                 &file_token_lo,
-                                                 &file_token_hi);
-  if (file_handle != PP_kInvalidFileHandle) {
+  std::vector<std::string> file_urls;
+  file_urls.push_back(url);
+  std::vector<NaClFileInfo> out_handles;
+  if (OpenNaClResources(instance, file_urls, &out_handles)) {

[Mark Seaborn, 2015/02/09 04:48:35]

I think there's a potential performance regression here...

Suppose we have an app whose main nexe is served via a "chrome-extension:" URL,
but which has other files served via HTTP(S).  Before, the fast path (validation
caching + opening without a copy) will work for the main nexe.  After, the fast
path will be disabled on the grounds that it doesn't work for all files in the
manifest file.

[Yusuke Sato, 2015/02/11 05:54:21]

I wasn't aware that HTTP(S) URL was allowed in "files". Slightly changed
DownloadNexe so that it only retrieves resource URLs from nmf that start with
"chrome-extension". Done.

     PP_NaClFileInfo file_info;
-    file_info.handle = file_handle;
-    file_info.token_lo = file_token_lo;
-    file_info.token_hi = file_token_hi;
+    file_info.handle =
+        IPC::PlatformFileForTransitToPlatformFile(out_handles[0].file);
+    file_info.token_lo = out_handles[0].file_token_lo;
+    file_info.token_hi = out_handles[0].file_token_hi;
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(callback, static_cast<int32_t>(PP_OK), file_info));
     return;
   }
 
   // The fast path didn't work, we'll fetch the file using URLLoader and write
   // it to local storage.
   base::File target_file(CreateTemporaryFile(instance));
   GURL gurl(url);
@@ skipping 252 matching lines @@
   &StreamPexe
 };
 
 }  // namespace
 
 const PPB_NaCl_Private* GetNaClPrivateInterface() {
   return &nacl_interface;
 }
 
 }  // namespace nacl