Reduce resource leak code for StartNaClExecution.

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 176 matching lines @@
 
 void SetCloseOnExec(NaClHandle fd) {
 #if defined(OS_POSIX)
   int flags = fcntl(fd, F_GETFD);
   CHECK_NE(flags, -1);
   int rc = fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
   CHECK_EQ(rc, 0);
 #endif
 }
 
-void CloseFile(base::File file) {
+void PostCloseFile(base::File file) {
+  if (!file.IsValid()) {
+    return;
+  }
+
   // The base::File destructor will close the file for us.
+  content::BrowserThread::GetBlockingPool()->PostTask(
+      FROM_HERE,
+      base::Bind(&ignore_result<base::File>, base::Passed(&file)));
+}
+
+// The maximum number of resource file handles the browser process accepts. Use
+// 200 because ARC's nmf has ~128 resource files as of May 2015. This prevents
+// untrusted code filling the FD/handle table.
+const size_t kMaxPreOpenResourceFiles = 200;
+
+void BatchOpenResourceFiles(
+    const std::vector<NaClResourcePrefetchRequest>& request_list,
+    const base::FilePath& profile_directory,
+    base::ProcessHandle target_process,
+    bool uses_nonsfi_mode,
+    std::vector<NaClResourcePrefetchResult>* result_list) {
+  if (!result_list) {
+    // If output buffer is null, it means there is no resource to be opened.
+    // Do nothing, then.
+    DCHECK(request_list.empty());
+    return;
+  }
+
+  NaClBrowserDelegate* browser_delegate = NaClBrowser::GetDelegate();
+  for (size_t i = 0;
+       i < request_list.size() &&
+           result_list->size() < kMaxPreOpenResourceFiles;
+       ++i) {
+    base::FilePath file_path;
+    if (!browser_delegate->MapUrlToLocalFilePath(
+            GURL(request_list[i].resource_url),
+            true,  // Use blocking API.
+            profile_directory,
+            &file_path)) {
+      continue;
+    }
+
+    base::File file = OpenNaClReadExecImpl(file_path, true /* executable */);
+    if (!file.IsValid())
+      continue;
+
+    // There's no validation caching in Non-SFI mode, so |file_path_metadata|
+    // is unnecessary, and would expose the file path to the plugin.
+    result_list->push_back(NaClResourcePrefetchResult(
+        IPC::TakeFileHandleForProcess(file.Pass(), target_process),
+        uses_nonsfi_mode ? base::FilePath() : file_path,
+        request_list[i].file_key));
+  }
+}
+
+base::File ReopenNexeFile(
+    base::File original_nexe_file, base::FilePath* nexe_file_path) {
+  // If no path is specified, use the original |nexe_file|.
+  if (!nexe_file_path || nexe_file_path->empty())
+    return original_nexe_file.Pass();
+
+  // Reopen the nexe file.
+  base::File reopened_nexe_file = OpenNaClReadExecImpl(
+      *nexe_file_path, true /* executable */);
+  if (!reopened_nexe_file.IsValid()) {
+    // On fail, clear the path, which will eventually passed to the loader.
+    nexe_file_path->clear();
+    return original_nexe_file.Pass();
+  }
+
+  // Note that, the |original_nexe_file| will be closed automatically.
+  return reopened_nexe_file.Pass();
+}
+
+// StartNaClExecution needs file operations. This function takes it, and
+// should run on blocking pool.
+base::File ResolveNaClFile(
+    base::FilePath* nexe_file_path,
+    base::File nexe_file,
+    const std::vector<NaClResourcePrefetchRequest>&
+        resource_prefetch_request_list,
+    const base::FilePath& profile_directory,
+    base::ProcessHandle target_process,
+    bool uses_nonsfi_mode,
+    std::vector<NaClResourcePrefetchResult>* prefetched_resource_files) {
+  BatchOpenResourceFiles(resource_prefetch_request_list,
+                         profile_directory,
+                         target_process,
+                         uses_nonsfi_mode,
+                         prefetched_resource_files);
+  return ReopenNexeFile(nexe_file.Pass(), nexe_file_path);
 }
 
 }  // namespace
 
 unsigned NaClProcessHost::keepalive_throttle_interval_milliseconds_ =
     ppapi::kKeepaliveThrottleIntervalDefaultMilliseconds;
 
 // Unfortunately, we cannot use ScopedGeneric directly for IPC::ChannelHandle,
 // because there is neither operator== nor operator != definition for it.
 // Instead, define a simple wrapper for IPC::ChannelHandle with an assumption
