summaryrefslogtreecommitdiff
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-09 10:50:58 +0000
commit902359b38d32e2261dedc810a06b4bee209b940d (patch)
treed1b847d5e841c1864875fcf3adf70abe209fb885
parenta775361f525f4a2054eec3c75be57bb0aa966356 (diff)
downloadqtwebengine-chromium-902359b38d32e2261dedc810a06b4bee209b940d.tar.gz
[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.cc58
-rw-r--r--chromium/v8/src/deoptimizer.h19
2 files changed, 67 insertions, 10 deletions
diff --git a/chromium/v8/src/deoptimizer.cc b/chromium/v8/src/deoptimizer.cc
index 983e222ca9d..0e9bf63249b 100644
--- a/chromium/v8/src/deoptimizer.cc
+++ b/chromium/v8/src/deoptimizer.cc
@@ -3222,7 +3222,8 @@ int TranslatedState::CreateNextTranslatedValue(
FATAL("We should never get here - unexpected deopt info.");
}
-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(
@@ -3596,25 +3597,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) {
@@ -3680,6 +3719,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() * kPointerSize);
Handle<ByteArray> object_storage = AllocateStorageFor(slot);
diff --git a/chromium/v8/src/deoptimizer.h b/chromium/v8/src/deoptimizer.h
index ef47fcc642c..c84139e786b 100644
--- a/chromium/v8/src/deoptimizer.h
+++ b/chromium/v8/src/deoptimizer.h
@@ -108,6 +108,8 @@ class TranslatedValue {
return storage_;
}
+ void ReplaceElementsArrayWithCopy();
+
Kind kind_;
MaterializationState materialization_state_ = kUninitialized;
TranslatedState* container_; // This is only needed for materialization of
@@ -286,7 +288,15 @@ class TranslatedFrame {
class TranslatedState {
public:
- TranslatedState() {}
+ // 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);
@@ -320,6 +330,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,
@@ -376,6 +392,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;