diff options
-rw-r--r-- | chrome/browser/nacl_host/nacl_broker_host_win.cc | 13 | ||||
-rw-r--r-- | chrome/browser/nacl_host/nacl_broker_host_win.h | 3 | ||||
-rw-r--r-- | chrome/browser/nacl_host/nacl_broker_service_win.cc | 5 | ||||
-rw-r--r-- | chrome/browser/nacl_host/nacl_broker_service_win.h | 3 | ||||
-rw-r--r-- | chrome/browser/nacl_host/nacl_process_host.cc | 29 | ||||
-rw-r--r-- | chrome/common/nacl_debug_exception_handler_win.cc | 35 | ||||
-rw-r--r-- | chrome/common/nacl_debug_exception_handler_win.h | 3 | ||||
-rw-r--r-- | chrome/common/nacl_messages.h | 5 | ||||
-rw-r--r-- | chrome/nacl/nacl_broker_listener.cc | 6 | ||||
-rw-r--r-- | chrome/nacl/nacl_broker_listener.h | 3 | ||||
-rw-r--r-- | ipc/ipc_message_utils.h | 11 |
11 files changed, 76 insertions, 40 deletions
diff --git a/chrome/browser/nacl_host/nacl_broker_host_win.cc b/chrome/browser/nacl_host/nacl_broker_host_win.cc index 50f3706..d2a7aad 100644 --- a/chrome/browser/nacl_host/nacl_broker_host_win.cc +++ b/chrome/browser/nacl_host/nacl_broker_host_win.cc @@ -15,6 +15,7 @@ #include "chrome/common/nacl_cmd_line.h" #include "chrome/common/nacl_messages.h" #include "content/public/browser/browser_child_process_host.h" +#include "content/public/browser/child_process_data.h" #include "content/public/common/child_process_host.h" NaClBrokerHost::NaClBrokerHost() { @@ -71,8 +72,16 @@ void NaClBrokerHost::OnLoaderLaunched(const std::string& loader_channel_id, NaClBrokerService::GetInstance()->OnLoaderLaunched(loader_channel_id, handle); } -bool NaClBrokerHost::LaunchDebugExceptionHandler(int32 pid) { - return process_->Send(new NaClProcessMsg_LaunchDebugExceptionHandler(pid)); +bool NaClBrokerHost::LaunchDebugExceptionHandler( + int32 pid, base::ProcessHandle process_handle) { + base::ProcessHandle broker_process = process_->GetData().handle; + base::ProcessHandle handle_in_broker_process; + if (!DuplicateHandle(::GetCurrentProcess(), process_handle, + broker_process, &handle_in_broker_process, + 0, /* bInheritHandle= */ FALSE, DUPLICATE_SAME_ACCESS)) + return false; + return process_->Send(new NaClProcessMsg_LaunchDebugExceptionHandler( + pid, handle_in_broker_process)); } void NaClBrokerHost::OnDebugExceptionHandlerLaunched(int32 pid) { diff --git a/chrome/browser/nacl_host/nacl_broker_host_win.h b/chrome/browser/nacl_host/nacl_broker_host_win.h index b44a094..2ed061c 100644 --- a/chrome/browser/nacl_host/nacl_broker_host_win.h +++ b/chrome/browser/nacl_host/nacl_broker_host_win.h @@ -28,7 +28,8 @@ class NaClBrokerHost : public content::BrowserChildProcessHostDelegate { // a Native Client loader process. bool LaunchLoader(const std::string& loader_channel_id); - bool LaunchDebugExceptionHandler(int32 pid); + bool LaunchDebugExceptionHandler(int32 pid, + base::ProcessHandle process_handle); // Stop the broker process. void StopBroker(); diff --git a/chrome/browser/nacl_host/nacl_broker_service_win.cc b/chrome/browser/nacl_host/nacl_broker_service_win.cc index da91ef9..9b1a748 100644 --- a/chrome/browser/nacl_host/nacl_broker_service_win.cc +++ b/chrome/browser/nacl_host/nacl_broker_service_win.cc @@ -67,12 +67,13 @@ void NaClBrokerService::OnLoaderDied() { } bool NaClBrokerService::LaunchDebugExceptionHandler( - base::WeakPtr<NaClProcessHost> nacl_process_host, int32 pid) { + base::WeakPtr<NaClProcessHost> nacl_process_host, int32 pid, + base::ProcessHandle process_handle) { pending_debuggers_[pid] = nacl_process_host; NaClBrokerHost* broker_host = GetBrokerHost(); if (!broker_host) return false; - return broker_host->LaunchDebugExceptionHandler(pid); + return broker_host->LaunchDebugExceptionHandler(pid, process_handle); } void NaClBrokerService::OnDebugExceptionHandlerLaunched(int32 pid) { diff --git a/chrome/browser/nacl_host/nacl_broker_service_win.h b/chrome/browser/nacl_host/nacl_broker_service_win.h index 78a891d..e6794a8 100644 --- a/chrome/browser/nacl_host/nacl_broker_service_win.h +++ b/chrome/browser/nacl_host/nacl_broker_service_win.h @@ -36,7 +36,8 @@ class NaClBrokerService { void OnLoaderDied(); bool LaunchDebugExceptionHandler(base::WeakPtr<NaClProcessHost> client, - int32 pid); + int32 pid, + base::ProcessHandle process_handle); // Called by NaClBrokerHost to notify the service that a debug // exception handler was started. diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc index 122c8db..e58d747 100644 --- a/chrome/browser/nacl_host/nacl_process_host.cc +++ b/chrome/browser/nacl_host/nacl_process_host.cc @@ -49,6 +49,7 @@ #include "base/threading/thread.h" #include "base/process_util.h" +#include "base/win/scoped_handle.h" #include "chrome/browser/nacl_host/nacl_broker_service_win.h" #include "content/public/common/sandbox_init.h" #include "native_client/src/trusted/service_runtime/win/debug_exception_handler.h" @@ -572,9 +573,33 @@ void NaClProcessHost::OnChannelConnected(int32 peer_pid) { } debug_context_->SetNaClProcessHost(weak_factory_.GetWeakPtr()); if (RunningOnWOW64()) { - if (!NaClBrokerService::GetInstance()->LaunchDebugExceptionHandler( - weak_factory_.GetWeakPtr(), peer_pid)) { + base::win::ScopedHandle process_handle; + // We cannot use process_->GetData().handle because it does not + // have the necessary access rights. We open the new handle here + // rather than in the NaCl broker process in case the NaCl loader + // process dies before the NaCl broker process receives the + // message we send. The debug exception handler uses + // DebugActiveProcess() to attach, but this takes a PID. We need + // to prevent the NaCl loader's PID from being reused before + // DebugActiveProcess() is called, and holding a process handle + // open achieves this. + if (!base::OpenProcessHandleWithAccess( + peer_pid, + base::kProcessAccessQueryInformation | + base::kProcessAccessSuspendResume | + base::kProcessAccessTerminate | + base::kProcessAccessVMOperation | + base::kProcessAccessVMRead | + base::kProcessAccessVMWrite | + base::kProcessAccessWaitForTermination, + process_handle.Receive())) { + LOG(ERROR) << "Failed to get process handle"; debug_context_->AllowAndSendStartMessage(); + } else { + if (!NaClBrokerService::GetInstance()->LaunchDebugExceptionHandler( + weak_factory_.GetWeakPtr(), peer_pid, process_handle)) { + debug_context_->AllowAndSendStartMessage(); + } } } else { // Start new thread for debug loop diff --git a/chrome/common/nacl_debug_exception_handler_win.cc b/chrome/common/nacl_debug_exception_handler_win.cc index 5a37955..4bb0e4f 100644 --- a/chrome/common/nacl_debug_exception_handler_win.cc +++ b/chrome/common/nacl_debug_exception_handler_win.cc @@ -6,16 +6,19 @@ #include "base/process_util.h" #include "base/threading/platform_thread.h" +#include "base/win/scoped_handle.h" #include "native_client/src/trusted/service_runtime/win/debug_exception_handler.h" namespace { class DebugExceptionHandler : public base::PlatformThread::Delegate { public: - DebugExceptionHandler(int32 pid, + DebugExceptionHandler(base::ProcessHandle nacl_process, base::MessageLoopProxy* message_loop, const base::Closure& on_connected) - : pid_(pid), message_loop_(message_loop), on_connected_(on_connected) { + : nacl_process_(nacl_process), + message_loop_(message_loop), + on_connected_(on_connected) { } virtual void ThreadMain() OVERRIDE { @@ -24,20 +27,11 @@ class DebugExceptionHandler : public base::PlatformThread::Delegate { // DebugActiveProcess()) on the same thread on which // NaClDebugLoop() receives debug events for the process. BOOL attached = false; - base::ProcessHandle process_handle = base::kNullProcessHandle; - if (!base::OpenProcessHandleWithAccess( - pid_, - base::kProcessAccessQueryInformation | - base::kProcessAccessSuspendResume | - base::kProcessAccessTerminate | - base::kProcessAccessVMOperation | - base::kProcessAccessVMRead | - base::kProcessAccessVMWrite | - base::kProcessAccessWaitForTermination, - &process_handle)) { - LOG(ERROR) << "Failed to get process handle"; + int pid = GetProcessId(nacl_process_); + if (pid == 0) { + LOG(ERROR) << "Invalid process handle"; } else { - attached = DebugActiveProcess(pid_); + attached = DebugActiveProcess(pid); if (!attached) { LOG(ERROR) << "Failed to connect to the process"; } @@ -50,16 +44,13 @@ class DebugExceptionHandler : public base::PlatformThread::Delegate { if (attached) { DWORD exit_code; - NaClDebugLoop(process_handle, &exit_code); - } - if (process_handle != base::kNullProcessHandle) { - base::CloseProcessHandle(process_handle); + NaClDebugLoop(nacl_process_, &exit_code); } delete this; } private: - int32 pid_; + base::win::ScopedHandle nacl_process_; base::MessageLoopProxy* message_loop_; base::Closure on_connected_; @@ -68,13 +59,13 @@ class DebugExceptionHandler : public base::PlatformThread::Delegate { } // namespace -void NaClStartDebugExceptionHandlerThread(int32 nacl_process_id, +void NaClStartDebugExceptionHandlerThread(base::ProcessHandle nacl_process, base::MessageLoopProxy* message_loop, const base::Closure& on_connected) { // The new PlatformThread will take ownership of the // DebugExceptionHandler object, which will delete itself on exit. DebugExceptionHandler* handler = new DebugExceptionHandler( - nacl_process_id, message_loop, on_connected); + nacl_process, message_loop, on_connected); if (!base::PlatformThread::CreateNonJoinable(0, handler)) { on_connected.Run(); delete handler; diff --git a/chrome/common/nacl_debug_exception_handler_win.h b/chrome/common/nacl_debug_exception_handler_win.h index 4965f93..99f769e 100644 --- a/chrome/common/nacl_debug_exception_handler_win.h +++ b/chrome/common/nacl_debug_exception_handler_win.h @@ -8,8 +8,9 @@ #include "base/callback.h" #include "base/message_loop.h" +#include "base/process.h" -void NaClStartDebugExceptionHandlerThread(int32 nacl_process_id, +void NaClStartDebugExceptionHandlerThread(base::ProcessHandle nacl_process, base::MessageLoopProxy* message_loop, const base::Closure& on_connected); diff --git a/chrome/common/nacl_messages.h b/chrome/common/nacl_messages.h index 67dab31..b244509 100644 --- a/chrome/common/nacl_messages.h +++ b/chrome/common/nacl_messages.h @@ -36,8 +36,9 @@ IPC_MESSAGE_CONTROL2(NaClProcessMsg_LoaderLaunched, // Tells the NaCl broker to attach a debug exception handler to the // given NaCl loader process. -IPC_MESSAGE_CONTROL1(NaClProcessMsg_LaunchDebugExceptionHandler, - int32 /* pid */) +IPC_MESSAGE_CONTROL2(NaClProcessMsg_LaunchDebugExceptionHandler, + int32 /* pid of the NaCl process */, + base::ProcessHandle /* handle of the NaCl process */) // Notify the browser process that the broker process finished // attaching a debug exception handler to the given NaCl loader diff --git a/chrome/nacl/nacl_broker_listener.cc b/chrome/nacl/nacl_broker_listener.cc index 086f2a6..4782e1e 100644 --- a/chrome/nacl/nacl_broker_listener.cc +++ b/chrome/nacl/nacl_broker_listener.cc @@ -98,9 +98,11 @@ void NaClBrokerListener::OnLaunchLoaderThroughBroker( loader_handle_in_browser)); } -void NaClBrokerListener::OnLaunchDebugExceptionHandler(int32 pid) { +void NaClBrokerListener::OnLaunchDebugExceptionHandler( + int32 pid, base::ProcessHandle process_handle) { base::Closure reply_sender(base::Bind(SendReply, channel_.get(), pid)); - NaClStartDebugExceptionHandlerThread(pid, base::MessageLoopProxy::current(), + NaClStartDebugExceptionHandlerThread(process_handle, + base::MessageLoopProxy::current(), reply_sender); } diff --git a/chrome/nacl/nacl_broker_listener.h b/chrome/nacl/nacl_broker_listener.h index f30e548..94f97c9 100644 --- a/chrome/nacl/nacl_broker_listener.h +++ b/chrome/nacl/nacl_broker_listener.h @@ -27,7 +27,8 @@ class NaClBrokerListener : public IPC::Channel::Listener { private: void OnLaunchLoaderThroughBroker(const std::string& loader_channel_id); - void OnLaunchDebugExceptionHandler(int32 pid); + void OnLaunchDebugExceptionHandler(int32 pid, + base::ProcessHandle process_handle); void OnStopBroker(); base::ProcessHandle browser_handle_; diff --git a/ipc/ipc_message_utils.h b/ipc/ipc_message_utils.h index 8772b67..9e259f1 100644 --- a/ipc/ipc_message_utils.h +++ b/ipc/ipc_message_utils.h @@ -706,15 +706,18 @@ struct ParamTraits<string16> { template <> struct ParamTraits<HANDLE> { typedef HANDLE param_type; + // Note that HWNDs/HANDLE/HCURSOR/HACCEL etc are always 32 bits, even on 64 + // bit systems. static void Write(Message* m, const param_type& p) { - // Note that HWNDs/HANDLE/HCURSOR/HACCEL etc are always 32 bits, even on 64 - // bit systems. m->WriteUInt32(reinterpret_cast<uint32>(p)); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - DCHECK_EQ(sizeof(param_type), sizeof(uint32)); - return m->ReadUInt32(iter, reinterpret_cast<uint32*>(r)); + uint32 temp; + if (!m->ReadUInt32(iter, &temp)) + return false; + *r = reinterpret_cast<HANDLE>(temp); + return true; } static void Log(const param_type& p, std::string* l) { l->append(StringPrintf("0x%X", p)); |