@@ skipping 57 matching lines @@
 #endif
   }
 
   IPC::ChannelHandle handle_;
 };
 
 NaClProcessHost::NaClProcessHost(
     const GURL& manifest_url,
     base::File nexe_file,
     const NaClFileToken& nexe_token,
-    const std::vector<NaClResourcePrefetchResult>& prefetched_resource_files,
+    const std::vector<NaClResourcePrefetchRequest>&
+        resource_prefetch_request_list,
     ppapi::PpapiPermissions permissions,
     int render_view_id,
     uint32 permission_bits,
     bool uses_nonsfi_mode,
     bool off_the_record,
     NaClAppProcessType process_type,
     const base::FilePath& profile_directory)
     : manifest_url_(manifest_url),
       nexe_file_(nexe_file.Pass()),
       nexe_token_(nexe_token),
-      prefetched_resource_files_(prefetched_resource_files),
+      resource_prefetch_request_list_(resource_prefetch_request_list),
       permissions_(permissions),
 #if defined(OS_WIN)
       process_launched_by_broker_(false),
 #endif
       reply_msg_(NULL),
 #if defined(OS_WIN)
       debug_exception_handler_requested_(false),
 #endif
       uses_nonsfi_mode_(uses_nonsfi_mode),
       enable_debug_stub_(false),
