1988128 - Assertion failure: (value & RESERVED_MASK) == 0, at /js/src/gc/Cell.h:109 with Debugger and wasm

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20250911-e21990811137 (debug build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off --more-compartments):

function wasmRunWithDebugger(wast, lib, init, done) {
    let g = newGlobal({newCompartment: true});
    let dbg = new Debugger(g);
    g.eval(`
      var wasm = wasmTextToBinary(\`${wast}\`);
      var lib = ${lib || 'undefined'};
      var m = new WebAssembly.Instance(new WebAssembly.Module(wasm), lib)
    `);
    var wasmScript = dbg.findScripts().filter(s => s.format == 'wasm')[0];
    init({dbg, wasmScript, g,});
    result = g.eval("m.exports.test()");
}
let WasmStructrefValues = [];
let WasmArrayrefValues = [];
let WasmEqrefValues = [	...WasmStructrefValues, ...WasmArrayrefValues];
gczeal(6, 7);
wasmRunWithDebugger(`
 (module (memory 1 1)
 (func (param i32) (local f64) nop) 
 (export "test" (func 0))
 (data (i32.const 0) "Abcx2A"))
`, undefined,
function ({dbg}) {
  dbg.onEnterFrame = function (frame) {
    if (frame.type != 'wasmcall') return;
    var memoryContent = frame.eval('new DataView(memory0.buffer).getUint8(3)').return;
    frame.onStep = function () {
          assertEq(frame.offset, 65);
    };
  };
}, function ({error}) {} 
);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x00005555579e74ef in void js::GCMarker::checkTraversedEdge<js::PropMap*, JSString>(js::PropMap*, JSString*) ()
#1  0x00005555579e6944 in void js::GCMarker::eagerlyMarkChildren<0u>(js::Scope*) ()
#2  0x00005555579ee6a3 in bool js::GCMarker::markAndTraversePrivateGCThing<0u>(JSObject*, js::gc::Cell*) ()
#3  0x00005555579ed1c8 in bool js::GCMarker::processMarkStackTop<0u>(JS::SliceBudget&) ()
#4  0x00005555579ec442 in bool js::GCMarker::markOneColor<0u, (js::gc::MarkColor)2>(JS::SliceBudget&) ()
#5  0x00005555579cfa34 in bool js::GCMarker::doMarking<0u>(JS::SliceBudget&, js::gc::ShouldReportMarkTime) ()
#6  0x00005555579a78b3 in js::GCMarker::markUntilBudgetExhausted(JS::SliceBudget&, js::gc::ShouldReportMarkTime) ()
#7  0x00005555579a6b6c in js::gc::GCRuntime::markUntilBudgetExhausted(JS::SliceBudget&, js::gc::GCRuntime::ParallelMarking, js::gc::ShouldReportMarkTime) ()
#8  0x00005555579abe7c in js::gc::GCRuntime::incrementalSlice(JS::SliceBudget&, JS::GCReason, bool) ()
#9  0x00005555579afbc3 in js::gc::GCRuntime::gcCycle(bool, JS::SliceBudget const&, JS::GCReason) ()
#10 0x00005555579b14dd in js::gc::GCRuntime::collect(bool, JS::SliceBudget const&, JS::GCReason) ()
#11 0x00005555579b4772 in js::gc::GCRuntime::runDebugGC() ()
#12 0x0000555557951621 in void* js::gc::CellAllocator::AllocTenuredCell<(js::AllowGC)1>(JSContext*, js::gc::AllocKind) ()
#13 0x00005555573dd74d in js::EvalScope* js::Scope::create<js::EvalScope>(JSContext*, js::ScopeKind, JS::Handle<js::Scope*>, JS::Handle<js::SharedShape*>, js::HandleBuffer<js::EvalScope::RuntimeData>) ()
#14 0x00005555573dceb7 in js::Scope* js::frontend::ScopeStencil::createSpecificScope<js::EvalScope, js::VarEnvironmentObject>(JSContext*, js::frontend::CompilationAtomCache&, JS::Handle<js::Scope*>, js::AbstractBaseScopeData<js::frontend::TaggedParserAtomIndex>*) const ()
#15 0x00005555578e21d6 in js::frontend::ScopeStencil::createScope(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationGCOutput&, js::AbstractBaseScopeData<js::frontend::TaggedParserAtomIndex>*) const ()
#16 0x00005555578e4a8a in js::frontend::CompilationStencil::instantiateStencilAfterPreparation(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&) ()
#17 0x00005555578e3d5c in js::frontend::CompilationStencil::instantiateStencils(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&) ()
#18 0x000055555784a2f9 in bool InstantiateStencilsImpl<js::frontend::CompilationStencil const>(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&) ()
#19 0x000055555784c64f in js::frontend::CompileEvalScript(JSContext*, JS::ReadOnlyCompileOptions const&, JS::SourceText<char16_t>&, JS::Handle<js::Scope*>, JS::Handle<JSObject*>) ()
#20 0x0000555557755e21 in js::DebuggerGenericEval(JSContext*, mozilla::Range<char16_t const>, JS::Handle<JSObject*>, js::EvalOptions const&, js::Debugger*, JS::Handle<JSObject*>, js::FrameIter*) ()
#21 0x0000555557756e0b in js::DebuggerFrame::eval(JSContext*, JS::Handle<js::DebuggerFrame*>, mozilla::Range<char16_t const>, JS::Handle<JSObject*>, js::EvalOptions const&) ()
#22 0x000055555775a231 in js::DebuggerFrame::CallData::evalMethod() ()
#23 0x000055555775d6b3 in bool js::DebuggerFrame::CallData::ToNative<&js::DebuggerFrame::CallData::evalMethod>(JSContext*, unsigned int, JS::Value*) ()
#24 0x000055555704ed45 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
#25 0x000055555704e450 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) ()
#26 0x00005555570622c9 in js::Interpret(JSContext*, js::RunState&) ()
#27 0x000055555704d9c4 in js::RunScript(JSContext*, js::RunState&) ()
#28 0x000055555704e476 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) ()
#29 0x000055555704f9ed in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) ()
#30 0x00005555576bef36 in js::Call(JSContext*, JS::Handle<JS::Value>, JSObject*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) ()
#31 0x000055555770c25b in js::Debugger::fireEnterFrame(JSContext*, js::ResumeMode&, JS::MutableHandle<JS::Value>) ()
#32 0x00005555577016ef in js::DebugAPI::slowPathOnEnterFrame(JSContext*, js::AbstractFramePtr) ()
#33 0x0000555557067e85 in js::DebugAPI::onEnterFrame(JSContext*, js::AbstractFramePtr) ()
#34 0x00005555582c3f5a in WasmHandleDebugTrap() ()
#35 0x0000231d217b124b in ?? ()
#36 0x00007fffffffac98 in ?? ()
#37 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x0	0
rcx	0x6d	109
rdx	0x7ffff7804563	140737345766755
rsi	0x0	0
rdi	0x7ffff7805700	140737345771264
rbp	0x7fffffff7750	140737488320336
rsp	0x7fffffff7730	140737488320304
r8	0x0	0
r9	0x3	3
r10	0x0	0
r11	0x293	659
r12	0x2	2
r13	0x90455aeb060	9914222030944
r14	0x2	2
r15	0x90455aeb060	9914222030944
rip	0x5555579e74ef <void js::GCMarker::checkTraversedEdge<js::PropMap*, JSString>(js::PropMap*, JSString*)+383>
=> 0x5555579e74ef <_ZN2js8GCMarker18checkTraversedEdgeIPNS_7PropMapE8JSStringEEvT_PT0_+383>:	mov    %rcx,(%rax)
   0x5555579e74f2 <_ZN2js8GCMarker18checkTraversedEdgeIPNS_7PropMapE8JSStringEEvT_PT0_+386>:	call   0x555556f67470 <abort>

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Unable to reproduce bug 1988128 using build mozilla-central 20250911040839-e21990811137. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 4

