Firefox · SpiderMonkey
CVE-2026-84118
UAF in SpiderMonkey
Overview
High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifjs/src/gc/AtomMarking-inl.h |
modified | |
ifjs/src/gc/Marking.cpp |
modified | |
whilejs/src/gc/Sweeping.cpp |
modified |
Files Changed
js/src/gc/AtomMarking-inl.hjs/src/gc/GCRuntime.hjs/src/gc/Marking.cppjs/src/gc/Sweeping.cppjs/src/gc/WeakMap-inl.hjs/src/gc/WeakMap.cppjs/src/jit-test/tests/gc/bug-1997896.js
Patch
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…
References
On This Page