Skip to content

Commit 902359b

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 a775361 commit 902359b

File tree

2 files changed

+67
-10
lines changed

2 files changed

+67
-10
lines changed

chromium/v8/src/deoptimizer.cc

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3222,7 +3222,8 @@ int TranslatedState::CreateNextTranslatedValue(
32223222
FATAL("We should never get here - unexpected deopt info.");
32233223
}
32243224

3225-
TranslatedState::TranslatedState(const JavaScriptFrame* frame) {
3225+
TranslatedState::TranslatedState(const JavaScriptFrame* frame)
3226+
: purpose_(kFrameInspection) {
32263227
int deopt_index = Safepoint::kNoDeoptimizationIndex;
32273228
DeoptimizationData* data =
32283229
static_cast<const OptimizedFrame*>(frame)->GetDeoptimizationData(
@@ -3596,25 +3597,63 @@ void TranslatedState::EnsureCapturedObjectAllocatedAt(
35963597
}
35973598

35983599
default:
3599-
CHECK(map->IsJSObjectMap());
36003600
EnsureJSObjectAllocated(slot, map);
3601-
TranslatedValue* properties_slot = &(frame->values_[value_index]);
3602-
value_index++;
3601+
int remaining_children_count = slot->GetChildrenCount() - 1;
3602+
3603+
TranslatedValue* properties_slot = frame->ValueAt(value_index);
3604+
value_index++, remaining_children_count--;
36033605
if (properties_slot->kind() == TranslatedValue::kCapturedObject) {
3604-
// If we are materializing the property array, make sure we put
3605-
// the mutable heap numbers at the right places.
3606+
// We are materializing the property array, so make sure we put the
3607+
// mutable heap numbers at the right places.
36063608
EnsurePropertiesAllocatedAndMarked(properties_slot, map);
36073609
EnsureChildrenAllocated(properties_slot->GetChildrenCount(), frame,
36083610
&value_index, worklist);
3611+
} else {
3612+
CHECK_EQ(properties_slot->kind(), TranslatedValue::kTagged);
36093613
}
3610-
// Make sure all the remaining children (after the map and properties) are
3611-
// allocated.
3612-
return EnsureChildrenAllocated(slot->GetChildrenCount() - 2, frame,
3614+
3615+
TranslatedValue* elements_slot = frame->ValueAt(value_index);
3616+
value_index++, remaining_children_count--;
3617+
if (elements_slot->kind() == TranslatedValue::kCapturedObject ||
3618+
!map->IsJSArrayMap()) {
3619+
// Handle this case with the other remaining children below.
3620+
value_index--, remaining_children_count++;
3621+
} else {
3622+
CHECK_EQ(elements_slot->kind(), TranslatedValue::kTagged);
3623+
elements_slot->GetValue();
3624+
if (purpose_ == kFrameInspection) {
3625+
// We are materializing a JSArray for the purpose of frame inspection.
3626+
// If we were to construct it with the above elements value then an
3627+
// actual deopt later on might create another JSArray instance with
3628+
// the same elements store. That would violate the key assumption
3629+
// behind left-trimming.
3630+
elements_slot->ReplaceElementsArrayWithCopy();
3631+
}
3632+
}
3633+
3634+
// Make sure all the remaining children (after the map, properties store,
3635+
// and possibly elements store) are allocated.
3636+
return EnsureChildrenAllocated(remaining_children_count, frame,
36133637
&value_index, worklist);
36143638
}
36153639
UNREACHABLE();
36163640
}
36173641

3642+
void TranslatedValue::ReplaceElementsArrayWithCopy() {
3643+
DCHECK_EQ(kind(), TranslatedValue::kTagged);
3644+
DCHECK_EQ(materialization_state(), TranslatedValue::kFinished);
3645+
auto elements = Handle<FixedArrayBase>::cast(GetValue());
3646+
DCHECK(elements->IsFixedArray() || elements->IsFixedDoubleArray());
3647+
if (elements->IsFixedDoubleArray()) {
3648+
DCHECK(!elements->IsCowArray());
3649+
set_storage(isolate()->factory()->CopyFixedDoubleArray(
3650+
Handle<FixedDoubleArray>::cast(elements)));
3651+
} else if (!elements->IsCowArray()) {
3652+
set_storage(isolate()->factory()->CopyFixedArray(
3653+
Handle<FixedArray>::cast(elements)));
3654+
}
3655+
}
3656+
36183657
void TranslatedState::EnsureChildrenAllocated(int count, TranslatedFrame* frame,
36193658
int* value_index,
36203659
std::stack<int>* worklist) {
@@ -3680,6 +3719,7 @@ Handle<ByteArray> TranslatedState::AllocateStorageFor(TranslatedValue* slot) {
36803719

36813720
void TranslatedState::EnsureJSObjectAllocated(TranslatedValue* slot,
36823721
Handle<Map> map) {
3722+
CHECK(map->IsJSObjectMap());
36833723
CHECK_EQ(map->instance_size(), slot->GetChildrenCount() * kPointerSize);
36843724

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

chromium/v8/src/deoptimizer.h

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ class TranslatedValue {
108108
return storage_;
109109
}
110110

111+
void ReplaceElementsArrayWithCopy();
112+
111113
Kind kind_;
112114
MaterializationState materialization_state_ = kUninitialized;
113115
TranslatedState* container_; // This is only needed for materialization of
@@ -286,7 +288,15 @@ class TranslatedFrame {
286288

287289
class TranslatedState {
288290
public:
289-
TranslatedState() {}
291+
// There are two constructors, each for a different purpose:
292+
293+
// The default constructor is for the purpose of deoptimizing an optimized
294+
// frame (replacing it with one or several unoptimized frames). It is used by
295+
// the Deoptimizer.
296+
TranslatedState() : purpose_(kDeoptimization) {}
297+
298+
// This constructor is for the purpose of merely inspecting an optimized
299+
// frame. It is used by stack trace generation and various debugging features.
290300
explicit TranslatedState(const JavaScriptFrame* frame);
291301

292302
void Prepare(Address stack_frame_pointer);
@@ -320,6 +330,12 @@ class TranslatedState {
320330
private:
321331
friend TranslatedValue;
322332

333+
// See the description of the constructors for an explanation of the two
334+
// purposes. The only actual difference is that in the kFrameInspection case
335+
// extra work is needed to not violate assumptions made by left-trimming. For
336+
// details, see the code around ReplaceElementsArrayWithCopy.
337+
enum Purpose { kDeoptimization, kFrameInspection };
338+
323339
TranslatedFrame CreateNextTranslatedFrame(TranslationIterator* iterator,
324340
FixedArray* literal_array,
325341
Address fp,
@@ -376,6 +392,7 @@ class TranslatedState {
376392
static Float32 GetFloatSlot(Address fp, int slot_index);
377393
static Float64 GetDoubleSlot(Address fp, int slot_index);
378394

395+
Purpose const purpose_;
379396
std::vector<TranslatedFrame> frames_;
380397
Isolate* isolate_ = nullptr;
381398
Address stack_frame_pointer_ = kNullAddress;

0 commit comments

Comments
 (0)