1577184 - AddressSanitizer: SEGV /src/obj-firefox/dist/include/mozilla/MozPromise.h:454:24 in mozilla::MozPromise<bool, nsresult, false>::ThenValueBase::Dispatch(mozilla::MozPromise<bool, nsresult, false>*)

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Testcase found while fuzzing mozilla-central 98cbe150f012.

==82546==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7ff38309126e bp 0x7ffd24723f00 sp 0x7ffd24723ec0 T0)
==82546==The signal is caused by a READ memory access.
==82546==Hint: address points to the zero page.
    #0 0x7ff38309126d in mozilla::MozPromise<bool, nsresult, false>::ThenValueBase::Dispatch(mozilla::MozPromise<bool, nsresult, false>*) /src/obj-firefox/dist/include/mozilla/MozPromise.h:454:24
    #1 0x7ff383095f4e in mozilla::MozPromise<bool, nsresult, false>::ThenInternal(already_AddRefed<mozilla::MozPromise<bool, nsresult, false>::ThenValueBase>, char const*) /src/obj-firefox/dist/include/mozilla/MozPromise.h:821:18
    #2 0x7ff38a4ed479 in Track /src/obj-firefox/dist/include/mozilla/MozPromise.h:890:18
    #3 0x7ff38a4ed479 in mozilla::dom::MediaElementAudioSourceNode::ListenForAllowedToPlay(mozilla::dom::MediaElementAudioSourceOptions const&) /src/dom/media/webaudio/MediaElementAudioSourceNode.cpp:89
    #4 0x7ff38a4ecb11 in mozilla::dom::MediaElementAudioSourceNode::Create(mozilla::dom::AudioContext&, mozilla::dom::MediaElementAudioSourceOptions const&, mozilla::ErrorResult&) /src/dom/media/webaudio/MediaElementAudioSourceNode.cpp:67:9
    #5 0x7ff38a463f94 in mozilla::dom::AudioContext::CreateMediaElementSource(mozilla::dom::HTMLMediaElement&, mozilla::ErrorResult&) /src/dom/media/webaudio/AudioContext.cpp:415:10
    #6 0x7ff386940e9b in mozilla::dom::AudioContext_Binding::createMediaElementSource(JSContext*, JS::Handle<JSObject*>, mozilla::dom::AudioContext*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/AudioContextBinding.cpp:629:94
    #7 0x7ff388d5073d in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3163:13
    #8 0x7ff38f946fc7 in CallJSNative /src/js/src/vm/Interpreter.cpp:447:13
    #9 0x7ff38f946fc7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:539
    #10 0x7ff38f92f26c in CallFromStack /src/js/src/vm/Interpreter.cpp:598:10
    #11 0x7ff38f92f26c in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3084
    #12 0x7ff38f91090f in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:424:10
    #13 0x7ff38f947acf in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:567:13
    #14 0x7ff38f949cf2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:610:8
    #15 0x7ff390458528 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2722:10
    #16 0x7ff3885b9280 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventListenerBinding.cpp:52:8
    #17 0x7ff3894de555 in HandleEvent<mozilla::dom::EventTarget *> /src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:66:12
    #18 0x7ff3894de555 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1035
    #19 0x7ff3894dffd0 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /src/dom/events/EventListenerManager.cpp:1233:17
    #20 0x7ff3894c6a4a in HandleEvent /src/obj-firefox/dist/include/mozilla/EventListenerManager.h:353:5
    #21 0x7ff3894c6a4a in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:349
    #22 0x7ff3894c5262 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:551:16
    #23 0x7ff3894cac2b in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:1047:11
    #24 0x7ff3894d1ca0 in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsPresContext*, nsEventStatus*) /src/dom/events/EventDispatcher.cpp
    #25 0x7ff3867ad9aa in nsINode::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&) /src/dom/base/nsINode.cpp:1061:17
    #26 0x7ff38614f1c9 in nsContentUtils::DispatchEvent(mozilla::dom::Document*, nsISupports*, nsTSubstring<char16_t> const&, mozilla::CanBubble, mozilla::Cancelable, mozilla::Composed, mozilla::Trusted, bool*, mozilla::ChromeOnlyDispatch) /src/dom/base/nsContentUtils.cpp:3973:28
    #27 0x7ff38614ef93 in nsContentUtils::DispatchTrustedEvent(mozilla::dom::Document*, nsISupports*, nsTSubstring<char16_t> const&, mozilla::CanBubble, mozilla::Cancelable, mozilla::Composed, bool*) /src/dom/base/nsContentUtils.cpp:3943:10
    #28 0x7ff386496002 in mozilla::dom::Document::DispatchContentLoadedEvents() /src/dom/base/Document.cpp:7060:3
    #29 0x7ff38657d044 in applyImpl<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /src/obj-firefox/dist/include/nsThreadUtils.h:1124:12
    #30 0x7ff38657d044 in apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /src/obj-firefox/dist/include/nsThreadUtils.h:1130
    #31 0x7ff38657d044 in mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run() /src/obj-firefox/dist/include/nsThreadUtils.h:1176
    #32 0x7ff38239c180 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1225:14
    #33 0x7ff3823a21c8 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #34 0x7ff38358fe6f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #35 0x7ff38348ae42 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #36 0x7ff38348ae42 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #37 0x7ff38348ae42 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #38 0x7ff38b797779 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #39 0x7ff38f40b9f0 in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:276:30
    #40 0x7ff38f688b53 in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4573:22
    #41 0x7ff38f68ac7e in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4711:8
    #42 0x7ff38f68cc8e in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4792:21
    #43 0x561ccc9d29d4 in do_main /src/browser/app/nsBrowserApp.cpp:213:22
    #44 0x561ccc9d29d4 in main /src/browser/app/nsBrowserApp.cpp:295
    #45 0x7ff3a3b5db96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #46 0x561ccc8f42ac in _start (/home/worker/builds/m-c-20190825213650-fuzzing-asan-opt/firefox+0x452ac)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /src/obj-firefox/dist/include/mozilla/MozPromise.h:454:24 in mozilla::MozPromise<bool, nsresult, false>::ThenValueBase::Dispatch(mozilla::MozPromise<bool, nsresult, false>*)
