Closed Bug 1547784 Opened 11 months ago Closed 8 months ago

AddressSanitizer: SEGV /builds/worker/workspace/build/src/dom/html/HTMLMediaElement.cpp:4584:15 in mozilla::dom::HTMLMediaElement::FinishDecoderSetup(mozilla::MediaDecoder*)

Categories

(Core :: Audio/Video, defect, P2, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: jkratzer, Assigned: bryce)

References

(Blocks 2 open bugs)

Details

(Keywords: crash, testcase)

Attachments

(3 files)

Attached file testcase.html

Testcase found while fuzzing mozilla-central rev 420e18a75314.

==20675==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fcce90f8953 bp 0x7ffef95142f0 sp 0x7ffef95141e0 T0)
==20675==The signal is caused by a READ memory access.
==20675==Hint: address points to the zero page.
#0 0x7fcce90f8952 in mozilla::dom::HTMLMediaElement::FinishDecoderSetup(mozilla::MediaDecoder*) /builds/worker/workspace/build/src/dom/html/HTMLMediaElement.cpp:4584:15
#1 0x7fcce90f09ea in mozilla::dom::HTMLMediaElement::LoadResource() /builds/worker/workspace/build/src/dom/html/HTMLMediaElement.cpp:2578:10
#2 0x7fcce90ed42d in mozilla::dom::HTMLMediaElement::SelectResource() /builds/worker/workspace/build/src/dom/html/HTMLMediaElement.cpp:2112:12
#3 0x7fcce90ea9b9 in mozilla::dom::HTMLMediaElement::SelectResourceWrapper() /builds/worker/workspace/build/src/dom/html/HTMLMediaElement.cpp:2055:3
#4 0x7fcce917822b in applyImpl<mozilla::dom::HTMLMediaElement, void (mozilla::dom::HTMLMediaElement::)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1122:12
#5 0x7fcce917822b in apply<mozilla::dom::HTMLMediaElement, void (mozilla::dom::HTMLMediaElement::
)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1128
#6 0x7fcce917822b in mozilla::detail::RunnableMethodImpl<mozilla::dom::HTMLMediaElement*, void (mozilla::dom::HTMLMediaElement::)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1174
#7 0x7fcce915eebf in mozilla::dom::nsSyncSection::Run() /builds/worker/workspace/build/src/dom/html/HTMLMediaElement.cpp:1916:16
#8 0x7fcce08910b7 in mozilla::CycleCollectedJSContext::ProcessStableStateQueue() /builds/worker/workspace/build/src/xpcom/base/CycleCollectedJSContext.cpp:382:12
#9 0x7fcce0894172 in mozilla::CycleCollectedJSContext::AfterProcessTask(unsigned int) /builds/worker/workspace/build/src/xpcom/base/CycleCollectedJSContext.cpp:441:3
#10 0x7fcce30157e5 in XPCJSContext::AfterProcessTask(unsigned int) /builds/worker/workspace/build/src/js/xpconnect/src/XPCJSContext.cpp:1273:28
#11 0x7fcce0b28296 in nsThread::ProcessNextEvent(bool, bool
) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1240:24
#12 0x7fcce0b2ef64 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
#13 0x7fcce1e9507f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:88:21
#14 0x7fcce1d6ddfe in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
#15 0x7fcce1d6ddfe in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
#16 0x7fcce1d6ddfe in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
#17 0x7fcceb432ad3 in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:137:27
#18 0x7fccefa3dd7e in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:919:20
#19 0x7fcce1d6ddfe in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
#20 0x7fcce1d6ddfe in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
#21 0x7fcce1d6ddfe in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
#22 0x7fccefa3ceec in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:757:34
#23 0x5589adad172e in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#24 0x5589adad172e in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:263
#25 0x7fcd04c5fb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

Flags: in-testsuite?

alwu:

Looks like you're active maintainer of HTMLMediaElement, could you take a look or redirect to another person?

Flags: needinfo?(alwu)
Priority: -- → P2

Sure, will take a look later, keep NI.

Component: DOM: Core & HTML → Audio/Video

This crash happened in [1], because we had released the decoder in previous line [2] as the document was not an active document [3]. The document which was used to create video was not a current document of the doc shell, so we detroied the decoder.

From the codebase, it seems like we don't want to keep media key alive when document becomes inactive [4], but what would happen if we set media key for a video element which is create from an inactive document?

[1] https://searchfox.org/mozilla-central/rev/b756e6d00728dda4121f8278a744381d8643317a/dom/html/HTMLMediaElement.cpp#4584
[2] https://searchfox.org/mozilla-central/rev/b756e6d00728dda4121f8278a744381d8643317a/dom/html/HTMLMediaElement.cpp#4581
[3] https://searchfox.org/mozilla-central/rev/b756e6d00728dda4121f8278a744381d8643317a/dom/html/HTMLMediaElement.cpp#6042-6050
[4] https://searchfox.org/mozilla-central/rev/b756e6d00728dda4121f8278a744381d8643317a/dom/html/HTMLMediaElement.cpp#6046


Hi, Bryce,
Because I'm not clear what the correct behavior is for the media key, do you mind have a look for this bug?
Thank you.

Flags: needinfo?(alwu) → needinfo?(bvandyk)

This fixes an edge case where it was possible for an HTMLMediaElement in the
middle of setup to have ownership transferred to a inactive document and deref a
null pointer. This happened because we have special handling for EME related
media where we perform more aggressive shutdown on for media in inactive
documents.

As far as I can tell, there's nothing specced that forbids performing EME
related functionality on elements in inactive documents. However, our code
already prevents doing so in other cases. E.g. if you create an inactive
document, place an HTMLMediaElement in it and try to setup EME related data on
it, then that will fail. So this fix just covers another such case.

While it would be nice to support doing these operations on inactive document's
media, it seems like very much an edge case, and something that would require a
large amount of reworking in how we handle inactive documents. We can cross that
bridge later should we ever need do so.

Depends on D40481

Holding off on landing this as I'm seeing another crash on the testcase and want to identify what's going on there.

Edit: think the other crash was down to me accidentally removing the reftest-wait from the test and getting into some bizzaro race. As such, I think this can go ahead.

Flags: needinfo?(bvandyk)
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/54b3b878f07d
Add crashtest for moving media element with media keys to inactive doc. r=alwu
https://hg.mozilla.org/integration/autoland/rev/21f0da4d5763
Return an error if an EME associated MediaElement becomes inactive as deocder setup finishes. r=alwu
Backout by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2fcf4972c072
Backed out 2 changesets for crashtest failrues on 1547784.html . CLOSED TREE

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

Keywords: regression

Not a regression to the best of my knowledge. Getting ready to reland.

Keywords: regression
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3b56af8ee88e
Add crashtest for moving media element with media keys to inactive doc. r=alwu
https://hg.mozilla.org/integration/autoland/rev/2a7a1901557e
Return an error if an EME associated MediaElement becomes inactive as deocder setup finishes. r=alwu
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Assignee: nobody → bvandyk

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.