Closed Bug 1770266 Opened 1 month ago Closed 1 month ago

Assertion failure: crossZoneRecords.has(object), at gc/FinalizationObservers.cpp:444


(Core :: JavaScript: GC, defect, P1)




102 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox100 --- wontfix
firefox101 --- wontfix
firefox102 --- fixed


(Reporter: gkw, Assigned: jonco)



(Keywords: regression, sec-other, testcase, Whiteboard: [adv-main102-])


(1 file)

Debugger.each = function (...y) {
  for (let x of y) {
    new FinalizationRegistry(function () {}).register(x);
Thread 1 "js-dbg-64-linux" received signal SIGSEGV, Segmentation fault.
js::gc::FinalizationObservers::checkTables (this=<optimized out>) at /home/skygentoo/trees/mozilla-central/js/src/gc/FinalizationObservers.cpp:444
444	        MOZ_ASSERT(crossZoneRecords.has(object));
(gdb) bt
#0  js::gc::FinalizationObservers::checkTables (this=<optimized out>) at /home/skygentoo/trees/mozilla-central/js/src/gc/FinalizationObservers.cpp:444
#1  0x00005555574e70e6 in js::gc::FinalizationObservers::clearRecords (this=0x7ffff69c35f0) at /home/skygentoo/trees/mozilla-central/js/src/gc/FinalizationObservers.cpp:167
#2  0x00005555575d73d6 in JS::Zone::clearRootsForShutdownGC (this=0x7ffff6a4a400) at /home/skygentoo/trees/mozilla-central/js/src/gc/Zone.cpp:920
#3  0x00005555574fb508 in js::gc::GCRuntime::endPreparePhase (this=<optimized out>, this@entry=0x7ffff6a18768, reason=reason@entry=JS::GCReason::DESTROY_RUNTIME) at /home/skygentoo/trees/mozilla-central/js/src/gc/GC.cpp:2620
#4  0x00005555574ff6e1 in js::gc::GCRuntime::incrementalSlice (this=this@entry=0x7ffff6a18768, budget=..., reason=reason@entry=JS::GCReason::DESTROY_RUNTIME, budgetWasIncreased=false) at /home/skygentoo/trees/mozilla-central/js/src/gc/GC.cpp:3180
#5  0x0000555557502e38 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff6a18768, nonincrementalByAPI=true, budgetArg=..., reason=reason@entry=JS::GCReason::DESTROY_RUNTIME) at /home/skygentoo/trees/mozilla-central/js/src/gc/GC.cpp:3732
#6  0x0000555557504076 in js::gc::GCRuntime::collect (this=this@entry=0x7ffff6a18768, nonincrementalByAPI=false, budget=..., reason=reason@entry=JS::GCReason::DESTROY_RUNTIME) at /home/skygentoo/trees/mozilla-central/js/src/gc/GC.cpp:3917
#7  0x00005555574dabb8 in js::gc::GCRuntime::gc (this=0x7ffff6a18768, options=JS::GCOptions::Shutdown, reason=JS::GCReason::DESTROY_RUNTIME) at /home/skygentoo/trees/mozilla-central/js/src/gc/GC.cpp:3994
#8  0x0000555556fc04dc in JSRuntime::destroyRuntime (this=0x7ffff6a18000) at /home/skygentoo/trees/mozilla-central/js/src/vm/Runtime.cpp:284
#9  0x0000555556e78306 in js::DestroyContext (cx=cx@entry=0x7ffff6a2a200) at /home/skygentoo/trees/mozilla-central/js/src/vm/JSContext.cpp:238
#10 0x00005555571eedba in JS_DestroyContext (cx=0x7ffff7c8ba60 <_IO_stdfile_2_lock>, cx@entry=0x7ffff6a2a200) at /home/skygentoo/trees/mozilla-central/js/src/jsapi.cpp:396
#11 0x0000555556b52d67 in main::$_3::operator() (this=<optimized out>) at /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:12549
#12 mozilla::ScopeExit<main::$_3>::~ScopeExit (this=<optimized out>) at /home/skygentoo/shell-cache/js-dbg-64-linux-x86_64-c1ef18d0a27e/objdir-js/dist/include/mozilla/ScopeExit.h:106
#13 main (argc=<optimized out>, argv=<optimized out>) at /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:12753

Run with --fuzzing-safe --no-threads --no-baseline --no-ion --more-compartments testcase.js, compile with AR=ar sh ./configure --enable-debug --with-ccache --enable-gczeal --enable-debug-symbols --disable-bootstrap --disable-tests, tested on m-c rev c1ef18d0a27e.

The first bad revision is:
user:        Jon Coppeard
date:        Wed Feb 23 10:01:11 2022 +0000
summary:     Bug 1756567 - Part 3: Check cross zone wrapper set was correct before clearing r=sfink

Not sure if this is s-s, I'd leave it to Jon. Note that regressor seems to be bug 1756567 which is s-s (and which I am prevented from adding to the Regressed by field due to lack of permissions), and one of the checkins landed a testcase from bug 1756590 which is also s-s.

Flags: sec-bounty?
Flags: needinfo?(jcoppeard)

Set release status flags based on info from the regressing bug 1756567

Group: core-security → javascript-core-security
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)

Something I didn't realise is that remapping a wrapper can make the wrapper
point at the original target again after nuking it. The code assumed that nuked
wrappers stayed dead.

Wrappers to finalization record objects (not registries) are not exposed so I
think we can skip recomputing wrappers to these.

Our assertions were a little over-eager here too. We shouldn't expect to see an
entry in the cross-zone records table for records that have their 'in map' flag
cleard, even if the entry hasn't been removed by sweeping yet.

Jon, can you set the sec- keyword as well as the severity field (next to the priority) of Bugzilla?

Blocks: 1755725
Priority: -- → P1

It sounded like maybe this isn't a sec issue but just a bogus assert and so it could be marked sec-other or unhidden?

Group: javascript-core-security
Keywords: sec-other
Pushed by
Don't recompute wrappers to finalization registry records r=sfink
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
Flags: sec-bounty? → sec-bounty-
Comment 1 is private: false
You need to log in before you can comment on or make changes to this bug.