Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7652)

Unified Diff: chrome/browser/nacl_host/nacl_process_host.cc

Issue 2832093: NaCl: Allow setting up multiple sockets for subprocess instead of just one (Closed) Base URL: git://codf21.jail/chromium.git
Patch Set: Whitespace fixes Created 10 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/nacl_host/nacl_process_host.cc
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index fb844d85249e14759ba2fda96b42c5e710bf9ea5..8ae96371ea33ca01f7df86dc7e7ebf99c95c35b2 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -25,13 +25,25 @@
#include "chrome/browser/nacl_host/nacl_broker_service_win.h"
#endif
+namespace {
+
+void SetCloseOnExec(nacl::Handle fd) {
+#if defined(OS_POSIX)
+ int flags = fcntl(fd, F_GETFD);
+ CHECK(flags != -1);
+ int rc = fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
+ CHECK(rc == 0);
+#endif
+}
+
+} // namespace
+
NaClProcessHost::NaClProcessHost(
ResourceDispatcherHost *resource_dispatcher_host,
const std::wstring& url)
: BrowserChildProcessHost(NACL_LOADER_PROCESS, resource_dispatcher_host),
resource_dispatcher_host_(resource_dispatcher_host),
reply_msg_(NULL),
- descriptor_(0),
running_on_wow64_(false) {
set_name(url);
#if defined(OS_WIN)
@@ -43,6 +55,13 @@ NaClProcessHost::~NaClProcessHost() {
if (!reply_msg_)
return;
+ for (size_t i = 0; i < sockets_for_renderer_.size(); i++) {
+ nacl::Close(sockets_for_renderer_[i]);
+ }
+ for (size_t i = 0; i < sockets_for_sel_ldr_.size(); i++) {
+ nacl::Close(sockets_for_sel_ldr_[i]);
+ }
+
// OnProcessLaunched didn't get called because the process couldn't launch.
// Don't keep the renderer hanging.
reply_msg_->set_reply_error();
@@ -50,20 +69,41 @@ NaClProcessHost::~NaClProcessHost() {
}
bool NaClProcessHost::Launch(ResourceMessageFilter* resource_message_filter,
- const int descriptor,
+ int socket_count,
IPC::Message* reply_msg) {
#ifdef DISABLE_NACL
NOTIMPLEMENTED() << "Native Client disabled at build time";
return false;
#else
- // Create a connected socket
- if (nacl::SocketPair(pair_) == -1)
+ // 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) {
return false;
+ }
+
+ // 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)
+ return false;
+ sockets_for_renderer_.push_back(pair[0]);
+ sockets_for_sel_ldr_.push_back(pair[1]);
+ SetCloseOnExec(pair[0]);
+ SetCloseOnExec(pair[1]);
+ }
// Launch the process
- descriptor_ = descriptor;
if (!LaunchSelLdr()) {
- nacl::Close(pair_[0]);
return false;
}
@@ -129,22 +169,34 @@ void NaClProcessHost::OnChildDied() {
}
void NaClProcessHost::OnProcessLaunched() {
- nacl::FileDescriptor imc_handle;
+ std::vector<nacl::FileDescriptor> handles_for_renderer;
base::ProcessHandle nacl_process_handle;
+
+ for (size_t i = 0; i < sockets_for_renderer_.size(); i++) {
#if defined(OS_WIN)
- // Duplicate the IMC handle
- // We assume the size of imc_handle has the same size as HANDLE, so the cast
- // below is safe.
- DCHECK(sizeof(HANDLE) == sizeof(imc_handle));
- DuplicateHandle(base::GetCurrentProcessHandle(),
- reinterpret_cast<HANDLE>(pair_[0]),
- resource_message_filter_->handle(),
- reinterpret_cast<HANDLE*>(&imc_handle),
- GENERIC_READ | GENERIC_WRITE,
- FALSE,
- DUPLICATE_CLOSE_SOURCE);
+ // Copy the handle into the renderer process.
+ HANDLE handle_in_renderer;
+ DuplicateHandle(base::GetCurrentProcessHandle(),
+ reinterpret_cast<HANDLE>(sockets_for_renderer_[i]),
+ resource_message_filter_->handle(),
+ &handle_in_renderer,
+ GENERIC_READ | GENERIC_WRITE,
+ FALSE,
+ DUPLICATE_CLOSE_SOURCE);
+ handles_for_renderer.push_back(
+ 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 = sockets_for_renderer_[i];
+ imc_handle.auto_close = true;
+ handles_for_renderer.push_back(imc_handle);
+#endif
+ }
- // Duplicate the process handle
+#if defined(OS_WIN)
+ // Copy the process handle into the renderer process.
DuplicateHandle(base::GetCurrentProcessHandle(),
handle(),
resource_message_filter_->handle(),
@@ -153,16 +205,6 @@ void NaClProcessHost::OnProcessLaunched() {
FALSE,
0);
#else
- int flags = fcntl(pair_[0], F_GETFD);
- if (flags != -1) {
- flags |= FD_CLOEXEC;
- fcntl(pair_[0], F_SETFD, flags);
- }
- // No need to dup the imc_handle - we don't pass it anywhere else so
- // it cannot be closed.
- imc_handle.fd = pair_[0];
- imc_handle.auto_close = true;
-
// We use pid as process handle on Posix
nacl_process_handle = handle();
#endif
@@ -171,30 +213,40 @@ void NaClProcessHost::OnProcessLaunched() {
base::ProcessId nacl_process_id = base::GetProcId(handle());
ViewHostMsg_LaunchNaCl::WriteReplyParams(
- reply_msg_, imc_handle, nacl_process_handle, nacl_process_id);
+ reply_msg_, handles_for_renderer, nacl_process_handle, nacl_process_id);
resource_message_filter_->Send(reply_msg_);
resource_message_filter_ = NULL;
reply_msg_ = NULL;
+ sockets_for_renderer_.clear();
SendStartMessage();
}
void NaClProcessHost::SendStartMessage() {
- nacl::FileDescriptor channel;
+ std::vector<nacl::FileDescriptor> handles_for_sel_ldr;
+ for (size_t i = 0; i < sockets_for_sel_ldr_.size(); i++) {
#if defined(OS_WIN)
- if (!DuplicateHandle(GetCurrentProcess(),
- reinterpret_cast<HANDLE>(pair_[1]),
- handle(),
- reinterpret_cast<HANDLE*>(&channel),
- GENERIC_READ | GENERIC_WRITE,
- FALSE, DUPLICATE_CLOSE_SOURCE)) {
- return;
- }
+ HANDLE channel;
+ if (!DuplicateHandle(GetCurrentProcess(),
+ reinterpret_cast<HANDLE>(sockets_for_sel_ldr_[i]),
+ handle(),
+ &channel,
+ GENERIC_READ | GENERIC_WRITE,
+ FALSE, DUPLICATE_CLOSE_SOURCE)) {
+ return;
+ }
+ handles_for_sel_ldr.push_back(
+ reinterpret_cast<nacl::FileDescriptor>(channel));
#else
- channel.fd = dup(pair_[1]);
- channel.auto_close = true;
+ nacl::FileDescriptor channel;
+ channel.fd = dup(sockets_for_sel_ldr_[i]);
+ channel.auto_close = true;
+ handles_for_sel_ldr.push_back(channel);
#endif
- Send(new NaClProcessMsg_Start(descriptor_, channel));
+ }
+
+ Send(new NaClProcessMsg_Start(handles_for_sel_ldr));
+ sockets_for_sel_ldr_.clear();
}
void NaClProcessHost::OnMessageReceived(const IPC::Message& msg) {

Powered by Google App Engine
This is Rietveld 408576698