NaCl: Simpler validation for main nexe.

components/nacl/browser/nacl_process_host.cc

 // Copyright (c) 2012 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/browser/nacl_process_host.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 614 matching lines @@
       IPC_MESSAGE_HANDLER(NaClProcessHostMsg_PpapiChannelsCreated,
                           OnPpapiChannelsCreated)
       IPC_MESSAGE_UNHANDLED(handled = false)
     IPC_END_MESSAGE_MAP()
   } else {
     IPC_BEGIN_MESSAGE_MAP(NaClProcessHost, msg)
       IPC_MESSAGE_HANDLER(NaClProcessMsg_QueryKnownToValidate,
                           OnQueryKnownToValidate)
       IPC_MESSAGE_HANDLER(NaClProcessMsg_SetKnownToValidate,
                           OnSetKnownToValidate)
-      IPC_MESSAGE_HANDLER_DELAY_REPLY(NaClProcessMsg_ResolveFileToken,
-                                      OnResolveFileToken)
       IPC_MESSAGE_HANDLER(NaClProcessMsg_ResolveFileTokenAsync,
                           OnResolveFileTokenAsync)
 
 #if defined(OS_WIN)
       IPC_MESSAGE_HANDLER_DELAY_REPLY(
           NaClProcessMsg_AttachDebugExceptionHandler,
           OnAttachDebugExceptionHandler)
       IPC_MESSAGE_HANDLER(NaClProcessHostMsg_DebugStubPortSelected,
                           OnDebugStubPortSelected)
 #endif
@@ skipping 168 matching lines @@
     // not created.
     DCHECK(!socket_for_sel_ldr_.IsValid());
 #endif
   } else {
     params.validation_cache_enabled = nacl_browser->ValidationCacheIsEnabled();
     params.validation_cache_key = nacl_browser->GetValidationCacheKey();
     params.version = NaClBrowser::GetDelegate()->GetVersionString();
     params.enable_debug_stub = enable_debug_stub_ &&
         NaClBrowser::GetDelegate()->URLMatchesDebugPatterns(manifest_url_);
 
-    // TODO(teravest): Resolve the file tokens right now instead of making the
-    // loader send IPC to resolve them later.
-    params.nexe_token_lo = nexe_token_.lo;
-    params.nexe_token_hi = nexe_token_.hi;
-
     const ChildProcessData& data = process_->GetData();
     if (!ShareHandleToSelLdr(data.handle,
                              socket_for_sel_ldr_.TakePlatformFile(),
                              true,
                              &params.handles)) {
       return false;
     }
 
     // Currently, everything uses the IRT except for the PNaCl Translators.
     bool uses_irt = process_type_ != kPNaClTranslatorProcessType;
@@ skipping 34 matching lines @@
     if (params.enable_debug_stub) {
       net::SocketDescriptor server_bound_socket = GetDebugStubSocketHandle();
       if (server_bound_socket != net::kInvalidSocket) {
         params.debug_stub_server_bound_socket =
             FileDescriptor(server_bound_socket, true);
       }
     }
 #endif
   }
 
-  params.nexe_file = IPC::TakeFileHandleForProcess(nexe_file_.Pass(),
-                                                   process_->GetData().handle);
   if (!crash_info_shmem_.ShareToProcess(process_->GetData().handle,
                                         &params.crash_info_shmem_handle)) {
     DLOG(ERROR) << "Failed to ShareToProcess() a shared memory buffer";
     return false;
   }
 
+  base::FilePath file_path;
+  if (NaClBrowser::GetInstance()->GetFilePath(nexe_token_.lo,
+                                              nexe_token_.hi,
+                                              &file_path)) {
+    // We have to reopen the file in the browser process; we don't want a
+    // compromised renderer to pass an arbitrary fd that could get loaded
+    // into the plugin process.
+    if (base::PostTaskAndReplyWithResult(
+           content::BrowserThread::GetBlockingPool(),
+           FROM_HERE,
+           base::Bind(OpenNaClReadExecImpl,
+                      file_path,
+                      true /* is_executable */),
+           base::Bind(&NaClProcessHost::StartNaClFileResolved,
+                      weak_factory_.GetWeakPtr(),
+                      params,
+                      file_path))) {
+      return true;
+    }
+  }
+
+  params.nexe_file = IPC::TakeFileHandleForProcess(nexe_file_.Pass(),
+                                                   process_->GetData().handle);
   process_->Send(new NaClProcessMsg_Start(params));
   return true;
 }
 
