Remove the socket_count parameter from NaCl Launch IPC messages.

chrome/browser/nacl_host/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 "chrome/browser/nacl_host/nacl_process_host.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 109 matching lines @@
   // Only allow NaCl plugins to request certain permissions. We don't want
   // a compromised renderer to be able to start a nacl plugin with e.g. Flash
   // permissions which may expand the surface area of the sandbox.
   uint32 masked_bits = permission_bits & ppapi::PERMISSION_DEV;
   return ppapi::PpapiPermissions::GetForCommandLine(masked_bits);
 }
 
 }  // namespace
 
 struct NaClProcessHost::NaClInternal {
-  std::vector<nacl::Handle> sockets_for_renderer;
+  nacl::Handle socket_for_renderer;
   std::vector<nacl::Handle> sockets_for_sel_ldr;

[Mark Seaborn, 2013/01/08 00:40:57]

You can simplify sockets_for_sel_ldr too.  It's only appended to in the same
place as sockets_for_renderer, AFAICS.

[jvoung (off chromium), 2013/01/08 19:35:39]

Done.

+
+  NaClInternal() : socket_for_renderer(nacl::kInvalidHandle) { }
 };
 
 // -----------------------------------------------------------------------------
 
 NaClProcessHost::PluginListener::PluginListener(NaClProcessHost* host)
     : host_(host) {
 }
 
 bool NaClProcessHost::PluginListener::OnMessageReceived(
     const IPC::Message& msg) {
@@ skipping 55 matching lines @@
   process_->GetTerminationStatus(&exit_code);
   std::string message =
       base::StringPrintf("NaCl process exited with status %i (0x%x)",
                          exit_code, exit_code);
   if (exit_code == 0) {
     LOG(INFO) << message;
   } else {
     LOG(ERROR) << message;
   }
 
-  for (size_t i = 0; i < internal_->sockets_for_renderer.size(); i++) {
-    if (nacl::Close(internal_->sockets_for_renderer[i]) != 0) {
+  if (internal_->socket_for_renderer != nacl::kInvalidHandle) {
+    if (nacl::Close(internal_->socket_for_renderer) != 0) {
       NOTREACHED() << "nacl::Close() failed";
     }
   }
+
   for (size_t i = 0; i < internal_->sockets_for_sel_ldr.size(); i++) {
     if (nacl::Close(internal_->sockets_for_sel_ldr[i]) != 0) {
       NOTREACHED() << "nacl::Close() failed";
     }
   }
 
   if (reply_msg_) {
     // The process failed to launch for some reason.
     // Don't keep the renderer hanging.
     reply_msg_->set_reply_error();
@@ skipping 23 matching lines @@
       !cmd->GetSwitchValuePath(switches::kNaClGdbScript).empty());
   UMA_HISTOGRAM_BOOLEAN(
       "NaCl.enable-nacl-debug",
       cmd->HasSwitch(switches::kEnableNaClDebug));
   NaClBrowser::GetInstance()->SetDebugPatterns(
       cmd->GetSwitchValueASCII(switches::kNaClDebugMask));
 }
 
 void NaClProcessHost::Launch(
     ChromeRenderMessageFilter* chrome_render_message_filter,
-    int socket_count,
     IPC::Message* reply_msg,
     scoped_refptr<ExtensionInfoMap> extension_info_map) {
   chrome_render_message_filter_ = chrome_render_message_filter;
   reply_msg_ = reply_msg;
   extension_info_map_ = extension_info_map;
 
-  // Place an arbitrary limit on the number of sockets to limit
-  // exposure in case the renderer is compromised.  We can increase
-  // this if necessary.
-  if (socket_count > 8) {
-    delete this;
-    return;
-  }
-
   // Start getting the IRT open asynchronously while we launch the NaCl process.
   // We'll make sure this actually finished in StartWithLaunchedProcess, below.
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
   nacl_browser->EnsureAllResourcesAvailable();
   if (!nacl_browser->IsOk()) {
     DLOG(ERROR) << "Cannot launch NaCl process";
     delete this;
     return;
   }
 
   // Rather than creating a socket pair in the renderer, and passing
   // one side through the browser to sel_ldr, socket pairs are created
   // in the browser and then passed to the renderer and sel_ldr.
   //
   // This is mainly for the benefit of Windows, where sockets cannot
   // be passed in messages, but are copied via DuplicateHandle().
   // This means the sandboxed renderer cannot send handles to the
   // browser process.
 
-  for (int i = 0; i < socket_count; i++) {
-    nacl::Handle pair[2];
-    // Create a connected socket
-    if (nacl::SocketPair(pair) == -1) {
-      delete this;
-      return;
-    }
-    internal_->sockets_for_renderer.push_back(pair[0]);
-    internal_->sockets_for_sel_ldr.push_back(pair[1]);
-    SetCloseOnExec(pair[0]);
-    SetCloseOnExec(pair[1]);
+  nacl::Handle pair[2];
+  // Create a connected socket
+  if (nacl::SocketPair(pair) == -1) {
+    delete this;
+    return;
   }
+  internal_->socket_for_renderer = pair[0];
+  internal_->sockets_for_sel_ldr.push_back(pair[1]);
+  SetCloseOnExec(pair[0]);
+  SetCloseOnExec(pair[1]);
 
   // Launch the process
   if (!LaunchSelLdr()) {
     delete this;
   }
 }
 
 #if defined(OS_WIN)
 void NaClProcessHost::OnChannelConnected(int32 peer_pid) {
   // Set process handle, if it was not set previously.
@@ skipping 300 matching lines @@
 void NaClProcessHost::OnResourcesReady() {
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
   if (!nacl_browser->IsReady() || !SendStart()) {
     DLOG(ERROR) << "Cannot launch NaCl process";
     delete this;
   }
 }
 
 bool NaClProcessHost::ReplyToRenderer(
     const IPC::ChannelHandle& channel_handle) {
-  std::vector<nacl::FileDescriptor> handles_for_renderer;
-  for (size_t i = 0; i < internal_->sockets_for_renderer.size(); i++) {
+  nacl::FileDescriptor handle_for_renderer;
 #if defined(OS_WIN)
-    // Copy the handle into the renderer process.
-    HANDLE handle_in_renderer;
-    if (!DuplicateHandle(base::GetCurrentProcessHandle(),
-                         reinterpret_cast<HANDLE>(
-                             internal_->sockets_for_renderer[i]),
-                         chrome_render_message_filter_->peer_handle(),
-                         &handle_in_renderer,
-                         0,  // Unused given DUPLICATE_SAME_ACCESS.
-                         FALSE,
-                         DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) {
-      DLOG(ERROR) << "DuplicateHandle() failed";
-      return false;
-    }
-    handles_for_renderer.push_back(
-        reinterpret_cast<nacl::FileDescriptor>(handle_in_renderer));
+  // Copy the handle into the renderer process.
+  HANDLE handle_in_renderer;
+  if (!DuplicateHandle(base::GetCurrentProcessHandle(),
+                       reinterpret_cast<HANDLE>(
+                           internal_->socket_for_renderer),
+                       chrome_render_message_filter_->peer_handle(),
+                       &handle_in_renderer,
+                       0,  // Unused given DUPLICATE_SAME_ACCESS.
+                       FALSE,
+                       DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) {
+    DLOG(ERROR) << "DuplicateHandle() failed";
+    return false;
+  }
+  handle_for_renderer = reinterpret_cast<nacl::FileDescriptor>(
+      handle_in_renderer);
 #else
-    // No need to dup the imc_handle - we don't pass it anywhere else so
-    // it cannot be closed.
-    nacl::FileDescriptor imc_handle;
-    imc_handle.fd = internal_->sockets_for_renderer[i];
-    imc_handle.auto_close = true;
-    handles_for_renderer.push_back(imc_handle);
+  // No need to dup the imc_handle - we don't pass it anywhere else so
+  // it cannot be closed.
+  nacl::FileDescriptor imc_handle;
+  imc_handle.fd = internal_->socket_for_renderer;
+  imc_handle.auto_close = true;
+  handle_for_renderer = imc_handle;
 #endif
-  }
 
 #if defined(OS_WIN)
   // If we are on 64-bit Windows, the NaCl process's sandbox is
   // managed by a different process from the renderer's sandbox.  We
   // need to inform the renderer's sandbox about the NaCl process so
   // that the renderer can send handles to the NaCl process using
   // BrokerDuplicateHandle().
   if (RunningOnWOW64()) {
     if (!content::BrokerAddTargetPeer(process_->GetData().handle)) {
       DLOG(ERROR) << "Failed to add NaCl process PID";
       return false;
     }
   }
 #endif
 
   const ChildProcessData& data = process_->GetData();
   ChromeViewHostMsg_LaunchNaCl::WriteReplyParams(
-      reply_msg_, handles_for_renderer,
+      reply_msg_, handle_for_renderer,
       channel_handle, base::GetProcId(data.handle), data.id);
   chrome_render_message_filter_->Send(reply_msg_);
   chrome_render_message_filter_ = NULL;
   reply_msg_ = NULL;
-  internal_->sockets_for_renderer.clear();
+  internal_->socket_for_renderer = nacl::kInvalidHandle;
   return true;
 }
 
 // TCP port we chose for NaCl debug stub. It can be any other number.
 static const int kDebugStubPort = 4014;
 
 #if defined(OS_POSIX)
 SocketDescriptor NaClProcessHost::GetDebugStubSocketHandle() {
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
   SocketDescriptor s;
@@ skipping 250 matching lines @@
   } else {
     NaClStartDebugExceptionHandlerThread(
         process_handle.Take(), info,
         base::MessageLoopProxy::current(),
         base::Bind(&NaClProcessHost::OnDebugExceptionHandlerLaunchedByBroker,
                    weak_factory_.GetWeakPtr()));
     return true;
   }
 }
 #endif