SFI NaCl: Batch-open resource files

components/nacl/loader/nacl_ipc_adapter.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_ipc_adapter.h"
 
 #include <limits.h>
 #include <string.h>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/shared_memory.h"
 #include "base/task_runner_util.h"
+#include "base/tuple.h"
 #include "build/build_config.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_platform_file.h"
 #include "native_client/src/public/nacl_desc.h"
 #include "native_client/src/trusted/desc/nacl_desc_base.h"
 #include "native_client/src/trusted/desc/nacl_desc_custom.h"
 #include "native_client/src/trusted/desc/nacl_desc_imc_shm.h"
 #include "native_client/src/trusted/desc/nacl_desc_io.h"
 #include "native_client/src/trusted/desc/nacl_desc_quota.h"
 #include "native_client/src/trusted/desc/nacl_desc_quota_interface.h"
@@ skipping 594 matching lines @@
 
   // Write empty file tokens.
   new_msg->WriteUInt64(0);  // token_lo
   new_msg->WriteUInt64(0);  // token_hi
   return new_msg.Pass();
 }
 
 void NaClIPCAdapter::OnFileTokenResolved(const IPC::Message& orig_msg,
                                          IPC::PlatformFileForTransit ipc_fd,
                                          base::FilePath file_path) {
+  base::AutoLock lock(lock_);
+  OnFileTokenResolvedLocked(orig_msg, ipc_fd, file_path);
+}
+
+void NaClIPCAdapter::OnFileTokenResolvedLocked(

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

Since this is reused for the new open_resource case when there's no file token
involved, it seems like you should rename this.

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

Done.

+    const IPC::Message& orig_msg,
+    IPC::PlatformFileForTransit ipc_fd,
+    base::FilePath file_path) {
+  lock_.AssertAcquired();
   // The path where an invalid ipc_fd is returned isn't currently
   // covered by any tests.
   if (ipc_fd == IPC::InvalidPlatformFileForTransit()) {
     // The file token didn't resolve successfully, so we give the
     // original FD to the client without making a validated NaClDesc.
     // However, we must rewrite the message to clear the file tokens.
     PickleIterator iter = IPC::SyncMessage::GetDataIterator(&orig_msg);
     ppapi::proxy::SerializedHandle sh;
 
     // We know that this can be read safely; see the original read in
     // OnMessageReceived().
     CHECK(IPC::ReadParam(&orig_msg, &iter, &sh));
     scoped_ptr<IPC::Message> new_msg = CreateOpenResourceReply(orig_msg, sh);
 
     scoped_ptr<NaClDescWrapper> desc_wrapper(new NaClDescWrapper(
         NaClDescIoDescFromHandleAllocCtor(
 #if defined(OS_WIN)
             sh.descriptor(),
 #else
             sh.descriptor().fd,
 #endif
             NACL_ABI_O_RDONLY)));
 
     scoped_refptr<RewrittenMessage> rewritten_msg(new RewrittenMessage);
     rewritten_msg->AddDescriptor(desc_wrapper.release());
-    {
-      base::AutoLock lock(lock_);
-      SaveMessage(*new_msg, rewritten_msg.get());
-      cond_var_.Signal();
-    }
+    SaveMessage(*new_msg, rewritten_msg.get());

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

Reverted this change.

+    cond_var_.Signal();
     return;
   }
 
   // The file token was sucessfully resolved.
   std::string file_path_str = file_path.AsUTF8Unsafe();
   base::PlatformFile handle =
       IPC::PlatformFileForTransitToPlatformFile(ipc_fd);
 
   ppapi::proxy::SerializedHandle sh;
   sh.set_file_handle(ipc_fd, PP_FILEOPENFLAG_READ, 0);
   scoped_ptr<IPC::Message> new_msg = CreateOpenResourceReply(orig_msg, sh);
   scoped_refptr<RewrittenMessage> rewritten_msg(new RewrittenMessage);
 
   struct NaClDesc* desc =
       NaClDescCreateWithFilePathMetadata(handle, file_path_str.c_str());
   rewritten_msg->AddDescriptor(new NaClDescWrapper(desc));
-  {
-    base::AutoLock lock(lock_);
-    SaveMessage(*new_msg, rewritten_msg.get());
-    cond_var_.Signal();
-  }
+  SaveMessage(*new_msg, rewritten_msg.get());

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

same. reverted.

+  cond_var_.Signal();
 }
 
 void NaClIPCAdapter::OnChannelConnected(int32 peer_pid) {
 }
 
 void NaClIPCAdapter::OnChannelError() {
   CloseChannel();
 }
 
 NaClIPCAdapter::~NaClIPCAdapter() {
@@ skipping 60 matching lines @@
     // here before we drop the message.
     return false;
   }
 
   // Scan all untrusted messages.
   scoped_ptr<IPC::Message> new_msg;
   locked_data_.nacl_msg_scanner_.ScanUntrustedMessage(*msg, &new_msg);
   if (new_msg)
     msg.reset(new_msg.release());
 
+  // Handle PpapiHostMsg_OpenResource locally without sending an IPC to the

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

Moved this to SendMessageOnIOThread so that
NaClIPCAdapter::OnFileTokenResolvedLocked (which is now renamed to
SaveOpenResourceMessage) is called on the IO thread too.

The comment at line 506
 // resolve_file_token_cb_ must be invoked from the main thread
was incorrect, and the slow path actually calls OnFileTokenResolved on the IO
thread. The fast path should do the same.

+  // renderer when possible.
+  PpapiHostMsg_OpenResource::Schema::SendParam send_params;
+  if (!open_resource_cb_.is_null() &&
+      (msg->type() == PpapiHostMsg_OpenResource::ID) &&

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

Nit: ()s not needed around this

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

Done.

+      PpapiHostMsg_OpenResource::ReadSendParam(msg.get(), &send_params)) {
+    const std::string key = get<0>(send_params);
+    if (open_resource_cb_.Run(
+            *msg.get(), key,
+            base::Bind(&NaClIPCAdapter::OnFileTokenResolvedLocked, this))) {
+      // The callback sent a reply to the untrusted side.
+      return true;
+    }
+  }
+
   // Actual send must be done on the I/O thread.
   task_runner_->PostTask(FROM_HERE,
       base::Bind(&NaClIPCAdapter::SendMessageOnIOThread, this,
                  base::Passed(&msg)));
   return true;
 }
 
 void NaClIPCAdapter::ClearToBeSent() {
   lock_.AssertAcquired();
 
@@ skipping 36 matching lines @@
   header.flags = msg.flags();
   header.num_fds = static_cast<uint16>(rewritten_msg->desc_count());
 
   rewritten_msg->SetData(header, msg.payload(), msg.payload_size());
   locked_data_.to_be_received_.push(rewritten_msg);
 }
 
 int TranslatePepperFileReadWriteOpenFlagsForTesting(int32_t pp_open_flags) {
   return TranslatePepperFileReadWriteOpenFlags(pp_open_flags);
 }