Medium firefox Logic Error 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionAn attacker could read 32 bits of values spilled onto the stack in a JIT compiled function.
ComponentSpiderMonkey
Bug ClassLogic Error
Tracker1947141
Fix commitc7c404f78c99 (firefox) +127/-55
CISA KEVNot listed
Creditedanbu
Disclosed2025-04-01

Changed Functions

FunctionChangeNotes
if
js/src/jit/BacktrackingAllocator.cpp
modified
LStackSlot
js/src/jit/LIR.h
modified
switch
js/src/jit/LIR.h
modified
if
js/src/jit/Safepoints.cpp
modified

Files Changed

  • js/src/jit/BacktrackingAllocator.cpp
  • js/src/jit/BacktrackingAllocator.h
  • js/src/jit/LIR.cpp
  • js/src/jit/LIR.h
  • js/src/jit/Safepoints.cpp
  • js/src/jit/StackSlotAllocator.h
  • js/src/jit/shared/CodeGenerator-shared.cpp
  • js/src/jit/shared/LIR-shared.h
diff --git a/js/src/jit/BacktrackingAllocator.cpp b/js/src/jit/BacktrackingAllocator.cpp
index ba8551469b0..d97f187c191 100644
--- a/js/src/jit/BacktrackingAllocator.cpp
+++ b/js/src/jit/BacktrackingAllocator.cpp
@@ -2095,7 +2095,7 @@ static bool CanMergeTypesInBundle(LDefinition::Type a, LDefinition::Type b) {
 
   // Only merge if the sizes match, so that we don't get confused about the
   // width of spill slots.
-  return StackSlotAllocator::width(a) == StackSlotAllocator::width(b);
+  return LStackSlot::width(a) == LStackSlot::width(b);
 }
 
 // Helper for ::tryMergeReusedRegister