[Iain Ireland [:iain]]

Took a quick look at this.

In WasmFunctionScope::create, we allocate scope data for a WasmFunction Scope, put two strings into it, and then create a scope using that data. In js::Scope::create<js::WasmFunctionScope>, we allocate a scope, then store the data into it. Allocating the scope triggers a GC, which frees the two strings, so the data is already bogus by the time we store it in the new scope.

This doesn't seem like it should happen, because we root the RuntimeData, but for whatever reason (missing barriers?) the strings are never marked. This code was all just rewritten in bug 1987111, so it seems likely that's the regressor.

Jon, can you take a look?

Comment 5

[Jon Coppeard (:jonco)]

Attachment: (secure)

Comment 6

[Jon Coppeard (:jonco)]

Comment on attachment 9513748 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Very difficult. It would require triggering OOM at exactly the right time and the patch doesn't touch anything that looks OOM related.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: beta only
• If not all supported branches, which bug introduced the flaw?: Bug 1987111
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: The same fix should apply
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely. It's a simple change that adds tracing at one particular point.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 7

[BugBot [:suhaib / :marco]]

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:jonco, could you consider increasing the severity of this security bug?

For more information, please visit BugBot documentation.

Comment 8

[BugBot [:suhaib / :marco]]

The bug is marked as tracked for firefox144 (beta) and tracked for firefox145 (nightly). However, the bug still has low severity.

:sdetar, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 9513748 [details]
(secure)

sec-approval=dveditz. Since this doesn't affect release it's fine to land the testcase with it.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ede6a5224c33
https://hg.mozilla.org/integration/autoland/rev/f0260861c207
Trace the contents of rooted buffers as well as the buffer itself r=sfink

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/f0260861c207

Comment 12

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:jonco, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox144 to wontfix.

For more information, please visit BugBot documentation.

Comment 13

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Possible crash / security vulnerability.
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: This is a simple change that adds some missing tracing. It's been on nightly for 4 days without issue.
• String changes made/needed: None
• Is Android affected?: yes

Comment 14

[Jon Coppeard (:jonco)]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D265094

Comment 15

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/dc580613b16d
https://hg.mozilla.org/releases/mozilla-beta/rev/40ecdc97d970