@@ skipping 12 matching lines @@
   // for this use case.
   process_->SetName(net::FormatUrl(manifest_url_, std::string()));
 
   enable_debug_stub_ = base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableNaClDebug);
   DCHECK(process_type_ != kUnknownNaClProcessType);
   enable_crash_throttling_ = process_type_ != kNativeNaClProcessType;
 }
 
 NaClProcessHost::~NaClProcessHost() {
+  PostCloseFile(nexe_file_.Pass());
+  PostCloseFile(socket_for_renderer_.Pass());
+  PostCloseFile(socket_for_sel_ldr_.Pass());
+
   // Report exit status only if the process was successfully started.
   if (process_->GetData().handle != base::kNullProcessHandle) {
     int exit_code = 0;
     process_->GetTerminationStatus(false /* known_dead */, &exit_code);
     std::string message =
         base::StringPrintf("NaCl process exited with status %i (0x%x)",
                            exit_code, exit_code);
     if (exit_code == 0) {
       VLOG(1) << message;
     } else {
       LOG(ERROR) << message;
     }
     NaClBrowser::GetInstance()->OnProcessEnd(process_->GetData().id);
   }
 
-  // Note: this does not work on Windows, though we currently support this
-  // prefetching feature only on POSIX platforms, so it should be ok.
-#if defined(OS_WIN)
-  DCHECK(prefetched_resource_files_.empty());
-#else
-  for (size_t i = 0; i < prefetched_resource_files_.size(); ++i) {
-    // The process failed to launch for some reason. Close resource file
-    // handles.
-    base::File file(IPC::PlatformFileForTransitToFile(
-        prefetched_resource_files_[i].file));
-    content::BrowserThread::GetBlockingPool()->PostTask(
-        FROM_HERE,
-        base::Bind(&CloseFile, base::Passed(file.Pass())));
-  }
-#endif
-
   if (reply_msg_) {
     // The process failed to launch for some reason.
     // Don't keep the renderer hanging.
     reply_msg_->set_reply_error();
     nacl_host_message_filter_->Send(reply_msg_);
   }
 #if defined(OS_WIN)
   if (process_launched_by_broker_) {
     NaClBrokerService::GetInstance()->OnLoaderDied();
   }
@@ skipping 497 matching lines @@
 bool NaClProcessHost::StartNaClExecution() {
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
 
   NaClStartParams params;
 
   // Enable PPAPI proxy channel creation only for renderer processes.
   params.enable_ipc_proxy = enable_ppapi_proxy();
   params.process_type = process_type_;
   bool enable_nacl_debug = enable_debug_stub_ &&
       NaClBrowser::GetDelegate()->URLMatchesDebugPatterns(manifest_url_);
+#if defined(OS_MACOSX)
+  base::ScopedFD memory_fd;
+#endif  // defined(OS_MACOSX)
+#if defined(OS_POSIX)
+  base::ScopedFD debug_stub_server_bound_socket;
+#endif  // defined(OS_POSIX)
+
   if (uses_nonsfi_mode_) {
     // Currently, non-SFI mode is supported only on Linux.
 #if defined(OS_LINUX)
     // In non-SFI mode, we do not use SRPC. Make sure that the socketpair is
     // not created.
     DCHECK(!socket_for_sel_ldr_.IsValid());
 #endif
     if (enable_nacl_debug) {
       base::ProcessId pid = base::GetProcId(process_->GetData().handle);
       LOG(WARNING) << "nonsfi nacl plugin running in " << pid;
     }
   } 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_nacl_debug;
 
-    const ChildProcessData& data = process_->GetData();
-    params.imc_bootstrap_handle =
-        IPC::TakeFileHandleForProcess(socket_for_sel_ldr_.Pass(), data.handle);
-    if (params.imc_bootstrap_handle == IPC::InvalidPlatformFileForTransit()) {
-      return false;
-    }
-
-    const base::File& irt_file = nacl_browser->IrtFile();
-    CHECK(irt_file.IsValid());
-    // Send over the IRT file handle.  We don't close our own copy!
-    params.irt_handle = IPC::GetFileHandleForProcess(
-        irt_file.GetPlatformFile(), data.handle, false);
-    if (params.irt_handle == IPC::InvalidPlatformFileForTransit()) {
-      return false;
-    }
+    CHECK(nacl_browser->IrtFile().IsValid());
 
 #if defined(OS_MACOSX)
     // For dynamic loading support, NaCl requires a file descriptor that
     // was created in /tmp, since those created with shm_open() are not
     // mappable with PROT_EXEC.  Rather than requiring an extra IPC
     // round trip out of the sandbox, we create an FD here.
     base::SharedMemory memory_buffer;
     base::SharedMemoryCreateOptions options;
     options.size = 1;
     options.executable = true;
     if (!memory_buffer.Create(options)) {
       DLOG(ERROR) << "Failed to allocate memory buffer";
       return false;
     }
-    base::ScopedFD memory_fd(dup(memory_buffer.handle().fd));
+    memory_fd.reset(dup(memory_buffer.handle().fd));
     if (!memory_fd.is_valid()) {
       DLOG(ERROR) << "Failed to dup() a file descriptor";
       return false;
     }
-    params.mac_shm_fd = IPC::GetFileHandleForProcess(
-        memory_fd.release(), data.handle, true);
 #endif
 
 #if defined(OS_POSIX)
-    if (params.enable_debug_stub) {
-      net::SocketDescriptor server_bound_socket = GetDebugStubSocketHandle();
-      if (server_bound_socket != net::kInvalidSocket) {
-        params.debug_stub_server_bound_socket = IPC::GetFileHandleForProcess(
-            server_bound_socket, data.handle, true);
-      }
-    }
+    if (params.enable_debug_stub)
+      debug_stub_server_bound_socket.reset(GetDebugStubSocketHandle());
 #endif
   }
 
-  if (!crash_info_shmem_.ShareToProcess(process_->GetData().handle,
-                                        &params.crash_info_shmem_handle)) {
-    DLOG(ERROR) << "Failed to ShareToProcess() a shared memory buffer";
-    return false;
-  }
+  // Transfer resources to |params|.
+  // Hereafter, we should never return false, and should always send an IPC
+  // to NaCl plugin process. Otherwise, the resources passed to |params| may
+  // be leaked. Note that these are resources for Plugin process, so we
+  // cannot properly release those in this (browser) process.
+  {
+    const ChildProcessData& data = process_->GetData();
+    if (!uses_nonsfi_mode_) {
+      params.imc_bootstrap_handle = IPC::TakeFileHandleForProcess(
+          socket_for_sel_ldr_.Pass(), data.handle);
+      // Send over the IRT file handle.  We don't close our own copy!
+      params.irt_handle = IPC::GetFileHandleForProcess(
+          nacl_browser->IrtFile().GetPlatformFile(), data.handle, false);
 
-  // Pass the pre-opened resource files to the loader. We do not have to reopen
-  // resource files here even for SFI mode because the descriptors are not from
-  // a renderer.
-  for (size_t i = 0; i < prefetched_resource_files_.size(); ++i) {
-    process_->Send(new NaClProcessMsg_AddPrefetchedResource(
-        NaClResourcePrefetchResult(
-            prefetched_resource_files_[i].file,
-            // For the same reason as the comment below, always use an empty
-            // base::FilePath for non-SFI mode.
-            (uses_nonsfi_mode_ ? base::FilePath() :
-             prefetched_resource_files_[i].file_path_metadata),
-            prefetched_resource_files_[i].file_key)));
-  }
-  prefetched_resource_files_.clear();
+#if defined(OS_MACOSX)
+      params.mac_shm_fd = IPC::GetFileHandleForProcess(
+          memory_fd.release(), data.handle, true);
+#endif
+#if defined(OS_POSIX)
+      params.debug_stub_server_bound_socket = IPC::GetFileHandleForProcess(
+          debug_stub_server_bound_socket.release(), data.handle, true);
+#endif
+    }
 
-  base::FilePath file_path;
-  if (uses_nonsfi_mode_) {
-    // Don't retrieve the file path when using nonsfi mode; there's no
-    // validation caching in that case, so it's unnecessary work, and would
-    // expose the file path to the plugin.
-  } else {
-    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;
-      }
+    if (!crash_info_shmem_.ShareToProcess(data.handle,
+                                          &params.crash_info_shmem_handle)) {
+      DLOG(ERROR) << "Failed to ShareToProcess() a shared memory buffer";
+      // Do not return here.
     }
   }
 
-  StartNaClFileResolved(params, base::FilePath(), base::File());
+  // 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.
+  // Don't retrieve the file path when using nonsfi mode; there's no
+  // validation caching in that case, so it's unnecessary work, and would
+  // expose the file path to the plugin.
+  scoped_ptr<base::FilePath> nexe_file_path;
+  if (!uses_nonsfi_mode_) {
+    nexe_file_path.reset(new base::FilePath);
+    if (!NaClBrowser::GetInstance()->GetFilePath(
+            nexe_token_.lo, nexe_token_.hi, nexe_file_path.get())) {
+      // Failed. Reset the pointer.
+      nexe_file_path.reset();
+    }
+  }
+
+  // Pass the pre-opened resource files to the loader.
+  scoped_ptr<std::vector<NaClResourcePrefetchResult> >
+      prefetched_resource_files;
+  if (!resource_prefetch_request_list_.empty()) {
+    prefetched_resource_files.reset(
+        new std::vector<NaClResourcePrefetchResult>);
+  }
+
+  // If file operation is necessary, run it on a blocking pool, where file
+  // operations are allowed.
+  if (nexe_file_path.get() || prefetched_resource_files.get()) {
+    if (base::PostTaskAndReplyWithResult(
+            content::BrowserThread::GetBlockingPool(),
+            FROM_HERE,
+            base::Bind(&ResolveNaClFile,
+                       nexe_file_path.get(),
+                       base::Passed(&nexe_file_),
+                       resource_prefetch_request_list_,
+                       profile_directory_,
+                       process_->GetData().handle,
+                       uses_nonsfi_mode_,
+                       prefetched_resource_files.get()),
+            base::Bind(&NaClProcessHost::StartNaClExecutionAfterFileResolved,
+                       weak_factory_.GetWeakPtr(),
+                       params,
+                       base::Passed(&nexe_file_path),
+                       base::Passed(&prefetched_resource_files)))) {
+      return true;
+    }
+  }
+
+  StartNaClExecutionAfterFileResolved(
+      params,
+      scoped_ptr<base::FilePath>(),
+      scoped_ptr<std::vector<NaClResourcePrefetchResult> >(),
+      nexe_file_.Pass());
   return true;
 }
 
