1849265 - Compartment mismatch in DebuggerImmediateRunnable::WorkerRun()

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect, P2)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/77a92730-e35e-4f11-a477-c70cb0230817

MOZ_CRASH Reason: *** Compartment mismatch 7f2b5f088790 vs. 7f2b69e184c0 at argument 1

Top 10 frames of crashing thread:

0  libxul.so  MOZ_Crash  mfbt/Assertions.h:281
0  libxul.so  js::ContextChecks::fail  js/src/vm/JSContext-inl.h:54
0  libxul.so  js::ContextChecks::check  js/src/vm/JSContext-inl.h:70
0  libxul.so  js::ContextChecks::check  js/src/vm/JSContext-inl.h:83
0  libxul.so  js::ContextChecks::check  js/src/vm/JSContext-inl.h:124
1  libxul.so  JSContext::checkImpl<JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray>  js/src/vm/JSContext-inl.h:206
1  libxul.so  JSContext::check<JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray>  js/src/vm/JSContext-inl.h:213
1  libxul.so  JS_CallFunctionValue  js/src/vm/CallAndConstruct.cpp:47
2  libxul.so  mozilla::dom::(anonymous namespace)::DebuggerImmediateRunnable::WorkerRun(JSContext*, mozilla::dom::WorkerPrivate*)  dom/workers/WorkerPrivate.cpp:665
3  libxul.so  mozilla::dom::WorkerRunnable::Run  dom/workers/WorkerRunnable.cpp:372

I found this crash on crash-stats. I'm marking it sec-moderate because it seems to require the debugger.

Comment 1

[Jens Stutte [:jstutte]]

Hi Eden, would you mind to take a look and triage? Thanks!

Comment 2

[Jens Stutte [:jstutte]]

IIUC those checks are active only in nightly. Do we know what the consequences are if this happens in release builds?

Comment 3

[Jens Stutte [:jstutte]]

FWIW I checked on crash-stats, it seems there are ten out of 52 reports that do not have DebuggerImmediateRunnable in the proto signature and crash on the main thread, not the worker one. Not sure if those would be equally concerning, at least they do not seem to have a clear pattern like the debugger one here.

Comment 4

[Andrew McCreight [:mccr8]]

(In reply to Jens Stutte [:jstutte] from comment #2)

IIUC those checks are active only in nightly. Do we know what the consequences are if this happens in release builds?

It can potentially cause a use-after-free crash, because it indicates that a cross compartment reference is not properly going through a cross compartment wrapper, so the GC might fail to trace a compartment.

Comment 5

[Eden Chuang[:edenchuang]]

Attachment: Bug 1849265 - P1 Synchronize WorkerPrivate::mParentStatus with mStatus while entering Killing status. r=#dom-worker-reviewers

Comment 6

[Eden Chuang[:edenchuang]]

https://treeherder.mozilla.org/logviewer?job_id=433125137&repo=try&lineNumber=110685. A similar issue is on WorkerGlobalScope.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a6678776d36
P1 Synchronize WorkerPrivate::mParentStatus with mStatus while entering Killing status. r=asuth

Comment 8

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

Backed out for causing asan failures on browser_localStorage_snapshotting.js:

https://hg.mozilla.org/integration/autoland/rev/aa1add343b69110ec5bba9727bbd680112e68e4f

Push with failures
Failure log

==14232==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x55add29ad952 bp 0x7fff97974190 sp 0x7fff97974170 T0)
==14232==ERROR: AddressSanitizer: out of memory: failed to allocate 0x1000 (4096) bytes of ReadFileToBuffer (error code: 12)
==14306==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x556a5566f952 bp 0x7fff73c71350 sp 0x7fff73c71330 T0)
==14230==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x555f6aae4952 bp 0x7ffff34f3310 sp 0x7ffff34f32f0 T0)
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:281:3 in MOZ_Crash
TEST-UNEXPECTED-FAIL | dom/tests/browser/browser_localStorage_snapshotting.js | Test timed out -
...

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99883f337579
P1 Synchronize WorkerPrivate::mParentStatus with mStatus while entering Killing status. r=asuth

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/99883f337579

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta and ESR115 approval.

Comment 12

[Eden Chuang[:edenchuang]]

Comment on attachment 9359777 [details]
Bug 1849265 - P1 Synchronize WorkerPrivate::mParentStatus with mStatus while entering Killing status. r=#dom-worker-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Users could meet the content process crash or unexpected behavior when the website uses Worker/ServiceWorker/SharedWorker, especially when Worker/ServiceWorker/SharedWorker is shutting down.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch synchronizes the WorkerPrivate::mParentStatus with WorkerPrivate::mStatus when the Worker enters the "Killing" status.
  I think it is a low risk. Although the patch changes the runnable dispatching logic, it makes the runnable dispatching do the right thing.
• String changes made/needed: No
• Is Android affected?: Yes

Comment 13

[Eden Chuang[:edenchuang]]

I think we don't need to uplift this patch to ESR since this bug seems to be a regression of bug 1800659, and 1800659 was fixed on 116, but was not uplifted to ESR115.

Comment 14

[Pascal Chevrel:pascalc]

Comment on attachment 9359777 [details]
Bug 1849265 - P1 Synchronize WorkerPrivate::mParentStatus with mStatus while entering Killing status. r=#dom-worker-reviewers

Approved for our last 120 beta, thanks.

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/c9cafe134648

Comment 16

[Daniel Veditz [:dveditz]]

Bulk-unhiding security bugs fixed in Firefox 119-121 (Fall 2023). Use "moo-doctrine-subsidy" to filter