@@ -3892,14 +3892,14 @@ bool BacktrackingAllocator::pickStackSlot(SpillSet* spillSet) {
       spillSet->spilledBundle(0)->firstRange()->vreg().type();
 
   SpillSlotList* slotList;
-  switch (StackSlotAllocator::width(type)) {
-    case 4:
+  switch (LStackSlot::width(type)) {
+    case LStackSlot::Word:
       slotList = &normalSlots;
       break;
-    case 8:
+    case LStackSlot::DoubleWord:
       slotList = &doubleSlots;
       break;
-    case 16:
+    case LStackSlot::QuadWord:
       slotList = &quadSlots;
       break;
     default:
@@ -3963,10 +3963,11 @@ bool BacktrackingAllocator::pickStackSlot(SpillSet* spillSet) {
   }
 
   // We need a new physical stack slot.
-  uint32_t stackSlot = stackSlotAllocator.allocateSlot(type);
+  LStackSlot::Width width = LStackSlot::width(type);
+  uint32_t stackSlot = stackSlotAllocator.allocateSlot(width);
 
   SpillSlot* spillSlot =
-      new (alloc().fallible()) SpillSlot(stackSlot, alloc().lifoAlloc());
+      new (alloc().fallible()) SpillSlot(stackSlot, width, alloc().lifoAlloc());
   if (!spillSlot) {
     return false;
   }
diff --git a/js/src/jit/BacktrackingAllocator.h b/js/src/jit/BacktrackingAllocator.h
index e3c815992f8..f65f0cee59b 100644
--- a/js/src/jit/BacktrackingAllocator.h
+++ b/js/src/jit/BacktrackingAllocator.h
@@ -726,8 +726,8 @@ class BacktrackingAllocator : protected RegisterAllocator {
     LStackSlot alloc;
     LiveRangePlusSet allocated;
 
-    SpillSlot(uint32_t slot, LifoAlloc* alloc)
-        : alloc(slot), allocated(alloc) {}
+    SpillSlot(uint32_t slot, LStackSlot::Width width, LifoAlloc* alloc)
+        : alloc(slot, width), allocated(alloc) {}
   };
   using SpillSlotList = InlineForwardList<SpillSlot>;
 
diff --git a/js/src/jit/LIR.cpp b/js/src/jit/LIR.cpp
index 7e374144e0c..cecdc666f62 100644
--- a/js/src/jit/LIR.cpp
+++ b/js/src/jit/LIR.cpp
@@ -494,7 +494,8 @@ UniqueChars LAllocation::toString() const {
         buf = JS_smprintf("%s", toFloatReg()->reg().name());
         break;
       case LAllocation::STACK_SLOT:
-        buf = JS_smprintf("stack:%u", toStackSlot()->slot());
+        buf = JS_smprintf("stack:%u(%u)", toStackSlot()->slot(),
+                          LStackSlot::ByteWidth(toStackSlot()->width()));
         break;
       case LAllocation::ARGUMENT_SLOT:
         buf = JS_smprintf("arg:%u", toArgument()->index());
diff --git a/js/src/jit/LIR.h b/js/src/jit/LIR.h
index 15ddc3ee821..fbce20245b8 100644
--- a/js/src/jit/LIR.h
+++ b/js/src/jit/LIR.h
@@ -392,10 +392,51 @@ class LConstantIndex : public LAllocation {
 
 // Stack slots are indices into the stack. The indices are byte indices.
 class LStackSlot : public LAllocation {
+  // Stack slots are aligned to 32-bit word boundaries.
+  static constexpr uint32_t SLOT_ALIGNMENT = 4;
+
+  // Stack slot width is stored in the two least significant bits.
+  static constexpr uint32_t WIDTH_MASK = SLOT_ALIGNMENT - 1;
+
+  // Remaining bits hold the stack slot offset.
+  static constexpr uint32_t SLOT_MASK = ~WIDTH_MASK;
+
  public:
-  explicit LStackSlot(uint32_t slot) : LAllocation(STACK_SLOT, slot) {}
+  enum Width {
+    Word,
+    DoubleWord,
+    QuadWord,
+  };
+
+  LStackSlot(uint32_t slot, Width width)
+      : LAllocation(STACK_SLOT, slotAndWidth(slot, width)) {}
+
+  uint32_t slot() const { return data() & SLOT_MASK; }
+
+  Width width() const { return Width(data() & WIDTH_MASK); }
+
+  // |Type| is LDefinition::Type, but can't forward declare a nested definition.
+  template <typename Type>
+  static Width width(Type type);
+
+  static uint32_t ByteWidth(Width width) {
+    switch (width) {
+      case Width::Word:
+        return 4;
+      case Width::DoubleWord:
+        return 8;
+      case Width::QuadWord:
+        return 16;
+    }
+    MOZ_CRASH("invalid width");
+  }
 
-  uint32_t slot() const { return data(); }
+ private:
+  static uint32_t slotAndWidth(uint32_t slot, Width width) {
+    MOZ_ASSERT(slot % SLOT_ALIGNMENT == 0);
+    MOZ_ASSERT(uint32_t(width) < SLOT_ALIGNMENT);
+    return slot | uint32_t(width);
+  }
 };
 
 // Stack area indicates a contiguous stack allocation meant to receive call
@@ -689,6 +730,41 @@ class LInt64Definition : public LInt64Value<LDefinition> {
   }
 };
 
+template <>
+inline LStackSlot::Width LStackSlot::width(LDefinition::Type type) {
+  switch (type) {
+#if JS_BITS_PER_WORD == 32
+    case LDefinition::GENERAL:
+    case LDefinition::OBJECT:
+    case LDefinition::SLOTS:
+    case LDefinition::WASM_ANYREF:
+#endif
+#ifdef JS_NUNBOX32
+    case LDefinition::TYPE:
+    case LDefinition::PAYLOAD:
+#endif
+    case LDefinition::INT32:
+    case LDefinition::FLOAT32:
+      return LStackSlot::Word;
+#if JS_BITS_PER_WORD == 64
+    case LDefinition::GENERAL:
+    case LDefinition::OBJECT:
+    case LDefinition::SLOTS:
+    case LDefinition::WASM_ANYREF:
+#endif
+#ifdef JS_PUNBOX64
+    case LDefinition::BOX:
+#endif
+    case LDefinition::DOUBLE:
+      return LStackSlot::DoubleWord;
+    case LDefinition::SIMD128:
+      return LStackSlot::QuadWord;
+    case LDefinition::STACKRESULTS:
+      MOZ_CRASH("Stack results area must be allocated manually");
+  }
+  MOZ_CRASH("Unknown slot type");
+}
+
 // Forward declarations of LIR types.
 #define LIROP(op) class L##op;
 LIR_OPCODE_LIST(LIROP)
diff --git a/js/src/jit/Safepoints.cpp b/js/src/jit/Safepoints.cpp
index db1c05b70ce..77245b1cc27 100644
--- a/js/src/jit/Safepoints.cpp
+++ b/js/src/jit/Safepoints.cpp
@@ -541,7 +541,7 @@ static inline LAllocation PartFromStream(CompactBufferReader& stream,
   }
 
   if (kind == Part_Stack) {
-    return LStackSlot(info);
+    return LStackSlot(info, LStackSlot::Word);
   }
 
   MOZ_ASSERT(kind == Part_Arg);
diff --git a/js/src/jit/StackSlotAllocator.h b/js/src/jit/StackSlotAllocator.h
index 1f11b0e785b..c8f8893c563 100644
--- a/js/src/jit/StackSlotAllocator.h
+++ b/js/src/jit/StackSlotAllocator.h
@@ -7,6 +7,7 @@
 #ifndef jit_StackSlotAllocator_h
 #define jit_StackSlotAllocator_h
 
+#include "jit/LIR.h"
 #include "jit/Registers.h"
 
 namespace js {
Loading diff…