==82546==ABORTING

Comment 1

[Jason Kratzer [:jkratzer]]

Testcase bisects to the following range:

Start: cc2984b1f9f28bc2ab198efd1378db80db36f4d6 (20190322101035)
End: e66a2b59914df9ac0ac6bad9887dc8585575bede (20190322164408)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cc2984b1f9f28bc2ab198efd1378db80db36f4d6&tochange=e66a2b59914df9ac0ac6bad9887dc8585575bede

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 3

[Andreas Pehrson [:pehrsons]]

I have a pernosco recording of this at https://pernos.co/debug/ZE7Z6cKWwdZPGzpxzTZlxg/index.html

Comment 4

[Andreas Pehrson [:pehrsons]]

Quite clear what's going on here,

The document.documentElement.innerHTML = '' from the testcase results in DOMEventTargetHelper::DisconnectFromOwner() which resets DOMEventTargetHelper::mParentObject resulting in a null aContext->GetOwnerGlobal() and thus a null mAbstractMainThread.

We need to handle when GetAbstractMainThread() returns nullptr.

This is a regression from bug 1477205 really, but bug 1513733 is also involved as it added the dependency on the main thread here. Bug 1538470 then fixed a more common case, but missed this corner case, even though it touched a bit of code directly involved.

Comment 5

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1577184 - Add crashtest. r?karlt

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1577184 - Guard against a shutting down AudioContext in MediaElementAudioSourceNode. r?karlt

Depends on D44486

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de9dfc1b30b0
Add crashtest. r=karlt
https://hg.mozilla.org/integration/autoland/rev/0d51d763ed32
Guard against a shutting down AudioContext in MediaElementAudioSourceNode. r=karlt

Comment 8

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/de9dfc1b30b0
https://hg.mozilla.org/mozilla-central/rev/0d51d763ed32

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Do you think this is a good candidate for uplift to beta?

Comment 10

[Andreas Pehrson [:pehrsons]]

Comment on attachment 9090072 [details]
Bug 1577184 - Guard against a shutting down AudioContext in MediaElementAudioSourceNode. r?karlt

Beta/Release Uplift Approval Request

• User impact if declined: Nullptr-deref crash that can be triggered at will when known.
• Is this code covered by automated tests?: Yes
• 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): Trivial change
• String changes made/needed:

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9090072 [details]
Bug 1577184 - Guard against a shutting down AudioContext in MediaElementAudioSourceNode. r?karlt

Fix for potential crash/corner case, OK for uplift for beta 6.

Comment 12

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5cb53f0ec731

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9090071 [details]
Bug 1577184 - Add crashtest. r?karlt

Let's uplift the test too.

Comment 14

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/401ccc238dc6