Skip to content

Commit 780395f

Browse files
bnoordhuiscjihrig
authored andcommitted
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: #7907 Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 9d45569 commit 780395f

File tree

1 file changed

+13
-10
lines changed

1 file changed

+13
-10
lines changed

src/inspector_agent.cc

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ class AgentImpl {
208208
State state_;
209209
node::Environment* parent_env_;
210210

211-
uv_async_t data_written_;
211+
uv_async_t* data_written_;
212212
uv_async_t io_thread_req_;
213213
inspector_socket_t* client_socket_;
214214
blink::V8Inspector* inspector_;
@@ -316,31 +316,34 @@ AgentImpl::AgentImpl(Environment* env) : port_(0),
316316
shutting_down_(false),
317317
state_(State::kNew),
318318
parent_env_(env),
319+
data_written_(new uv_async_t()),
319320
client_socket_(nullptr),
320321
inspector_(nullptr),
321322
platform_(nullptr),
322323
dispatching_messages_(false),
323324
frontend_session_id_(0),
324325
backend_session_id_(0) {
325326
CHECK_EQ(0, uv_sem_init(&start_sem_, 0));
326-
memset(&data_written_, 0, sizeof(data_written_));
327327
memset(&io_thread_req_, 0, sizeof(io_thread_req_));
328+
CHECK_EQ(0, uv_async_init(env->event_loop(), data_written_, nullptr));
329+
uv_unref(reinterpret_cast<uv_handle_t*>(data_written_));
328330
}
329331

330332
AgentImpl::~AgentImpl() {
331-
if (!inspector_)
332-
return;
333-
uv_close(reinterpret_cast<uv_handle_t*>(&data_written_), nullptr);
333+
auto close_cb = [](uv_handle_t* handle) {
334+
delete reinterpret_cast<uv_async_t*>(handle);
335+
};
336+
uv_close(reinterpret_cast<uv_handle_t*>(data_written_), close_cb);
337+
data_written_ = nullptr;
334338
}
335339

336340
bool AgentImpl::Start(v8::Platform* platform, int port, bool wait) {
337341
auto env = parent_env_;
338342
inspector_ = new V8NodeInspector(this, env, platform);
339343
platform_ = platform;
340-
int err = uv_async_init(env->event_loop(), &data_written_, nullptr);
341-
CHECK_EQ(err, 0);
342344

343-
uv_unref(reinterpret_cast<uv_handle_t*>(&data_written_));
345+
int err = uv_loop_init(&child_loop_);
346+
CHECK_EQ(err, 0);
344347

345348
port_ = port;
346349
wait_ = wait;
@@ -516,7 +519,7 @@ void AgentImpl::PostIncomingMessage(const String16& message) {
516519
platform_->CallOnForegroundThread(isolate,
517520
new DispatchOnInspectorBackendTask(this));
518521
isolate->RequestInterrupt(InterruptCallback, this);
519-
uv_async_send(&data_written_);
522+
uv_async_send(data_written_);
520523
}
521524

522525
void AgentImpl::OnInspectorConnectionIO(inspector_socket_t* socket) {
@@ -558,7 +561,7 @@ void AgentImpl::DispatchMessages() {
558561
inspector_->dispatchMessageFromFrontend(message);
559562
}
560563
}
561-
uv_async_send(&data_written_);
564+
uv_async_send(data_written_);
562565
dispatching_messages_ = false;
563566
}
564567

0 commit comments

Comments
 (0)