Skip to content

Commit a39493f

Browse files
psmarshallBridgeAR
authored andcommitted
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: #24272 Refs: v8/v8@b87d408 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
1 parent fe7ef1a commit a39493f

File tree

4 files changed

+53
-2
lines changed

4 files changed

+53
-2
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030

3131
# Reset this number to 0 on major V8 upgrades.
3232
# Increment by one for each non-official patch applied to deps/v8.
33-
'v8_embedder_string': '-node.10',
33+
'v8_embedder_string': '-node.11',
3434

3535
# Enable disassembler for `--print-code` v8 options
3636
'v8_enable_disassembler': 1,

deps/v8/src/profiler/heap-profiler.cc

+8-1
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,14 @@ HeapProfiler::~HeapProfiler() = default;
2323

2424
void HeapProfiler::DeleteAllSnapshots() {
2525
snapshots_.clear();
26-
names_.reset(new StringsStorage());
26+
MaybeClearStringsStorage();
2727
}
2828

29+
void HeapProfiler::MaybeClearStringsStorage() {
30+
if (snapshots_.empty() && !sampling_heap_profiler_ && !allocation_tracker_) {
31+
names_.reset(new StringsStorage());
32+
}
33+
}
2934

3035
void HeapProfiler::RemoveSnapshot(HeapSnapshot* snapshot) {
3136
snapshots_.erase(
@@ -126,6 +131,7 @@ bool HeapProfiler::StartSamplingHeapProfiler(
126131

127132
void HeapProfiler::StopSamplingHeapProfiler() {
128133
sampling_heap_profiler_.reset();
134+
MaybeClearStringsStorage();
129135
}
130136

131137

@@ -159,6 +165,7 @@ void HeapProfiler::StopHeapObjectsTracking() {
159165
ids_->StopHeapObjectsTracking();
160166
if (allocation_tracker_) {
161167
allocation_tracker_.reset();
168+
MaybeClearStringsStorage();
162169
heap()->RemoveHeapObjectAllocationTracker(this);
163170
}
164171
}

deps/v8/src/profiler/heap-profiler.h

+2
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ class HeapProfiler : public HeapObjectAllocationTracker {
9292
v8::PersistentValueVector<v8::Object>* objects);
9393

9494
private:
95+
void MaybeClearStringsStorage();
96+
9597
Heap* heap() const;
9698

9799
// Mapping from HeapObject addresses to objects' uids.

deps/v8/test/cctest/test-heap-profiler.cc

+42
Original file line numberDiff line numberDiff line change
@@ -3900,3 +3900,45 @@ TEST(WeakReference) {
39003900
const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
39013901
CHECK(ValidateSnapshot(snapshot));
39023902
}
3903+
3904+
TEST(Bug8373_1) {
3905+
LocalContext env;
3906+
v8::HandleScope scope(env->GetIsolate());
3907+
v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
3908+
3909+
heap_profiler->StartSamplingHeapProfiler(100);
3910+
3911+
heap_profiler->TakeHeapSnapshot();
3912+
// Causes the StringsStorage to be deleted.
3913+
heap_profiler->DeleteAllHeapSnapshots();
3914+
3915+
// Triggers an allocation sample that tries to use the StringsStorage.
3916+
for (int i = 0; i < 2 * 1024; ++i) {
3917+
CompileRun(
3918+
"new Array(64);"
3919+
"new Uint8Array(16);");
3920+
}
3921+
3922+
heap_profiler->StopSamplingHeapProfiler();
3923+
}
3924+
3925+
TEST(Bug8373_2) {
3926+
LocalContext env;
3927+
v8::HandleScope scope(env->GetIsolate());
3928+
v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
3929+
3930+
heap_profiler->StartTrackingHeapObjects(true);
3931+
3932+
heap_profiler->TakeHeapSnapshot();
3933+
// Causes the StringsStorage to be deleted.
3934+
heap_profiler->DeleteAllHeapSnapshots();
3935+
3936+
// Triggers an allocations that try to use the StringsStorage.
3937+
for (int i = 0; i < 2 * 1024; ++i) {
3938+
CompileRun(
3939+
"new Array(64);"
3940+
"new Uint8Array(16);");
3941+
}
3942+
3943+
heap_profiler->StopTrackingHeapObjects();
3944+
}

0 commit comments

Comments
 (0)