Assertion failure: (Incremental marking verification failed), at gc/Verifier.cpp:759
Categories
(Core :: JavaScript: GC, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox113 | --- | fixed |
People
(Reporter: lukas.bernhard, Assigned: jonco)
References
(Blocks 3 open bugs)
Details
Attachments
(2 files)
Steps to reproduce:
On git commit 1d6e2f82287c298f77f21ad0f62f1aed6155577c I encountered an assertion violation in the GC, hence flagging as s-s.
Unfortunately, the I only managed to reproduce the crash when supplying 2 separate files via the reprl interface (standard for Fuzzilli fuzzing). Similar to bug 1813556, neither concatenating nor -f a.js -f b.js worked. I wrote a small python script which instantiates the js-shell in reprl mode and feeds the 2 snippets.
First, there will be an assertion violation at Assertion failure: rt->supportedImportAssertions.ref().empty()
, at js/src/vm/Modules.cpp:45. This one in probably unrelated; I removed this assert as it triggers occasionally during fuzzing.
#0 0x000055555850af71 in js::gc::MarkingValidator::validate (this=0x7ffff4cf1fc0)
at js/src/gc/Verifier.cpp:759
#1 0x00005555584ff412 in js::gc::GCRuntime::validateIncrementalMarking (this=0x7ffff7423728)
at js/src/gc/Verifier.cpp:775
#2 0x00005555584fe96c in js::gc::GCRuntime::beginSweepingSweepGroup (this=0x7ffff7423728, gcx=0x7ffff7423740,
budget=...) at js/src/gc/Sweeping.cpp:1506
#3 0x000055555852d8ea in sweepaction::SweepActionCall::run (this=0x7ffff741c2e0, args=...)
at js/src/gc/Sweeping.cpp:2058
#4 0x0000555558536f0d in sweepaction::SweepActionSequence::run (this=0x7ffff7406060, args=...)
at js/src/gc/Sweeping.cpp:2128
#5 0x000055555852cc7a in sweepaction::SweepActionForEach<js::gc::SweepGroupsIter, JSRuntime*>::run (
this=0x7ffff741a0a0, args=...) at js/src/gc/Sweeping.cpp:2163
#6 0x00005555585027be in js::gc::GCRuntime::performSweepActions (this=0x7ffff7423728, budget=...)
at js/src/gc/Sweeping.cpp:2305
#7 0x0000555558403a83 in js::gc::GCRuntime::incrementalSlice (this=0x7ffff7423728, budget=...,
reason=JS::GCReason::DEBUG_GC, budgetWasIncreased=false) at js/src/gc/GC.cpp:3683
#8 0x0000555558405e55 in js::gc::GCRuntime::gcCycle (this=0x7ffff7423728, nonincrementalByAPI=false, budgetArg=...,
reason=JS::GCReason::DEBUG_GC) at js/src/gc/GC.cpp:4194
#9 0x0000555558406dd2 in js::gc::GCRuntime::collect (this=0x7ffff7423728, nonincrementalByAPI=false, budget=...,
reason=JS::GCReason::DEBUG_GC) at js/src/gc/GC.cpp:4382
#10 0x0000555558407a11 in js::gc::GCRuntime::debugGCSlice (this=0x7ffff7423728, budget=...)
at js/src/gc/GC.cpp:4538
#11 0x0000555557e3a417 in GCSlice (cx=0x7ffff742f100, argc=1, vp=0x7fffffffa818)
at js/src/builtin/TestingFunctions.cpp:2638
#12 0x00002f6730bd9a3c in ?? ()
#13 0x0000000000000000 in ?? ()
Reporter | ||
Updated•2 years ago
|
Reporter | ||
Updated•2 years ago
|
Updated•2 years ago
|
Comment 1•2 years ago
|
||
Is this assertion a security issue, and in a non-debug build what would happen next?
Assignee | ||
Comment 2•2 years ago
|
||
This is complaining because the atom "wrapWithProto" is not being marked by incremental marking.
Non-incremental marking would mark this as part of a Warp snapshot, traced via WarpCacheIR::traceData, via WarpSnapshot::trace, via GlobalHelperThreadState::trace.
The atom is stored in a WarpGCPtr, which has the following comment:
// Note: no pre-barrier is needed because this is a constant. No post-barrier
// is needed because the value is always tenured.
const T ptr_;
The first part of the comment is not entirely true. A pre-barrier is required when an edge is removed, as well when it is mutated. So being constant throughout its lifetime is not sufficient to remove the barrier. I suspect this is what's wrong.
Presumably this pointer ends up baked into the JIT code. If so then this is security issue, as it would cause use after free if the string is collected.
Assignee | ||
Comment 3•2 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #2)
The first part of the comment is not entirely true. A pre-barrier is required when an edge is removed, as well when it is mutated. So being constant throughout its lifetime is not sufficient to remove the barrier. I suspect this is what's wrong.
OK, this is not what's wrong, but the comment is still misleading. These WarpGCPtrs are marked as roots, which is why they don't need the prebarrier.
Updated•2 years ago
|
Comment 4•2 years ago
|
||
I'm confused about what comment 3 is saying about comment 2. Does the fact that the pointers are marked as roots mean that it's not a security issue, or was it simply an explanation about the code comment?
Assignee | ||
Comment 5•2 years ago
|
||
(In reply to Daniel Veditz [:dveditz] from comment #4)
That was simply an explanation about my previous comment. A failure in incremental marking verification means that we could potentially have a UAF, so this is still a security issue.
Assignee | ||
Comment 6•2 years ago
|
||
It turns out that this is a problem with incremental marking verification, and therefore not security sensitive.
Assignee | ||
Comment 7•2 years ago
|
||
Assignee | ||
Updated•2 years ago
|
Comment 9•2 years ago
|
||
bugherder |
Description
•