1799229 - Crash in [@ JS::loader::ModuleLoaderBase::DisallowImportMaps]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/f479ef91-0c35-4a28-902b-e918f0221103

Reason: EXCEPTION_ACCESS_VIOLATION_WRITE

Top 10 frames of crashing thread:

0  xul.dll  JS::loader::ModuleLoaderBase::DisallowImportMaps  js/loader/ModuleLoaderBase.h:294
0  xul.dll  nsContentSink::ProcessLinkFromHeader  dom/base/nsContentSink.cpp:334
1  xul.dll  nsContentSink::DoProcessLinkHeader  dom/base/nsContentSink.cpp:255
2  xul.dll  mozilla::detail::RunnableMethodArguments<>::applyImpl  xpcom/threads/nsThreadUtils.h:1147
2  xul.dll  mozilla::detail::RunnableMethodArguments<>::apply  xpcom/threads/nsThreadUtils.h:1153
2  xul.dll  mozilla::detail::RunnableMethodImpl<nsMemoryReporterManager*, nsresult   xpcom/threads/nsThreadUtils.h:1200
3  xul.dll  mozilla::RunnableTask::Run  xpcom/threads/TaskController.cpp:538
3  xul.dll  mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal  xpcom/threads/TaskController.cpp:851
4  xul.dll  mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal  xpcom/threads/TaskController.cpp:683
4  xul.dll  mozilla::TaskController::ProcessPendingMTTask  xpcom/threads/TaskController.cpp:461

Null deref crash. The line we're crashing on in nsContentSink::ProcessLinkFromHeader() is this: mDocument->ScriptLoader()->GetModuleLoader()->DisallowImportMaps();

I'm guessing that something in that long chain of dereferences is null, so some null checks might be in order. This is happening off a runnable, so maybe something went away in the meanwhile.

The first instance I see of this is on build 20221010214639, but maybe the signature just changed around then.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Andrew McCreight [:mccr8] from comment #0)

Crash report: https://crash-stats.mozilla.org/report/index/f479ef91-0c35-4a28-902b-e918f0221103
0 xul.dll JS::loader::ModuleLoaderBase::DisallowImportMaps js/loader/ModuleLoaderBase.h:294
0 xul.dll nsContentSink::ProcessLinkFromHeader dom/base/nsContentSink.cpp:334
1 xul.dll nsContentSink::DoProcessLinkHeader dom/base/nsContentSink.cpp:255
2 xul.dll mozilla::detail::RunnableMethodArguments<>::applyImpl xpcom/threads/nsThreadUtils.h:1147
2 xul.dll mozilla::detail::RunnableMethodArguments<>::apply xpcom/threads/nsThreadUtils.h:1153
2 xul.dll mozilla::detail::RunnableMethodImpl<nsMemoryReporterManager*, nsresult xpcom/threads/nsThreadUtils.h:1200

I think this Runnable comes from [nsMemoryReporterManager::GetReportsExtends]
(https://searchfox.org/mozilla-central/rev/0062309958c212504556ae21a7e3eb120c9bd093/xpcom/base/nsMemoryReporterManager.cpp#1803)

However, I am not sure how it triggers nsContentSink::ProcessLinkFromHeader
and why the script global is not set (so ModuleLoader will be null) when DoProcessLinkHeader is called.

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Bug 1799229 : Check if ModuleLoader exists before calling DisallowImportMaps().

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 9303493 [details]
Bug 1799229 : Check if ModuleLoader exists before calling DisallowImportMaps().

Beta/Release Uplift Approval Request

• User impact if declined: crash by null pointer dereferencing
• 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): It just checks if the pointer is valid before dereferencing it.
• String changes made/needed: no
• Is Android affected?: Yes

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6dc6fdc72146
Check if ModuleLoader exists before calling DisallowImportMaps(). r=smaug

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/6dc6fdc72146

Comment 6

[Dianna Smith [:diannaS]]

Comment on attachment 9303493 [details]
Bug 1799229 : Check if ModuleLoader exists before calling DisallowImportMaps().

Approved for 108.0b3

Comment 7

[Dianna Smith [:diannaS]]

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