1560215 - AddressSanitizer: SEGV /builds/worker/workspace/build/src/dom/media/encoder/MediaEncoder.cpp:466:11 in operator()

Status: RESOLVED FIXED

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

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Testcase found while fuzzing mozilla-central rev 19cf79b6f07d.

==15760==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7f131057bfa9 bp 0x7f12f2b24ad0 sp 0x7f12f2b24a90 T38)
==15760==The signal is caused by a WRITE memory access.
==15760==Hint: address points to the zero page.
    #0 0x7f131057bfa8 in operator() /builds/worker/workspace/build/src/dom/media/encoder/MediaEncoder.cpp:466:11
    #1 0x7f131057bfa8 in mozilla::detail::RunnableFunction<mozilla::MediaEncoder::Suspend()::$_0>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:564
    #2 0x7f131042289a in mozilla::MediaStreamGraphImpl::RunMessagesInQueue() /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:1157:20
    #3 0x7f1310427578 in mozilla::MediaStreamGraphImpl::OneIterationImpl(long) /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:1384:3
    #4 0x7f13100e144a in mozilla::ThreadedDriver::RunThread() /builds/worker/workspace/build/src/dom/media/GraphDriver.cpp:311:41
    #5 0x7f13100f598b in mozilla::MediaStreamGraphInitThreadRunnable::Run() /builds/worker/workspace/build/src/dom/media/GraphDriver.cpp:208:14
    #6 0x7f1307765b33 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1215:14
    #7 0x7f130776d8f4 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #8 0x7f1308b782e1 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:303:20
    #9 0x7f1308a4dace in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #10 0x7f1308a4dace in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #11 0x7f1308a4dace in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #12 0x7f130775d633 in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:459:11
    #13 0x7f132d9540bd in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #14 0x7f132d5986da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    #15 0x7f132c57688e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/dom/media/encoder/MediaEncoder.cpp:466:11 in operator()
Thread T38 (MediaStreamGrph) created by T0 (file:// Content) here:
    #0 0x55f7a2fff3dd in __interceptor_pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
    #1 0x7f132d9461b8 in _PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:433:14
    #2 0x7f132d92fd9e in PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:518:12
    #3 0x7f1307760759 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:672:8
    #4 0x7f130776c5ad in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, unsigned int, nsIThread**) /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:415:12
    #5 0x7f1307771734 in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, nsIRunnable*, unsigned int) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:139:57
    #6 0x7f13100dfd64 in NS_NewNamedThread<16> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:71:10
    #7 0x7f13100dfd64 in mozilla::ThreadedDriver::Start() /builds/worker/workspace/build/src/dom/media/GraphDriver.cpp:226
    #8 0x7f1310429ae6 in mozilla::MediaStreamGraphImpl::RunInStableState(bool) /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:1715:17
    #9 0x7f1310458a0e in mozilla::(anonymous namespace)::MediaStreamGraphStableStateRunnable::Run() /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:1579:15
    #10 0x7f13074ca1c7 in mozilla::CycleCollectedJSContext::ProcessStableStateQueue() /builds/worker/workspace/build/src/xpcom/base/CycleCollectedJSContext.cpp:434:12
    #11 0x7f13074ce432 in mozilla::CycleCollectedJSContext::AfterProcessTask(unsigned int) /builds/worker/workspace/build/src/xpcom/base/CycleCollectedJSContext.cpp:493:3
    #12 0x7f1309c58665 in XPCJSContext::AfterProcessTask(unsigned int) /builds/worker/workspace/build/src/js/xpconnect/src/XPCJSContext.cpp:1274:28
    #13 0x7f1307766a08 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1273:24
    #14 0x7f130776d8f4 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #15 0x7f1308b76b7f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:88:21
    #16 0x7f1308a4dace in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #17 0x7f1308a4dace in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #18 0x7f1308a4dace in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #19 0x7f131215f563 in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:137:27
    #20 0x7f13167a581e in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:919:20
    #21 0x7f1308a4dace in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #22 0x7f1308a4dace in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #23 0x7f1308a4dace in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #24 0x7f13167a4361 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:754:34
    #25 0x55f7a3049eb3 in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #26 0x55f7a3049eb3 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:267
    #27 0x7f132c476b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

Comment 1

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

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

Comment 2

[Andreas Pehrson [:pehrsons]]

There are two regressing bugs at play here.

The first is from bug 1333341 where a recorder can be set in an error state even though it just doesn't have any data to return for the moment.
The second is from bug 1423253 which added an extra async step in MediaEncoder::Suspend and MediaEncoder::Resume, and thus might race with shutdown when it tries to dispatch an event on the encoder thread.

In the testcase on this bug, these are combined such that MediaEncoder::Suspend's async step from the second bug is done after the recorder was set in an error state by the first bug, and so we attempt to dispatch to the encoder thread when it is already shut down. The crash that fuzzing found was from a diagnostic assert that fails because the dispatch failed.

In release this is harmless and safe, but I'll still fix both bugs.

Comment 3

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1560215 - Add crashtest. r?padenot

Comment 4

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1560215 - Accomodate extra async step added by RunOnGraph. r?padenot

It leads to a race with MediaRecorder::Session::Shutdown where the RunOnGraph
runner was dispatched to the graph before, but tries to dispatch to the encoder
thread after, the encoder thread had BeginShutdown() called on it.

Allowing the encoder thread dispatch to fail in this case is reasonable and
safe.

Depends on D36508

Comment 5

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1560215 - Properly handle errors from VP8TrackEncoder::GetEncodedPartitions. r?bryce

In the particular case of this bug's crashtest, a dataavailable callback from
the track encoder was raised at a point when
VP8TrackEncoder::GetEncodedPartitions would not actually return any data.
This would make VP8TrackEncoder::GetEncodedTrack propagate the error and set the
recorder in an error state, and cancel any ongoing recordings.

VP8TrackEncoder::GetEncodedPartitions was changed in bug 1333341 to distinguish
end-of-stream from other, real, errors; but callsites were not updated.
This patch fixes the callsites.

Depends on D36509

Comment 6

[Andreas Pehrson [:pehrsons]]

test-only: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5b2cd7ced4117569ba39a3d302899fc03539102
with fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c5a88af9c81623184ad80ae5ae6ce37c27559b0

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c66b92abbb53
Add crashtest. r=padenot
https://hg.mozilla.org/integration/autoland/rev/44341752990f
Accomodate extra async step added by RunOnGraph. r=padenot
https://hg.mozilla.org/integration/autoland/rev/ab46619fcd01
Properly handle errors from VP8TrackEncoder::GetEncodedPartitions. r=bryce

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/c66b92abbb53
https://hg.mozilla.org/mozilla-central/rev/44341752990f
https://hg.mozilla.org/mozilla-central/rev/ab46619fcd01