High firefox UAF 🔧 Commit mapped

Overview

High
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionUse-after-free in the JavaScript: GC component
ComponentSpiderMonkey
Bug ClassUAF
Tracker2057457
Fix commite147abd18359 (firefox) +91/-72
CISA KEVNot listed
Creditedx0e
Disclosed2026-09-01

Changed Functions

FunctionChangeNotes
if
js/src/gc/AtomMarking-inl.h
modified
if
js/src/gc/Marking.cpp
modified
while
js/src/gc/Sweeping.cpp
modified

Files Changed

  • js/src/gc/AtomMarking-inl.h
  • js/src/gc/GCRuntime.h
  • js/src/gc/Marking.cpp
  • js/src/gc/Sweeping.cpp
  • js/src/gc/WeakMap-inl.h
  • js/src/gc/WeakMap.cpp
  • js/src/jit-test/tests/gc/bug-1997896.js
diff --git a/js/src/gc/AtomMarking-inl.h b/js/src/gc/AtomMarking-inl.h
index 63236f47fa1..949cb437a52 100644
--- a/js/src/gc/AtomMarking-inl.h
+++ b/js/src/gc/AtomMarking-inl.h
@@ -118,31 +118,80 @@ inline void AtomRefRuntime::maybeUnmarkGrayAtomically(Zone* zone,
   MOZ_ASSERT(getRefColor(zone, symbol) == CellColor::Black);
 }
 