-void NaClProcessHost::StartNaClFileResolved(
+void NaClProcessHost::StartNaClExecutionAfterFileResolved(
     NaClStartParams params,
-    const base::FilePath& file_path,
-    base::File checked_nexe_file) {
-  if (checked_nexe_file.IsValid()) {
-    // Release the file received from the renderer. This has to be done on a
-    // thread where IO is permitted, though.
-    content::BrowserThread::GetBlockingPool()->PostTask(
-        FROM_HERE,
-        base::Bind(&CloseFile, base::Passed(nexe_file_.Pass())));
-    params.nexe_file_path_metadata = file_path;
-    params.nexe_file = IPC::TakeFileHandleForProcess(
-        checked_nexe_file.Pass(), process_->GetData().handle);
-  } else {
-    params.nexe_file = IPC::TakeFileHandleForProcess(
-        nexe_file_.Pass(), process_->GetData().handle);
+    scoped_ptr<base::FilePath> nexe_file_path,
+    scoped_ptr<std::vector<NaClResourcePrefetchResult> >
+        prefetched_resource_files,
+    base::File nexe_file) {
+  // Pass the pre-opened resource files to the loader.
+  if (prefetched_resource_files.get()) {
+    for (size_t i = 0; i < prefetched_resource_files->size(); ++i) {
+      process_->Send(new NaClProcessMsg_AddPrefetchedResource(
+          (*prefetched_resource_files)[i]));
+    }
   }
 
 #if defined(OS_LINUX)
   // In Non-SFI mode, create socket pairs for IPC channels here, unlike in
   // SFI-mode, in which those channels are created in nacl_listener.cc.
   // This is for security hardening. We can then prohibit the socketpair()
   // system call in nacl_helper and nacl_helper_nonsfi.
   if (uses_nonsfi_mode_) {
     // Note: here, because some FDs/handles for the NaCl loader process are
     // already opened, they are transferred to NaCl loader process even if
@@ skipping 44 matching lines @@
       params.ppapi_renderer_channel_handle =
           ppapi_renderer_server_channel_handle.release();
       params.trusted_service_channel_handle =
           trusted_service_server_channel_handle.release();
       params.manifest_service_channel_handle =
           manifest_service_server_channel_handle.release();
     }
   }
 #endif
 
+  // Pass the nexe file and its path to params.
+  params.nexe_file = IPC::TakeFileHandleForProcess(
+      nexe_file.Pass(), process_->GetData().handle);
+  if (nexe_file_path.get()) {
+    params.nexe_file_path_metadata = *nexe_file_path;
+  }
+
   process_->Send(new NaClProcessMsg_Start(params));
 }
 
 #if defined(OS_LINUX)
 // static
 bool NaClProcessHost::CreateChannelHandlePair(
     ScopedChannelHandle* channel_handle1,
     ScopedChannelHandle* channel_handle2) {
   DCHECK(channel_handle1);
   DCHECK(channel_handle2);
@@ skipping 277 matching lines @@
         process.Pass(), info,
         base::MessageLoopProxy::current(),
         base::Bind(&NaClProcessHost::OnDebugExceptionHandlerLaunchedByBroker,
                    weak_factory_.GetWeakPtr()));
     return true;
   }
 }
 #endif
 
 }  // namespace nacl