Firefox · SpiderMonkey
CVE-2024-8384
Memory Corruption in SpiderMonkey
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
MakeScopeExitjs/src/gc/Sweeping.cpp |
modified | |
ifjs/src/gc/Sweeping.cpp |
modified | |
oomTestjs/src/jit-test/tests/gc/bug-1911288.js |
modified | |
ifjs/src/vm/Compartment.cpp |
modified | |
switchjs/src/vm/Compartment.cpp |
modified |
Files Changed
js/src/gc/Sweeping.cppjs/src/gc/Verifier.cppjs/src/jit-test/tests/gc/bug-1911288.jsjs/src/vm/Compartment.cppjs/src/vm/Compartment.h
Patch
diff --git a/js/src/gc/Sweeping.cpp b/js/src/gc/Sweeping.cpp
index 4175fc68be2..2d32900fc02 100644
--- a/js/src/gc/Sweeping.cpp
+++ b/js/src/gc/Sweeping.cpp
@@ -566,23 +566,32 @@ IncrementalProgress GCRuntime::markWeakReferencesInCurrentGroup(
IncrementalProgress GCRuntime::markGrayRoots(SliceBudget& budget,
gcstats::PhaseKind phase) {
- MOZ_ASSERT(marker().markColor() == MarkColor::Gray);
+ MOZ_ASSERT(marker().markColor() == MarkColor::Black);
gcstats::AutoPhase ap(stats(), phase);
- AutoUpdateLiveCompartments updateLive(this);
- marker().setRootMarkingMode(true);
- auto guard =
- mozilla::MakeScopeExit([this]() { marker().setRootMarkingMode(false); });
+ {
+ AutoSetMarkColor setColorGray(marker(), MarkColor::Gray);
- IncrementalProgress result =
- traceEmbeddingGrayRoots(marker().tracer(), budget);
- if (result == NotFinished) {
- return NotFinished;
+ AutoUpdateLiveCompartments updateLive(this);
+ marker().setRootMarkingMode(true);
+ auto guard = mozilla::MakeScopeExit(
+ [this]() { marker().setRootMarkingMode(false); });
+
+ IncrementalProgress result =
+ traceEmbeddingGrayRoots(marker().tracer(), budget);
+ if (result == NotFinished) {
+ return NotFinished;
+ }
+
+ Compartment::traceIncomingCrossCompartmentEdgesForZoneGC(
+ marker().tracer(), Compartment::GrayEdges);
}
+ // Also mark any incoming cross compartment edges that were originally gray
+ // but have been marked black by a barrier.
Compartment::traceIncomingCrossCompartmentEdgesForZoneGC(
- marker().tracer(), Compartment::GrayEdges);
+ marker().tracer(), Compartment::BlackEdges);
return Finished;
}
@@ -1144,8 +1153,6 @@ IncrementalProgress GCRuntime::markGrayRootsInCurrentGroup(
JS::GCContext* gcx, SliceBudget& budget) {
gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::MARK);
- AutoSetMarkColor setColorGray(marker(), MarkColor::Gray);
-
// Check that the zone state is set correctly for the current sweep group as
// that determines what gets marked.
MOZ_ASSERT(atomsZone()->wasGCStarted() ==
diff --git a/js/src/gc/Verifier.cpp b/js/src/gc/Verifier.cpp
index f139d542325..a2a330c3595 100644
--- a/js/src/gc/Verifier.cpp
+++ b/js/src/gc/Verifier.cpp
@@ -607,9 +607,13 @@ void js::gc::MarkingValidator::nonIncrementalMark(AutoGCSession& session) {
zone->changeGCState(zone->initialMarkingState(), Zone::MarkBlackAndGray);
}
- AutoSetMarkColor setColorGray(*gcmarker, MarkColor::Gray);
-
+ /*
+ * markAllGrayReferences may mark both gray and black, so it manages the
+ * mark color internally.
+ */
gc->markAllGrayReferences(gcstats::PhaseKind::MARK_GRAY);
+
+ AutoSetMarkColor setColorGray(*gcmarker, MarkColor::Gray);
gc->markAllWeakReferences();
/* Restore zone state. */
diff --git a/js/src/jit-test/tests/gc/bug-1911288.js b/js/src/jit-test/tests/gc/bug-1911288.js
new file mode 100644
index 00000000000..335d6adb575
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-1911288.js
@@ -0,0 +1,18 @@
+Object.defineProperty(this, "x", {});
+try {
+ oomTest(function() {
+ let m = parseModule(``);
+ });
+ var dbg = new Debugger;
+ dbg.onNewGlobalObject = function (global) {}
+ let t39 = {};
+ addMarkObservers([t39]);
+ let g33 = newGlobal({newCompartment: true});
+ g33.t39 = t39;
+ g33.eval("grayRoot().push(t);");
+} catch (exc) {}
+gcparam("allocationThreshold", 1 /* MB */);
+gcparam("incrementalGCEnabled", false);
+oomTest(function() {
+ var lfGlobal = newGlobal({sameZoneAs: this});
+});
diff --git a/js/src/vm/Compartment.cpp b/js/src/vm/Compartment.cpp
index 75008f0bfd5..45857b197a4 100644
--- a/js/src/vm/Compartment.cpp
+++ b/js/src/vm/Compartment.cpp
@@ -485,13 +485,18 @@ bool Compartment::wrap(JSContext* cx, MutableHandle<GCVector<Value>> vec) {
static inline bool ShouldTraceWrapper(JSObject* wrapper,
Compartment::EdgeSelector whichEdges) {
- if (whichEdges == Compartment::AllEdges) {
- return true;
+ switch (whichEdges) {
+ case Compartment::AllEdges:
+ return true;
+ case Compartment::NonGrayEdges:
+ return !wrapper->isMarkedGray();
+ case Compartment::GrayEdges:
+ return wrapper->isMarkedGray();
+ case Compartment::BlackEdges:
+ return wrapper->isMarkedBlack();
+ default:
+ MOZ_CRASH("Unexpected EdgeSelector value");
}
-
- bool isGray = wrapper->isMarkedGray();
- return (whichEdges == Compartment::NonGrayEdges && !isGray) ||
- (whichEdges == Compartment::GrayEdges && isGray);
}
void Compartment::traceWrapperTargetsInCollectedZones(JSTracer* trc,
diff --git a/js/src/vm/Compartment.h b/js/src/vm/Compartment.h
index 6cea7a7c3db..34aaf2dd00c 100644
--- a/js/src/vm/Compartment.h
+++ b/js/src/vm/Compartment.h
@@ -417,7 +417,7 @@ class JS::Compartment {
* dangling (full GCs naturally follow pointers across compartments) and
* when compacting to update cross-compartment pointers.
*/
- enum EdgeSelector { AllEdges, NonGrayEdges, GrayEdges };
+ enum EdgeSelector { AllEdges, NonGrayEdges, GrayEdges, BlackEdges };
void traceWrapperTargetsInCollectedZones(JSTracer* trc,
EdgeSelector whichEdges);
static void traceIncomingCrossCompartmentEdgesForZoneGC(
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/js/src/jit-test/tests/gc/bug-1911288.js b/js/src/jit-test/tests/gc/bug-1911288.js
new file mode 100644
index 00000000000..335d6adb575
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-1911288.js
@@ -0,0 +1,18 @@
+Object.defineProperty(this, "x", {});
+try {
+ oomTest(function() {
+ let m = parseModule(``);
+ });
+ var dbg = new Debugger;
+ dbg.onNewGlobalObject = function (global) {}
+ let t39 = {};
+ addMarkObservers([t39]);
+ let g33 = newGlobal({newCompartment: true});
+ g33.t39 = t39;
+ g33.eval("grayRoot().push(t);");
+} catch (exc) {}
+gcparam("allocationThreshold", 1 /* MB */);
+gcparam("incrementalGCEnabled", false);
+oomTest(function() {
+ var lfGlobal = newGlobal({sameZoneAs: this});
+});
Loading diff…
References
On This Page