summaryrefslogtreecommitdiff
path: root/deps
diff options
context:
space:
mode:
authorPeter Marshall <petermarshall@chromium.org>2018-11-09 13:06:07 +0100
committerDaniel Bevenius <daniel.bevenius@gmail.com>2018-11-12 07:24:09 +0100
commita8847aa5e009e4c487a0d895cfeaea0080e33a29 (patch)
treeec3e576dd8da4934533e7fdc190d7a65b1af8d70 /deps
parentd6f52f5a38b2e93f62da2c18a58bc85991f11234 (diff)
downloadnode-new-a8847aa5e009e4c487a0d895cfeaea0080e33a29.tar.gz
deps: cherry-pick b87d408 from upstream V8
Original commit message: [heap-profiler] Fix a use-after-free when snapshots are deleted If a caller starts the sampling heap profiler and takes a snapshot, and then deletes the snapshot before the sampling has completed, a use-after-free will occur on the StringsStorage pointer. The same issue applies for StartTrackingHeapObjects which shares the same StringsStorage object. Bug: v8:8373 Change-Id: I5d69d60d3f9465f9dd3b3bef107c204e0fda0643 Reviewed-on: https://chromium-review.googlesource.com/c/1301477 Commit-Queue: Peter Marshall <petermarshall@chromium.org> Reviewed-by: Alexei Filippov <alph@chromium.org> Cr-Commit-Position: refs/heads/master@{#57114} PR-URL: https://github.com/nodejs/node/pull/24272 Refs: https://github.com/v8/v8/commit/b87d408f65b9ab49a4d199e850d2358995deaeb2 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Diffstat (limited to 'deps')
-rw-r--r--deps/v8/src/profiler/heap-profiler.cc9
-rw-r--r--deps/v8/src/profiler/heap-profiler.h2
-rw-r--r--deps/v8/test/cctest/test-heap-profiler.cc42
3 files changed, 52 insertions, 1 deletions
diff --git a/deps/v8/src/profiler/heap-profiler.cc b/deps/v8/src/profiler/heap-profiler.cc
index 71e297c4bf..3a1df29bd4 100644
--- a/deps/v8/src/profiler/heap-profiler.cc
+++ b/deps/v8/src/profiler/heap-profiler.cc
@@ -23,9 +23,14 @@ HeapProfiler::~HeapProfiler() = default;
void HeapProfiler::DeleteAllSnapshots() {
snapshots_.clear();
- names_.reset(new StringsStorage());
+ MaybeClearStringsStorage();
}
+void HeapProfiler::MaybeClearStringsStorage() {
+ if (snapshots_.empty() && !sampling_heap_profiler_ && !allocation_tracker_) {
+ names_.reset(new StringsStorage());
+ }
+}
void HeapProfiler::RemoveSnapshot(HeapSnapshot* snapshot) {
snapshots_.erase(
@@ -126,6 +131,7 @@ bool HeapProfiler::StartSamplingHeapProfiler(
void HeapProfiler::StopSamplingHeapProfiler() {
sampling_heap_profiler_.reset();
+ MaybeClearStringsStorage();
}
@@ -159,6 +165,7 @@ void HeapProfiler::StopHeapObjectsTracking() {
ids_->StopHeapObjectsTracking();
if (allocation_tracker_) {
allocation_tracker_.reset();
+ MaybeClearStringsStorage();
heap()->RemoveHeapObjectAllocationTracker(this);
}
}
diff --git a/deps/v8/src/profiler/heap-profiler.h b/deps/v8/src/profiler/heap-profiler.h
index 8ce379d59d..099c0e24fa 100644
--- a/deps/v8/src/profiler/heap-profiler.h
+++ b/deps/v8/src/profiler/heap-profiler.h
@@ -92,6 +92,8 @@ class HeapProfiler : public HeapObjectAllocationTracker {
v8::PersistentValueVector<v8::Object>* objects);
private:
+ void MaybeClearStringsStorage();
+
Heap* heap() const;
// Mapping from HeapObject addresses to objects' uids.
diff --git a/deps/v8/test/cctest/test-heap-profiler.cc b/deps/v8/test/cctest/test-heap-profiler.cc
index 5d8094d635..e4e5f4c8dc 100644
--- a/deps/v8/test/cctest/test-heap-profiler.cc
+++ b/deps/v8/test/cctest/test-heap-profiler.cc
@@ -3900,3 +3900,45 @@ TEST(WeakReference) {
const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
CHECK(ValidateSnapshot(snapshot));
}
+
+TEST(Bug8373_1) {
+ LocalContext env;
+ v8::HandleScope scope(env->GetIsolate());
+ v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
+
+ heap_profiler->StartSamplingHeapProfiler(100);
+
+ heap_profiler->TakeHeapSnapshot();
+ // Causes the StringsStorage to be deleted.
+ heap_profiler->DeleteAllHeapSnapshots();
+
+ // Triggers an allocation sample that tries to use the StringsStorage.
+ for (int i = 0; i < 2 * 1024; ++i) {
+ CompileRun(
+ "new Array(64);"
+ "new Uint8Array(16);");
+ }
+
+ heap_profiler->StopSamplingHeapProfiler();
+}
+
+TEST(Bug8373_2) {
+ LocalContext env;
+ v8::HandleScope scope(env->GetIsolate());
+ v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
+
+ heap_profiler->StartTrackingHeapObjects(true);
+
+ heap_profiler->TakeHeapSnapshot();
+ // Causes the StringsStorage to be deleted.
+ heap_profiler->DeleteAllHeapSnapshots();
+
+ // Triggers an allocations that try to use the StringsStorage.
+ for (int i = 0; i < 2 * 1024; ++i) {
+ CompileRun(
+ "new Array(64);"
+ "new Uint8Array(16);");
+ }
+
+ heap_profiler->StopTrackingHeapObjects();
+}