From 80b10b4fe24874d9155980b608b3b49fd5a43ea8 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 20 Jul 2016 14:01:58 +0200 Subject: [PATCH] src: fix use-after-free in inspector agent uv_close() is an asynchronous operation. Calling it on a data member inside the destructor is unsound because its memory is about to be reclaimed but libuv is not done with it yet. PR-URL: https://github.com/nodejs/node/pull/7907 Reviewed-By: Ali Ijaz Sheikh Reviewed-By: Eugene Ostroukhov Reviewed-By: James M Snell --- src/inspector_agent.cc | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 0446f462e92f7e..55ee1810752494 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -209,7 +209,7 @@ class AgentImpl { State state_; node::Environment* parent_env_; - uv_async_t data_written_; + uv_async_t* data_written_; uv_async_t io_thread_req_; inspector_socket_t* client_socket_; blink::V8Inspector* inspector_; @@ -317,6 +317,7 @@ AgentImpl::AgentImpl(Environment* env) : port_(0), shutting_down_(false), state_(State::kNew), parent_env_(env), + data_written_(new uv_async_t()), client_socket_(nullptr), inspector_(nullptr), platform_(nullptr), @@ -324,24 +325,26 @@ AgentImpl::AgentImpl(Environment* env) : port_(0), frontend_session_id_(0), backend_session_id_(0) { CHECK_EQ(0, uv_sem_init(&start_sem_, 0)); - memset(&data_written_, 0, sizeof(data_written_)); memset(&io_thread_req_, 0, sizeof(io_thread_req_)); + CHECK_EQ(0, uv_async_init(env->event_loop(), data_written_, nullptr)); + uv_unref(reinterpret_cast(data_written_)); } AgentImpl::~AgentImpl() { - if (!inspector_) - return; - uv_close(reinterpret_cast(&data_written_), nullptr); + auto close_cb = [](uv_handle_t* handle) { + delete reinterpret_cast(handle); + }; + uv_close(reinterpret_cast(data_written_), close_cb); + data_written_ = nullptr; } bool AgentImpl::Start(v8::Platform* platform, int port, bool wait) { auto env = parent_env_; inspector_ = new V8NodeInspector(this, env, platform); platform_ = platform; - int err = uv_async_init(env->event_loop(), &data_written_, nullptr); - CHECK_EQ(err, 0); - uv_unref(reinterpret_cast(&data_written_)); + int err = uv_loop_init(&child_loop_); + CHECK_EQ(err, 0); port_ = port; wait_ = wait; @@ -517,7 +520,7 @@ void AgentImpl::PostIncomingMessage(const String16& message) { platform_->CallOnForegroundThread(isolate, new DispatchOnInspectorBackendTask(this)); isolate->RequestInterrupt(InterruptCallback, this); - uv_async_send(&data_written_); + uv_async_send(data_written_); } void AgentImpl::OnInspectorConnectionIO(inspector_socket_t* socket) { @@ -559,7 +562,7 @@ void AgentImpl::DispatchMessages() { inspector_->dispatchMessageFromFrontend(message); } } - uv_async_send(&data_written_); + uv_async_send(data_written_); dispatching_messages_ = false; }