Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 733cb1e

Browse filesBrowse files
psmarshallcodebytere
psmarshall
authored and
codebytere
committed
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 51643c2 commit 733cb1e
Copy full SHA for 733cb1e

File tree

Expand file treeCollapse file tree

4 files changed

+53
-2
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+53
-2
lines changed
Open diff view settings
Collapse file

‎common.gypi‎

Copy file name to clipboardExpand all lines: common.gypi
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333

3434
# Reset this number to 0 on major V8 upgrades.
3535
# Increment by one for each non-official patch applied to deps/v8.
36-
'v8_embedder_string': '-node.46',
36+
'v8_embedder_string': '-node.12',
3737

3838
# Enable disassembler for `--print-code` v8 options
3939
'v8_enable_disassembler': 1,
Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/src/profiler/heap-profiler.cc
+8-1Lines changed: 8 additions & 1 deletion
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
}
Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/src/profiler/heap-profiler.h
+2Lines changed: 2 additions & 0 deletions
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.
Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/test/cctest/test-heap-profiler.cc
+42Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3690,3 +3690,45 @@ TEST(WeakReference) {
36903690
const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
36913691
CHECK(ValidateSnapshot(snapshot));
36923692
}
3693+
3694+
TEST(Bug8373_1) {
3695+
LocalContext env;
3696+
v8::HandleScope scope(env->GetIsolate());
3697+
v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
3698+
3699+
heap_profiler->StartSamplingHeapProfiler(100);
3700+
3701+
heap_profiler->TakeHeapSnapshot();
3702+
// Causes the StringsStorage to be deleted.
3703+
heap_profiler->DeleteAllHeapSnapshots();
3704+
3705+
// Triggers an allocation sample that tries to use the StringsStorage.
3706+
for (int i = 0; i < 2 * 1024; ++i) {
3707+
CompileRun(
3708+
"new Array(64);"
3709+
"new Uint8Array(16);");
3710+
}
3711+
3712+
heap_profiler->StopSamplingHeapProfiler();
3713+
}
3714+
3715+
TEST(Bug8373_2) {
3716+
LocalContext env;
3717+
v8::HandleScope scope(env->GetIsolate());
3718+
v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
3719+
3720+
heap_profiler->StartTrackingHeapObjects(true);
3721+
3722+
heap_profiler->TakeHeapSnapshot();
3723+
// Causes the StringsStorage to be deleted.
3724+
heap_profiler->DeleteAllHeapSnapshots();
3725+
3726+
// Triggers an allocations that try to use the StringsStorage.
3727+
for (int i = 0; i < 2 * 1024; ++i) {
3728+
CompileRun(
3729+
"new Array(64);"
3730+
"new Uint8Array(16);");
3731+
}
3732+
3733+
heap_profiler->StopTrackingHeapObjects();
3734+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.