-inline bool GCRuntime::isSymbolReferencedByUncollectedZone(JS::Symbol* sym,
-                                                           MarkColor color) {
-  MOZ_ASSERT(sym->zone()->isAtomsZone());
+template <typename T>
+inline void GCRuntime::maybeMarkWeaklyHeldAtom(T* atom) {
+  // To effectively refine atom references we do it at the end of collection
+  // before marking atoms referenced from uncollected zones in
+  // GCRuntime::updateAtomsBitmap. The refinement step is to AND each zone's
+  // references with the atoms currently marked.
+  //
+  // Conceptually, it would be simplest to mark references from uncollected
+  // zones at the start of collection. Delaying this ensures that references
+  // from uncollected zones don't stop us removing dead references from
+  // collected zones (otherwise we would require all zones with references to
+  // that atom to be collected at the same time to drop them).
+  //
+  // The problem is weak references: we need to keep the zone's reference but we
+  // never directly mark the atom. To fix this we mark atoms referenced by
+  // uncollected zones when we encounter weak references to them. This would
+  // have happened later anyway so the final mark state is not affected. Doing
+  // this means the atom is marked before the refinement step which then keeps
+  // the reference.
+
+  static_assert(std::is_same_v<T, JSAtom> || std::is_same_v<T, JS::Symbol>);
+
+  Zone* zone = atom->zoneFromAnyThread();
+  MOZ_ASSERT(zone->isAtomsZone());
+  if (!zone->isGCMarkingOrSweeping()) {
+    return;
+  }
+
+  CellColor refColor = isAtomReferencedByUncollectedZone(&atom->asTenured());
+  if (refColor == CellColor::White) {
+    return;
+  }
+
+  // Set the mark bits directly since this may be called after normal marking
+  // has finished. Implicitly marked edges are handled via weakmap marking which
+  // happens after this.
+  MarkColor color = AsMarkColor(refColor);
+  (void)atom->asTenured().markIfUnmarked(color);
+  if constexpr (std::is_same_v<T, JS::Symbol>) {
+    if (JSAtom* description = atom->description()) {
+      (void)description->asTenured().markIfUnmarked(color);
+    }
+  }
+}
+
+inline CellColor GCRuntime::isAtomReferencedByUncollectedZone(
+    TenuredCell* atom) {
+  MOZ_ASSERT(atom->zoneFromAnyThread()->isAtomsZone());
 
   if (!atomsUsedByUncollectedZones.ref()) {
-    return false;
+    return CellColor::White;
   }
 
   MOZ_ASSERT(atomsZone()->wasGCStarted());
 
-  size_t bit = AtomRefRuntime::getAtomBit(sym);
+  size_t bit = AtomRefRuntime::getAtomBit(atom);
   size_t blackBit = bit + size_t(ColorBit::BlackBit);
   size_t grayOrBlackBit = bit + size_t(ColorBit::GrayOrBlackBit);
   MOZ_ASSERT(grayOrBlackBit / JS_BITS_PER_WORD < atomReferences.allocatedWords);
 
   const DenseBitmap& bitmap = *atomsUsedByUncollectedZones.ref();
   if (grayOrBlackBit >= bitmap.count()) {
-    return false;  // Atom created during collection.
+    return CellColor::White;  // Atom created during collection.
   }
 
   if (bitmap.getBit(blackBit)) {
-    return true;
+    return CellColor::Black;
+  }
+
+  if (bitmap.getBit(grayOrBlackBit)) {
+    return CellColor::Gray;
   }
 
-  return color == MarkColor::Gray && bitmap.getBit(grayOrBlackBit);
+  return CellColor::White;
 }
 
 void AtomRefRuntime::recordChildren(Zone* zone, JSAtom*) {}
diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h
index 566de344d58..9108a644c0c 100644
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -780,9 +780,11 @@ class GCRuntime {
   static void* refillFreeList(JS::Zone* zone, AllocKind thingKind);
   void attemptLastDitchGC();
 
-  // Return whether |sym| is marked at least |color| in the atom reference state
-  // for uncollected zones.
-  bool isSymbolReferencedByUncollectedZone(JS::Symbol* sym, MarkColor color);
+  // Return the mark color for |sym| in the atom reference state for uncollected
+  // zones, or MarkColor::White if it's not referenced.
+  CellColor isAtomReferencedByUncollectedZone(TenuredCell* atom);
+  template <typename T>
+  void maybeMarkWeaklyHeldAtom(T* atom);
 
   // Test mark queue.
 #ifdef DEBUG
@@ -973,7 +975,6 @@ class GCRuntime {
 
   template <class ZoneIterT>
   IncrementalProgress markWeakReferences(JS::SliceBudget& budget);
-  void markIncomingGraySymbolEdgesFromUncollectedZones();
   IncrementalProgress markWeakReferencesInCurrentGroup(JS::SliceBudget& budget);
   IncrementalProgress markGrayRoots(JS::SliceBudget& budget,
                                     gcstats::PhaseKind phase);
diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp
index 8b4ec0c5eb9..ec85c3aad9b 100644
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -3207,9 +3207,21 @@ inline bool SweepingTracer::onEdge(T** thingp, const char* name) {
   //  - atoms
   //  - the jitcode map
   //  - the mark queue
-  bool sweepZone =
+  bool sweepingZone =
       zone->isGCSweeping() || (zone->isAtomsZone() && zone->isGCMarking());
-  return !(sweepZone && !cell->isMarkedAny());
+  if (!sweepingZone) {
+    return true;
+  }
+
+  if constexpr (std::is_same_v<T, JS::Symbol>) {
+    runtime()->gc.maybeMarkWeaklyHeldAtom(thing);
+  } else if constexpr (std::is_same_v<T, JSString>) {
+    if (thing->isAtom()) {
+      runtime()->gc.maybeMarkWeaklyHeldAtom(&thing->asAtom());
+    }
+  }
+
+  return cell->isMarkedAny();
 }
 
 namespace js::gc {
diff --git a/js/src/gc/Sweeping.cpp b/js/src/gc/Sweeping.cpp
index 4220d3f0484..3c961ea7a9b 100644
--- a/js/src/gc/Sweeping.cpp
+++ b/js/src/gc/Sweeping.cpp
@@ -673,8 +673,6 @@ IncrementalProgress GCRuntime::markWeakReferences(
     }
   }
 
-  markIncomingGraySymbolEdgesFromUncollectedZones();
-
   bool markedAny = true;
   while (markedAny) {
     if (!marker().markUntilBudgetExhausted(budget)) {
@@ -697,37 +695,6 @@ IncrementalProgress GCRuntime::markWeakReferences(
   return Finished;
 }
 
-void GCRuntime::markIncomingGraySymbolEdgesFromUncollectedZones() {
-  // We need to mark through ephemeron edges where the source is a live symbol
-  // that is referenced from an uncollected zone and which may not have been
-  // marked in this GC. At the same time we want to avoid unnecessarily holding
-  // on to symbols in zone GCs (by recording them in the atom reference bitmap),
-  // which is why we don't just mark all such symbols at the start of GC.
-  //
-  // This situation arises because WeakMap::markEntry may find an unmarked
-  // symbol key that is marked gray by uncollected zones while it is currently
-  // marking black. It can't mark it at that time so it leaves it alone; we mark
-  // it here instead when we are gray weak marking.
-  //
-  // Atoms referenced by uncollected zones will be marked later in
-  // updateAtomsBitmap() which prevents them dying, but since this is after
-  // we've done ephemeron marking it won't mark through the ephemeron edges.
-
-  if (marker().markColor() != MarkColor::Gray || !atomsZone()->isGCMarking()) {
-    return;
-  }
-
-  for (auto iter = atomsZone()->gcEphemeronEdges().iter(); !iter.done();
-       iter.next()) {
-    auto* symbol = iter.get().key()->as<JS::Symbol>();
-    if (isSymbolReferencedByUncollectedZone(symbol, marker().markColor())) {
-      TraceManuallyBarrieredEdge(marker().tracer(), &symbol,
-                                 "incoming symbol edge");
-      MOZ_ASSERT(symbol == iter.get().key());
-    }
-  }
-}
-
 IncrementalProgress GCRuntime::markWeakReferencesInCurrentGroup(
     SliceBudget& budget) {
   return markWeakReferences<SweepGroupZonesIter>(budget);
@@ -1751,14 +1718,6 @@ IncrementalProgress GCRuntime::beginSweepingSweepGroup(JS::GCContext* gcx,
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/js/src/jit-test/tests/gc/bug-1997896.js b/js/src/jit-test/tests/gc/bug-1997896.js
index 48180d227ea..e55bb88de59 100644
--- a/js/src/jit-test/tests/gc/bug-1997896.js
+++ b/js/src/jit-test/tests/gc/bug-1997896.js
@@ -50,5 +50,6 @@ startgc();
 assertEq(gcstate(), 'NotActive');
 
 checkMarks(['gray', 'black', 'black']);
-assertEq(getAtomMarkColor(g1, i), 'gray');
+// Mark observers weak edge (and any JIT weak edges) hold the atom ref black.
+assertEq(getAtomMarkColor(g1, i), 'black');
 assertEq(getAtomMarkColor(g2, i), 'black');
Loading diff…