Skip to content

Commit 1bf155c

Browse files
GeorgNeismibrunin
authored andcommitted
[Backport] CVE-2021-21195: Use after free in V8
Partial cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/2780300: Merged: [deoptimizer] Fix bug in OptimizedFrame::Summarize Revision: 3353a7d0b017146d543434be4036a81aaf7d25ae BUG=chromium:1182647 NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true [email protected] Change-Id: I86abd6a3f34169be5f99aa9f54bb7bb3706fa85a Reviewed-by: Georg Neis <[email protected]> Reviewed-by: Benedikt Meurer <[email protected]> Commit-Queue: Georg Neis <[email protected]> Cr-Commit-Position: refs/branch-heads/8.9@{#49} Cr-Branched-From: 16b9bbbd581c25391981aa03180b76aa60463a3e-refs/heads/8.9.255@{#1} Cr-Branched-From: d16a2a688498bd1c3e6a49edb25d8c4ca56232dc-refs/heads/master@{#72039} Reviewed-by: Jüri Valdmann <[email protected]>
1 parent 8d49f9a commit 1bf155c

File tree

2 files changed

+67
-10
lines changed

2 files changed

+67
-10
lines changed

chromium/v8/src/deoptimizer/deoptimizer.cc

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3550,7 +3550,8 @@ Address TranslatedState::DecompressIfNeeded(intptr_t value) {
35503550
}
35513551
}
35523552

