From b9644337aa22b8b887bb5a3879c4759ec225a804 Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor@indutny.com>
Date: Wed, 31 Mar 2021 13:29:07 -0700
Subject: [PATCH] env: fix use-after-free in keep-alive allocators

As per comment in the change it is possible that
`Environment:AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose`
is called after:

1. `Node::Stop(env)`
2. `Environment::~Environment()`

In this case platform's `per_isolate_` map won't have an entry for our
isolate and the `AddIsolateFinishedCallback` will invoke the callback
immediately leaving us with `keep_alive_allocators_` set to freed pointer.

This commit also changes the assertion in the
`AddIsolateFinishedCallback` that is triggered incorrectly.
---
 src/env.cc           | 12 ++++++++++++
 src/node_platform.cc |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/env.cc b/src/env.cc
index 295b68741dc5c6..a807d75179af5b 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -1153,6 +1153,18 @@ char* Environment::Reallocate(char* data, size_t old_size, size_t size) {
 
 void Environment::AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(
     std::shared_ptr<v8::ArrayBuffer::Allocator> allocator) {
+  // It is possible that we can arrive here after:
+  // 1. Node::Stop(env)
+  // 2. Environment::~Environment
+  //
+  // In this case platform's `per_isolate_` map won't have an entry for our
+  // isolate and the `AddIsolateFinishedCallback` will invoke the callback
+  // immediately leaving us with `keep_alive_allocators_` set to freed pointer.
+  if (is_stopping() && started_cleanup_) {
+    allocator.reset();
+    return;
+  }
+
   if (keep_alive_allocators_ == nullptr) {
     MultiIsolatePlatform* platform = isolate_data()->platform();
     CHECK_NOT_NULL(platform);
diff --git a/src/node_platform.cc b/src/node_platform.cc
index d7b394aae81ed9..8703372818770b 100644
--- a/src/node_platform.cc
+++ b/src/node_platform.cc
@@ -359,10 +359,10 @@ void NodePlatform::AddIsolateFinishedCallback(Isolate* isolate,
   Mutex::ScopedLock lock(per_isolate_mutex_);
   auto it = per_isolate_.find(isolate);
   if (it == per_isolate_.end()) {
-    CHECK(it->second);
     cb(data);
     return;
   }
+  CHECK(it->second);
   it->second->AddShutdownCallback(cb, data);
 }