1793407 - Crash in [@ mozilla::dom::WorkerLoadContext::IsTopLevel] called from CacheLoadHandler

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/bb463ef3-ad8b-4258-8143-619240221002

Reason: EXC_BAD_ACCESS / EXC_I386_GPFLT

Top 10 frames of crashing thread:

0 XUL mozilla::dom::WorkerLoadContext::IsTopLevel dom/workers/loader/WorkerLoadContext.h:87
0 XUL mozilla::dom::workerinternals::loader::CacheLoadHandler::ResolvedCallback dom/workers/loader/CacheLoadHandler.cpp:393
1 XUL mozilla::dom:: dom/promise/Promise.cpp:433
2 XUL mozilla::dom::NativeHandlerCallback dom/promise/Promise.cpp
3 XUL CallJSNative js/src/vm/Interpreter.cpp:459
3 XUL js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:547
3 XUL InternalCall js/src/vm/Interpreter.cpp:614
3 XUL js::Call js/src/vm/Interpreter.cpp:646
4 XUL js::Call js/src/vm/Interpreter.h:116
4 XUL PromiseReactionJob js/src/builtin/Promise.cpp:2240

This appears to be a use-after-free crash and it's been introduced recently. The first affected buildid is 20220921095211.

Comment 1

[Andrew McCreight [:mccr8]]

Why do you say that it looks like a UAF? The crash you linked looks like it is crashing on null.

Comment 2

[Gabriele Svelto [:gsvelto]]

Yes, it's not obvious from that one (the crash address is rubbish because of bug 1493342, which will get fixed in the coming weeks) but in this one you can see the poison pattern in the rdi register and the crash is happening on that.

Comment 3

[Andrew McCreight [:mccr8]]

Thanks for the explanation.

Yulia, is this similar to one of the UAFs you have on your radar? Thanks.

Comment 4

[Yulia Startsev [:yulia]]

It looks like, yes. If my next attempt doesn't fix this, I'll revert the work I did on cancellation behavior, and that should fix this.

Comment 5

[Jens Stutte [:jstutte]]

Yulia, assigning this to you for monitoring.

Comment 6

[Jens Stutte [:jstutte]]

Do we suspect this to be always related to bug 1792984 ?

Comment 7

[Yulia Startsev [:yulia]]

This appears to be largely resolved by 1792984 -- but there are still cases where this can happen because there isn't a guarantee that the load handlers live shorter lives than the script loader. I had been thinking about this separately in https://bugzilla.mozilla.org/show_bug.cgi?id=1797327

Comment 8

[Jens Stutte [:jstutte]]

Now that bug 1797327 is fixed, can we end monitoring here?

Comment 9

[Yulia Startsev [:yulia]]

This should be resolved, and we haven't seen crashes in the last few versions

Comment 10

[Yulia Startsev [:yulia]]

Opps, didn't mean to close this one.

Comment 11

[Daniel Veditz [:dveditz]]

There were a couple of crashes in 108 nightly, but both were before recent fixes landed and were uplifted to 107 Beta. The last beta with any crashes is 107 beta-6, also before patches landed (we've shipped betas 7, 8, and 9 since then).

There are three crashes in 106.1 release, but the stacks look different from the ones with a UAF signature (they don't have a bunch of JS on the stack). Is this specific crash a regression in 107 that never affected a Release build?

Comment 12

[Jens Stutte [:jstutte]]

Yulia, do we know what regressed this specific bug?

The three crashes in release indeed look differently, this seems to be just another case of mRequest being nullptr unexpectedly, similar to bug 1798610. Not sure if bug 1798667 is supposed to have this solved, too.

Comment 13

[Yulia Startsev [:yulia]]

This was due to the worker load context becoming null, which isn't possible now as we hold a refptr to it instead of a raw pointer. I am not sure exactly what triggered this behavior but it is likely related to the introduction to the ScriptLoadRequestList. This is resolved by https://bugzilla.mozilla.org/show_bug.cgi?id=1798667