3553-
TranslatedState::TranslatedState(const JavaScriptFrame* frame) {
3553+
TranslatedState::TranslatedState(const JavaScriptFrame* frame)
3554+
: purpose_(kFrameInspection) {
35543555
int deopt_index = Safepoint::kNoDeoptimizationIndex;
35553556
DeoptimizationData data =
35563557
static_cast<const OptimizedFrame*>(frame)->GetDeoptimizationData(
@@ -3947,25 +3948,63 @@ void TranslatedState::EnsureCapturedObjectAllocatedAt(
39473948
}
39483949

39493950
default:
3950-
CHECK(map->IsJSObjectMap());
39513951
EnsureJSObjectAllocated(slot, map);
3952-
TranslatedValue* properties_slot = &(frame->values_[value_index]);
3953-
value_index++;
3952+
int remaining_children_count = slot->GetChildrenCount() - 1;
3953+
3954+
TranslatedValue* properties_slot = frame->ValueAt(value_index);
3955+
value_index++, remaining_children_count--;
39543956
if (properties_slot->kind() == TranslatedValue::kCapturedObject) {
3955-
// If we are materializing the property array, make sure we put
3956-
// the mutable heap numbers at the right places.
3957+
// We are materializing the property array, so make sure we put the
3958+
// mutable heap numbers at the right places.
39573959
EnsurePropertiesAllocatedAndMarked(properties_slot, map);
39583960
EnsureChildrenAllocated(properties_slot->GetChildrenCount(), frame,
39593961
&value_index, worklist);
3962+
} else {
3963+
CHECK_EQ(properties_slot->kind(), TranslatedValue::kTagged);
39603964
}
3961-
// Make sure all the remaining children (after the map and properties) are
3962-
// allocated.
3963-
return EnsureChildrenAllocated(slot->GetChildrenCount() - 2, frame,
3965+
3966+
TranslatedValue* elements_slot = frame->ValueAt(value_index);
3967+
value_index++, remaining_children_count--;
3968+
if (elements_slot->kind() == TranslatedValue::kCapturedObject ||
3969+
!map->IsJSArrayMap()) {
3970+
// Handle this case with the other remaining children below.
3971+
value_index--, remaining_children_count++;
3972+
} else {
3973+
CHECK_EQ(elements_slot->kind(), TranslatedValue::kTagged);
3974+
elements_slot->GetValue();
3975+
if (purpose_ == kFrameInspection) {
3976+
// We are materializing a JSArray for the purpose of frame inspection.
3977+
// If we were to construct it with the above elements value then an
3978+
// actual deopt later on might create another JSArray instance with
3979+
// the same elements store. That would violate the key assumption
3980+
// behind left-trimming.
3981+
elements_slot->ReplaceElementsArrayWithCopy();
3982+
}
3983+
}
3984+
3985+
// Make sure all the remaining children (after the map, properties store,
3986+
// and possibly elements store) are allocated.
3987+
return EnsureChildrenAllocated(remaining_children_count, frame,
39643988
&value_index, worklist);
39653989
}
39663990
UNREACHABLE();
39673991
}
39683992

3993+
void TranslatedValue::ReplaceElementsArrayWithCopy() {
3994+
DCHECK_EQ(kind(), TranslatedValue::kTagged);
3995+
DCHECK_EQ(materialization_state(), TranslatedValue::kFinished);
3996+
auto elements = Handle<FixedArrayBase>::cast(GetValue());
3997+
DCHECK(elements->IsFixedArray() || elements->IsFixedDoubleArray());
3998+
if (elements->IsFixedDoubleArray()) {
3999+
DCHECK(!elements->IsCowArray());
4000+
set_storage(isolate()->factory()->CopyFixedDoubleArray(
4001+
Handle<FixedDoubleArray>::cast(elements)));
4002+
} else if (!elements->IsCowArray()) {
4003+
set_storage(isolate()->factory()->CopyFixedArray(
4004+
Handle<FixedArray>::cast(elements)));
4005+
}
4006+
}
4007+
39694008
void TranslatedState::EnsureChildrenAllocated(int count, TranslatedFrame* frame,
39704009
int* value_index,
39714010
std::stack<int>* worklist) {
@@ -4030,6 +4069,7 @@ Handle<ByteArray> TranslatedState::AllocateStorageFor(TranslatedValue* slot) {
40304069

40314070
void TranslatedState::EnsureJSObjectAllocated(TranslatedValue* slot,
40324071
Handle<Map> map) {
4072+
CHECK(map->IsJSObjectMap());
40334073
CHECK_EQ(map->instance_size(), slot->GetChildrenCount() * kTaggedSize);
40344074

40354075
Handle<ByteArray> object_storage = AllocateStorageFor(slot);

chromium/v8/src/deoptimizer/deoptimizer.h

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ class TranslatedValue {
121121
return storage_;
122122
}
123123

124+
void ReplaceElementsArrayWithCopy();
125+
124126
Kind kind_;
125127
MaterializationState materialization_state_ = kUninitialized;
126128
TranslatedState* container_; // This is only needed for materialization of
@@ -317,7 +319,15 @@ class TranslatedFrame {
317319

318320
class TranslatedState {
319321
public:
320-
TranslatedState() = default;
322+
// There are two constructors, each for a different purpose:
323+
324+
// The default constructor is for the purpose of deoptimizing an optimized
325+
// frame (replacing it with one or several unoptimized frames). It is used by
326+
// the Deoptimizer.
327+
TranslatedState() : purpose_(kDeoptimization) {}
328+
329+
// This constructor is for the purpose of merely inspecting an optimized
330+
// frame. It is used by stack trace generation and various debugging features.
321331
explicit TranslatedState(const JavaScriptFrame* frame);
322332

323333
void Prepare(Address stack_frame_pointer);
@@ -352,6 +362,12 @@ class TranslatedState {
352362
private:
353363
friend TranslatedValue;
354364

365+
// See the description of the constructors for an explanation of the two
366+
// purposes. The only actual difference is that in the kFrameInspection case
367+
// extra work is needed to not violate assumptions made by left-trimming. For
368+
// details, see the code around ReplaceElementsArrayWithCopy.
369+
enum Purpose { kDeoptimization, kFrameInspection };
370+
355371
TranslatedFrame CreateNextTranslatedFrame(TranslationIterator* iterator,
356372
FixedArray literal_array,
357373
Address fp, FILE* trace_file);
@@ -408,6 +424,7 @@ class TranslatedState {
408424
static Float32 GetFloatSlot(Address fp, int slot_index);
409425
static Float64 GetDoubleSlot(Address fp, int slot_index);
410426

427+
Purpose const purpose_;
411428
std::vector<TranslatedFrame> frames_;
412429
Isolate* isolate_ = nullptr;
413430
Address stack_frame_pointer_ = kNullAddress;

0 commit comments

Comments
 (0)