summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTobias Tebbi <tebbi@chromium.org>2022-06-17 10:14:44 +0000
committerMichael BrĂ¼ning <michael.bruning@qt.io>2022-07-25 14:23:44 +0000
commit1461ea96319c7ed5eb5323ba17749b57a6825832 (patch)
tree8e10589ec803ddf066a01d627caa6dd8e45f412f
parentcf40317ef3d2a563c174deecc9a4f89caca38fe5 (diff)
downloadqtwebengine-chromium-1461ea96319c7ed5eb5323ba17749b57a6825832.tar.gz
[Backport] CVE-2022-2295: Type Confusion in V8
Partial cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/3726007: make CanCover() transitive In addition to checking that a node is owned, CanCover() also needs to check if there are any side-effects in between the current node and the merged node. When merging inputs of inputs, this check was done with the wrong side-effect level of the in-between node. We partially fixed this before with `CanCoverTransitively`. This CL addresses the issue by always comparing to the side-effect level of the node from which we started, making `CanCoverTransitively` superfluous. (cherry picked from commit 6048f754931e0971cab58fb0de785482d175175b) Bug: chromium:1336869 No-Try: true No-Presubmit: true No-Tree-Checks: true Change-Id: I78479b32461ede81138f8b5d48d60058cfb5fa0a Commit-Queue: Tobias Tebbi <tebbi@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#81217} Commit-Queue: Roger Felipe Zanoni da Silva <rzanoni@google.com> Reviewed-by: Artem Sumaneev <asumaneev@google.com> Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Owners-Override: Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/branch-heads/9.6@{#70} Cr-Branched-From: 0b7bda016178bf438f09b3c93da572ae3663a1f7-refs/heads/9.6.180@{#1} Cr-Branched-From: 41a5a247d9430b953e38631e88d17790306f7a4c-refs/heads/main@{#77244} Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r--chromium/v8/src/compiler/backend/instruction-selector.cc21
-rw-r--r--chromium/v8/src/compiler/backend/instruction-selector.h14
-rw-r--r--chromium/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc2
-rw-r--r--chromium/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc2
-rw-r--r--chromium/v8/src/compiler/backend/x64/instruction-selector-x64.cc2
5 files changed, 13 insertions, 28 deletions
diff --git a/chromium/v8/src/compiler/backend/instruction-selector.cc b/chromium/v8/src/compiler/backend/instruction-selector.cc
index 92c11e09b44..27a5d85b1b9 100644
--- a/chromium/v8/src/compiler/backend/instruction-selector.cc
+++ b/chromium/v8/src/compiler/backend/instruction-selector.cc
@@ -284,7 +284,7 @@ Instruction* InstructionSelector::Emit(Instruction* instr) {
bool InstructionSelector::CanCover(Node* user, Node* node) const {
// 1. Both {user} and {node} must be in the same basic block.
- if (schedule()->block(node) != schedule()->block(user)) {
+ if (schedule()->block(node) != current_block_) {
return false;
}
// 2. Pure {node}s must be owned by the {user}.
@@ -292,7 +292,7 @@ bool InstructionSelector::CanCover(Node* user, Node* node) const {
return node->OwnedBy(user);
}
// 3. Impure {node}s must match the effect level of {user}.
- if (GetEffectLevel(node) != GetEffectLevel(user)) {
+ if (GetEffectLevel(node) != current_effect_level_) {
return false;
}
// 4. Only {node} must have value edges pointing to {user}.
@@ -304,21 +304,6 @@ bool InstructionSelector::CanCover(Node* user, Node* node) const {
return true;
}
-bool InstructionSelector::CanCoverTransitively(Node* user, Node* node,
- Node* node_input) const {
- if (CanCover(user, node) && CanCover(node, node_input)) {
- // If {node} is pure, transitivity might not hold.
- if (node->op()->HasProperty(Operator::kPure)) {
- // If {node_input} is pure, the effect levels do not matter.
- if (node_input->op()->HasProperty(Operator::kPure)) return true;
- // Otherwise, {user} and {node_input} must have the same effect level.
- return GetEffectLevel(user) == GetEffectLevel(node_input);
- }
- return true;
- }
- return false;
-}
-
bool InstructionSelector::IsOnlyUserOfNodeInSameBlock(Node* user,
Node* node) const {
BasicBlock* bb_user = schedule()->block(user);
@@ -1226,6 +1211,7 @@ void InstructionSelector::VisitBlock(BasicBlock* block) {
int effect_level = 0;
for (Node* const node : *block) {
SetEffectLevel(node, effect_level);
+ current_effect_level_ = effect_level;
if (node->opcode() == IrOpcode::kStore ||
node->opcode() == IrOpcode::kUnalignedStore ||
node->opcode() == IrOpcode::kCall ||
@@ -1245,6 +1231,7 @@ void InstructionSelector::VisitBlock(BasicBlock* block) {
// control input should be on the same effect level as the last node.
if (block->control_input() != nullptr) {
SetEffectLevel(block->control_input(), effect_level);
+ current_effect_level_ = effect_level;
}
auto FinishEmittedInstructions = [&](Node* node, int instruction_start) {
diff --git a/chromium/v8/src/compiler/backend/instruction-selector.h b/chromium/v8/src/compiler/backend/instruction-selector.h
index 11a329d1d6e..f8ede1f37ff 100644
--- a/chromium/v8/src/compiler/backend/instruction-selector.h
+++ b/chromium/v8/src/compiler/backend/instruction-selector.h
@@ -452,15 +452,12 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
// Used in pattern matching during code generation.
// Check if {node} can be covered while generating code for the current
// instruction. A node can be covered if the {user} of the node has the only
- // edge and the two are in the same basic block.
- // Before fusing two instructions a and b, it is useful to check that
- // CanCover(a, b) holds. If this is not the case, code for b must still be
- // generated for other users, and fusing is unlikely to improve performance.
+ // edge, the two are in the same basic block, and there are no side-effects
+ // in-between. The last check is crucial for soundness.
+ // For pure nodes, CanCover(a,b) is checked to avoid duplicated execution:
+ // If this is not the case, code for b must still be generated for other
+ // users, and fusing is unlikely to improve performance.
bool CanCover(Node* user, Node* node) const;
- // CanCover is not transitive. The counter example are Nodes A,B,C such that
- // CanCover(A, B) and CanCover(B,C) and B is pure: The the effect level of A
- // and B might differ. CanCoverTransitively does the additional checks.
- bool CanCoverTransitively(Node* user, Node* node, Node* node_input) const;
// Used in pattern matching during code generation.
// This function checks that {node} and {user} are in the same basic block,
@@ -787,6 +784,7 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
BoolVector defined_;
BoolVector used_;
IntVector effect_level_;
+ int current_effect_level_;
IntVector virtual_registers_;
IntVector virtual_register_rename_;
InstructionScheduler* scheduler_;
diff --git a/chromium/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc b/chromium/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc
index bec7bbefdcb..42c00365037 100644
--- a/chromium/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc
+++ b/chromium/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc
@@ -1527,7 +1527,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) {
if (CanCover(node, value)) {
switch (value->opcode()) {
case IrOpcode::kWord64Sar: {
- if (CanCoverTransitively(node, value, value->InputAt(0)) &&
+ if (CanCover(value, value->InputAt(0)) &&
TryEmitExtendingLoad(this, value, node)) {
return;
} else {
diff --git a/chromium/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc b/chromium/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc
index 72706201e2a..210add8dfb6 100644
--- a/chromium/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc
+++ b/chromium/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc
@@ -1323,7 +1323,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) {
if (CanCover(node, value)) {
switch (value->opcode()) {
case IrOpcode::kWord64Sar: {
- if (CanCoverTransitively(node, value, value->InputAt(0)) &&
+ if (CanCover(value, value->InputAt(0)) &&
TryEmitExtendingLoad(this, value, node)) {
return;
} else {
diff --git a/chromium/v8/src/compiler/backend/x64/instruction-selector-x64.cc b/chromium/v8/src/compiler/backend/x64/instruction-selector-x64.cc
index 53ee75064bb..5c86dbc0c69 100644
--- a/chromium/v8/src/compiler/backend/x64/instruction-selector-x64.cc
+++ b/chromium/v8/src/compiler/backend/x64/instruction-selector-x64.cc
@@ -1729,7 +1729,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) {
case IrOpcode::kWord64Shr: {
Int64BinopMatcher m(value);
if (m.right().Is(32)) {
- if (CanCoverTransitively(node, value, value->InputAt(0)) &&
+ if (CanCover(value, value->InputAt(0)) &&
TryMatchLoadWord64AndShiftRight(this, value, kX64Movl)) {
return EmitIdentity(node);
}