+void ClosePlatformFile(base::PlatformFile file) {

[Mark Seaborn, 2014/10/14 17:37:24]

Shouldn't this be in an anon namespace?  Doesn't base/ provide a function to
close a PlatformFile?

[teravest, 2014/10/14 18:15:00]

I've moved this to an anonymous namespace and changed to pass a base::File
instead of having a routine to close a PlatformFile.

+#if defined(OS_WIN)
+  ::CloseHandle(file);
+#elif defined(OS_POSIX)
+  IGNORE_EINTR(::close(file));
+#endif
+}
+
+void NaClProcessHost::StartNaClFileResolved(
+    NaClStartParams params,
+    const base::FilePath& file_path,
+    base::File nexe_file) {
+  if (nexe_file.IsValid()) {
+    // Release the file received from the renderer. This has to be done on a
+    // thread where IO is permitted, though.
+    base::File close_nexe_file = nexe_file_.Pass();
+    content::BrowserThread::GetBlockingPool()->PostTask(
+        FROM_HERE,
+        base::Bind(&ClosePlatformFile, close_nexe_file.TakePlatformFile()));
+    params.nexe_file_path = file_path;
+    params.nexe_file = IPC::TakeFileHandleForProcess(
+        nexe_file.Pass(), process_->GetData().handle);
+  } else {
+    params.nexe_file = IPC::TakeFileHandleForProcess(
+        nexe_file_.Pass(), process_->GetData().handle);
+  }
+  process_->Send(new NaClProcessMsg_Start(params));
+}
+
 // This method is called when NaClProcessHostMsg_PpapiChannelCreated is
 // received.
 void NaClProcessHost::OnPpapiChannelsCreated(
     const IPC::ChannelHandle& browser_channel_handle,
     const IPC::ChannelHandle& ppapi_renderer_channel_handle,
     const IPC::ChannelHandle& trusted_renderer_channel_handle,
     const IPC::ChannelHandle& manifest_service_channel_handle) {
   if (!enable_ppapi_proxy()) {
     ReplyToRenderer(IPC::ChannelHandle(),
                     trusted_renderer_channel_handle,
@@ skipping 82 matching lines @@
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
   *result = nacl_browser->QueryKnownToValidate(signature, off_the_record_);
 }
 
 void NaClProcessHost::OnSetKnownToValidate(const std::string& signature) {
   CHECK(!uses_nonsfi_mode_);
   NaClBrowser::GetInstance()->SetKnownToValidate(
       signature, off_the_record_);
 }
 
-void NaClProcessHost::OnResolveFileToken(uint64 file_token_lo,
-                                         uint64 file_token_hi,
-                                         IPC::Message* reply_msg) {
+void NaClProcessHost::OnResolveFileTokenAsync(uint64 file_token_lo,
+                                              uint64 file_token_hi) {
   // Was the file registered?
   //
   // Note that the file path cache is of bounded size, and old entries can get
   // evicted.  If a large number of NaCl modules are being launched at once,
   // resolving the file_token may fail because the path cache was thrashed
   // while the file_token was in flight.  In this case the query fails, and we
   // need to fall back to the slower path.
   //
   // However: each NaCl process will consume 2-3 entries as it starts up, this
   // means that eviction will not happen unless you start up 33+ NaCl processes
   // at the same time, and this still requires worst-case timing.  As a
   // practical matter, no entries should be evicted prematurely.
   // The cache itself should take ~ (150 characters * 2 bytes/char + ~60 bytes
   // data structure overhead) * 100 = 35k when full, so making it bigger should
   // not be a problem, if needed.
   //
   // Each NaCl process will consume 2-3 entries because the manifest and main
   // nexe are currently not resolved.  Shared libraries will be resolved.  They
   // will be loaded sequentially, so they will only consume a single entry
   // while the load is in flight.
-  //
-  // TODO(ncbray): track behavior with UMA. If entries are getting evicted or
-  // bogus keys are getting queried, this would be good to know.
   CHECK(!uses_nonsfi_mode_);
   base::FilePath file_path;
   if (!NaClBrowser::GetInstance()->GetFilePath(
-        file_token_lo, file_token_hi, &file_path)) {
-    NaClProcessMsg_ResolveFileToken::WriteReplyParams(
-        reply_msg,
-        IPC::InvalidPlatformFileForTransit(),
-        base::FilePath());
-    Send(reply_msg);
-    return;
-  }
-
-  // Open the file.
-  if (!base::PostTaskAndReplyWithResult(
-          content::BrowserThread::GetBlockingPool(),
-          FROM_HERE,
-          base::Bind(OpenNaClReadExecImpl, file_path, true /* is_executable */),
-          base::Bind(&NaClProcessHost::FileResolved,
-                     weak_factory_.GetWeakPtr(),
-                     file_path,
-                     reply_msg))) {
-     NaClProcessMsg_ResolveFileToken::WriteReplyParams(
-         reply_msg,
-         IPC::InvalidPlatformFileForTransit(),
-         base::FilePath());
-     Send(reply_msg);
-  }
-}
-
-void NaClProcessHost::OnResolveFileTokenAsync(uint64 file_token_lo,
-                                              uint64 file_token_hi) {
-  // See the comment at OnResolveFileToken() for details of the file path cache
-  // behavior.
-  CHECK(!uses_nonsfi_mode_);
-  base::FilePath file_path;
-  if (!NaClBrowser::GetInstance()->GetFilePath(
         file_token_lo, file_token_hi, &file_path)) {
     Send(new NaClProcessMsg_ResolveFileTokenAsyncReply(
              file_token_lo,
              file_token_hi,
              IPC::PlatformFileForTransit(),
              base::FilePath()));
     return;
   }
 
   // Open the file.
   if (!base::PostTaskAndReplyWithResult(
           content::BrowserThread::GetBlockingPool(),
           FROM_HERE,
           base::Bind(OpenNaClReadExecImpl, file_path, true /* is_executable */),
           base::Bind(&NaClProcessHost::FileResolvedAsync,
                      weak_factory_.GetWeakPtr(),
                      file_token_lo,
                      file_token_hi,
                      file_path))) {
     Send(new NaClProcessMsg_ResolveFileTokenAsyncReply(
             file_token_lo,
             file_token_hi,
             IPC::PlatformFileForTransit(),
             base::FilePath()));
   }
 }
 
-void NaClProcessHost::FileResolved(
-    const base::FilePath& file_path,
-    IPC::Message* reply_msg,
-    base::File file) {
-  if (file.IsValid()) {
-    IPC::PlatformFileForTransit handle = IPC::TakeFileHandleForProcess(
-        file.Pass(),
-        process_->GetData().handle);
-    NaClProcessMsg_ResolveFileToken::WriteReplyParams(
-        reply_msg,
-        handle,
-        file_path);
-  } else {
-    NaClProcessMsg_ResolveFileToken::WriteReplyParams(
-        reply_msg,
-        IPC::InvalidPlatformFileForTransit(),
-        base::FilePath());
-  }
-  Send(reply_msg);
-}
-
 void NaClProcessHost::FileResolvedAsync(
     uint64_t file_token_lo,
     uint64_t file_token_hi,
     const base::FilePath& file_path,
     base::File file) {
   base::FilePath out_file_path;
   IPC::PlatformFileForTransit out_handle;
   if (file.IsValid()) {
     out_file_path = file_path;
     out_handle = IPC::TakeFileHandleForProcess(
@@ skipping 76 matching lines @@
         process_handle.Take(), info,
         base::MessageLoopProxy::current(),
         base::Bind(&NaClProcessHost::OnDebugExceptionHandlerLaunchedByBroker,
                    weak_factory_.GetWeakPtr()));
     return true;
   }
 }
 #endif
 
 }  // namespace nacl