summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGeorg Neis <neis@chromium.org>2021-03-23 17:37:21 +0100
committerMichael Brüning <michael.bruning@qt.io>2021-04-01 11:13:04 +0000
commit1bf155cf60759d4cd2c44655737e3368e086b3f4 (patch)
tree6d7b257d16c7f48e3272cbc8eb8ed406a75ec18d
parent8d49f9a2bf7da4b42e085cd97cc6d611ce81338a (diff)
[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 R=bmeurer@chromium.org Change-Id: I86abd6a3f34169be5f99aa9f54bb7bb3706fa85a Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Commit-Queue: Georg Neis <neis@chromium.org> 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 <juri.valdmann@qt.io>
-rw-r--r--chromium/v8/src/deoptimizer/deoptimizer.cc58
-rw-r--r--chromium/v8/src/deoptimizer/deoptimizer.h19
2 files changed, 67 insertions, 10 deletions
diff --git a/chromium/v8/src/deoptimizer/deoptimizer.cc b/chromium/v8/src/deoptimizer/deoptimizer.cc
index 52ee2d4f167..1024a3572a2 100644
--- a/chromium/v8/src/deoptimizer/deoptimizer.cc
+++ b/chromium/v8/src/deoptimizer/deoptimizer.cc
@@ -3550,7 +3550,8 @@ Address TranslatedState::DecompressIfNeeded(intptr_t value) {
}
}
-TranslatedState::TranslatedState(const JavaScriptFrame* frame) {
+TranslatedState::TranslatedState(const JavaScriptFrame* frame)
+ : purpose_(kFrameInspection) {
int deopt_index = Safepoint::kNoDeoptimizationIndex;
DeoptimizationData data =
static_cast<const OptimizedFrame*>(frame)->GetDeoptimizationData(
@@ -3947,25 +3948,63 @@ void TranslatedState::EnsureCapturedObjectAllocatedAt(
}
default:
- CHECK(map->IsJSObjectMap());
EnsureJSObjectAllocated(slot, map);
- TranslatedValue* properties_slot = &(frame->values_[value_index]);
- value_index++;
+ int remaining_children_count = slot->GetChildrenCount() - 1;
+
+ TranslatedValue* properties_slot = frame->ValueAt(value_index);
+ value_index++, remaining_children_count--;
if (properties_slot->kind() == TranslatedValue::kCapturedObject) {
- // If we are materializing the property array, make sure we put
- // the mutable heap numbers at the right places.
+ // We are materializing the property array, so make sure we put the
+ // mutable heap numbers at the right places.
EnsurePropertiesAllocatedAndMarked(properties_slot, map);
EnsureChildrenAllocated(properties_slot->GetChildrenCount(), frame,
&value_index, worklist);
+ } else {
+ CHECK_EQ(properties_slot->kind(), TranslatedValue::kTagged);
}
- // Make sure all the remaining children (after the map and properties) are
- // allocated.
- return EnsureChildrenAllocated(slot->GetChildrenCount() - 2, frame,
+
+ TranslatedValue* elements_slot = frame->ValueAt(value_index);
+ value_index++, remaining_children_count--;
+ if (elements_slot->kind() == TranslatedValue::kCapturedObject ||
+ !map->IsJSArrayMap()) {
+ // Handle this case with the other remaining children below.
+ value_index--, remaining_children_count++;
+ } else {
+ CHECK_EQ(elements_slot->kind(), TranslatedValue::kTagged);
+ elements_slot->GetValue();
+ if (purpose_ == kFrameInspection) {
+ // We are materializing a JSArray for the purpose of frame inspection.
+ // If we were to construct it with the above elements value then an
+ // actual deopt later on might create another JSArray instance with
+ // the same elements store. That would violate the key assumption
+ // behind left-trimming.
+ elements_slot->ReplaceElementsArrayWithCopy();
+ }
+ }
+
+ // Make sure all the remaining children (after the map, properties store,
+ // and possibly elements store) are allocated.
+ return EnsureChildrenAllocated(remaining_children_count, frame,
&value_index, worklist);
}
UNREACHABLE();
}
+void TranslatedValue::ReplaceElementsArrayWithCopy() {
+ DCHECK_EQ(kind(), TranslatedValue::kTagged);
+ DCHECK_EQ(materialization_state(), TranslatedValue::kFinished);
+ auto elements = Handle<FixedArrayBase>::cast(GetValue());
+ DCHECK(elements->IsFixedArray() || elements->IsFixedDoubleArray());
+ if (elements->IsFixedDoubleArray()) {
+ DCHECK(!elements->IsCowArray());
+ set_storage(isolate()->factory()->CopyFixedDoubleArray(
+ Handle<FixedDoubleArray>::cast(elements)));
+ } else if (!elements->IsCowArray()) {
+ set_storage(isolate()->factory()->CopyFixedArray(
+ Handle<FixedArray>::cast(elements)));
+ }
+}
+
void TranslatedState::EnsureChildrenAllocated(int count, TranslatedFrame* frame,
int* value_index,
std::stack<int>* worklist) {
@@ -4030,6 +4069,7 @@ Handle<ByteArray> TranslatedState::AllocateStorageFor(TranslatedValue* slot) {
void TranslatedState::EnsureJSObjectAllocated(TranslatedValue* slot,
Handle<Map> map) {
+ CHECK(map->IsJSObjectMap());
CHECK_EQ(map->instance_size(), slot->GetChildrenCount() * kTaggedSize);
Handle<ByteArray> object_storage = AllocateStorageFor(slot);
diff --git a/chromium/v8/src/deoptimizer/deoptimizer.h b/chromium/v8/src/deoptimizer/deoptimizer.h
index 152e5e510e9..2198f9d79c1 100644
--- a/chromium/v8/src/deoptimizer/deoptimizer.h
+++ b/chromium/v8/src/deoptimizer/deoptimizer.h
@@ -121,6 +121,8 @@ class TranslatedValue {
return storage_;
}
+ void ReplaceElementsArrayWithCopy();
+
Kind kind_;
MaterializationState materialization_state_ = kUninitialized;
TranslatedState* container_; // This is only needed for materialization of
@@ -317,7 +319,15 @@ class TranslatedFrame {
class TranslatedState {
public:
- TranslatedState() = default;
+ // There are two constructors, each for a different purpose:
+
+ // The default constructor is for the purpose of deoptimizing an optimized
+ // frame (replacing it with one or several unoptimized frames). It is used by
+ // the Deoptimizer.
+ TranslatedState() : purpose_(kDeoptimization) {}
+
+ // This constructor is for the purpose of merely inspecting an optimized
+ // frame. It is used by stack trace generation and various debugging features.
explicit TranslatedState(const JavaScriptFrame* frame);
void Prepare(Address stack_frame_pointer);
@@ -352,6 +362,12 @@ class TranslatedState {
private:
friend TranslatedValue;
+ // See the description of the constructors for an explanation of the two
+ // purposes. The only actual difference is that in the kFrameInspection case
+ // extra work is needed to not violate assumptions made by left-trimming. For
+ // details, see the code around ReplaceElementsArrayWithCopy.
+ enum Purpose { kDeoptimization, kFrameInspection };
+
TranslatedFrame CreateNextTranslatedFrame(TranslationIterator* iterator,
FixedArray literal_array,
Address fp, FILE* trace_file);
@@ -408,6 +424,7 @@ class TranslatedState {
static Float32 GetFloatSlot(Address fp, int slot_index);
static Float64 GetDoubleSlot(Address fp, int slot_index);
+ Purpose const purpose_;
std::vector<TranslatedFrame> frames_;
Isolate* isolate_ = nullptr;
Address stack_frame_pointer_ = kNullAddress;