1429507 - Crash near null [@ GraphRate]

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Attachment: trigger.html

Testcase found while fuzzing mozilla-central rev d5f42a23909e.

==16524==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000028 (pc 0x7ff95bde72b1 bp 0x7ff93fbf9d90 sp 0x7ff93fbf9cc0 T34)
==16524==The signal is caused by a READ memory access.
==16524==Hint: address points to the zero page.
    #0 0x7ff95bde72b0 in GraphRate /builds/worker/workspace/build/src/dom/media/StreamTracks.h:195:12
    #1 0x7ff95bde72b0 in GraphRate /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.h:278
    #2 0x7ff95bde72b0 in AddTrack /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.h:739
    #3 0x7ff95bde72b0 in mozilla::MediaEngineDefaultVideoSource::Start(mozilla::SourceMediaStream*, int, nsMainThreadPtrHandle<nsIPrincipal> const&) /builds/worker/workspace/build/src/dom/media/webrtc/MediaEngineDefault.cpp:187
    #4 0x7ff95b79bbe9 in mozilla::GetUserMediaStreamRunnable::Run()::{lambda()#1}::operator()() /builds/worker/workspace/build/src/dom/media/MediaManager.cpp:1227:44
    #5 0x7ff95b79b42c in mozilla::media::LambdaTask<mozilla::GetUserMediaStreamRunnable::Run()::{lambda()#1}>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/media/MediaTaskUtils.h:37:5
    #6 0x7ff95588c4fd in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1040:14
    #7 0x7ff9558a7fb0 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:517:10
    #8 0x7ff95673f845 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:364:5
    #9 0x7ff956691ff9 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #10 0x7ff956691ff9 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #11 0x7ff956691ff9 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #12 0x7ff9566b156f in base::Thread::ThreadMain() /builds/worker/workspace/build/src/ipc/chromium/src/base/thread.cc:181:16
    #13 0x7ff9566a2fdc in ThreadFunc(void*) /builds/worker/workspace/build/src/ipc/chromium/src/base/platform_thread_posix.cc:38:13
    #14 0x7ff9759aa6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    #15 0x7ff974a2c3dc in clone /build/glibc-bfm8X4/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/dom/media/StreamTracks.h:195:12 in GraphRate
Thread T34 (MediaManager) created by T0 (file:// Content) here:
    #0 0x4ac52d in __interceptor_pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:204:3
    #1 0x7ff9566a092f in CreateThread /builds/worker/workspace/build/src/ipc/chromium/src/base/platform_thread_posix.cc:135:14
    #2 0x7ff9566a092f in PlatformThread::Create(unsigned long, PlatformThread::Delegate*, unsigned long*) /builds/worker/workspace/build/src/ipc/chromium/src/base/platform_thread_posix.cc:146
    #3 0x7ff9566b0f0f in base::Thread::StartWithOptions(base::Thread::Options const&) /builds/worker/workspace/build/src/ipc/chromium/src/base/thread.cc:99:8
    #4 0x7ff95b72a2f8 in mozilla::MediaManager::Get() /builds/worker/workspace/build/src/dom/media/MediaManager.cpp:1863:36
    #5 0x7ff95b603cae in mozilla::dom::MediaDevices::GetUserMedia(mozilla::dom::MediaStreamConstraints const&, mozilla::dom::CallerType, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/media/MediaDevices.cpp:194:9
    #6 0x7ff95922436f in getUserMedia /builds/worker/workspace/build/src/obj-firefox/dom/bindings/MediaDevicesBinding.cpp:185:45
    #7 0x7ff95922436f in mozilla::dom::MediaDevicesBinding::getUserMedia_promiseWrapper(JSContext*, JS::Handle<JSObject*>, mozilla::dom::MediaDevices*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/MediaDevicesBinding.cpp:202
    #8 0x7ff95ab7400f in mozilla::dom::GenericPromiseReturningBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3088:13
    #9 0x7ff9616980a4 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #10 0x7ff9616980a4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
    #11 0x7ff961699102 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
    #12 0x7ff9624652f2 in js::ForwardingProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:176:12
    #13 0x7ff962412dbd in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:359:23
    #14 0x7ff9624434f1 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:511:21
    #15 0x7ff962445d84 in js::proxy_Call(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:770:12
    #16 0x7ff961698862 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #17 0x7ff961698862 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:455
    #18 0x7ff9616830d6 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
    #19 0x7ff9616830d6 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
    #20 0x7ff961669a70 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
    #21 0x7ff96169b041 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:706:15
    #22 0x7ff96169b7df in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:738:12
    #23 0x7ff9621920f6 in ExecuteScript(JSContext*, JS::AutoObjectVector&, JS::Handle<JSScript*>, JS::Value*) /builds/worker/workspace/build/src/js/src/jsapi.cpp:4712:12
    #24 0x7ff958c30bb6 in nsJSUtils::ExecutionContext::CompileAndExec(JS::CompileOptions&, JS::SourceBufferHolder&, JS::MutableHandle<JSScript*>) /builds/worker/workspace/build/src/dom/base/nsJSUtils.cpp:266:8
    #25 0x7ff95cae8ffd in mozilla::dom::ScriptLoader::EvaluateScript(mozilla::dom::ScriptLoadRequest*) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:2233:25
    #26 0x7ff95cae3289 in mozilla::dom::ScriptLoader::ProcessRequest(mozilla::dom::ScriptLoadRequest*) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:1876:10
    #27 0x7ff95cac891b in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:1574:10
    #28 0x7ff95cac4b39 in mozilla::dom::ScriptElement::MaybeProcessScript() /builds/worker/workspace/build/src/dom/script/ScriptElement.cpp:147:18
    #29 0x7ff9579ea6a6 in AttemptToExecute /builds/worker/workspace/build/src/obj-firefox/dist/include/nsIScriptElement.h:240:18
    #30 0x7ff9579ea6a6 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) /builds/worker/workspace/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:736
    #31 0x7ff9579e3aed in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/worker/workspace/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:540:7
    #32 0x7ff9579efa2b in nsHtml5ExecutorFlusher::Run() /builds/worker/workspace/build/src/parser/html/nsHtml5StreamParser.cpp:131:20
    #33 0x7ff955865cc0 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:395:25
    #34 0x7ff95588c4fd in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1040:14
    #35 0x7ff9558a7fb0 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:517:10
    #36 0x7ff95673e44a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:97:21
    #37 0x7ff956691ff9 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #38 0x7ff956691ff9 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #39 0x7ff956691ff9 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #40 0x7ff95cc7718a in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:157:27
    #41 0x7ff9613ac41b in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:877:22
    #42 0x7ff956691ff9 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #43 0x7ff956691ff9 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #44 0x7ff956691ff9 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #45 0x7ff9613abe01 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:703:34
    #46 0x4f2dfc in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
    #47 0x4f2dfc in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:280
    #48 0x7ff97494582f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291

==16524==ABORTING

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

[Tracking Requested - why for this release]: fuzz crash

Comment 2

[Alex Chronopoulos [:achronop]]

I can repro in my Nightly reliably given the attached file. I am on Linux 20180111100722
https://crash-stats.mozilla.com/report/index/3bc9a67b-511e-4d76-a575-51b060180112

Comment 3

[Alex Chronopoulos [:achronop]]

mozregression on Linux:

22:49.68 INFO: Last good revision: e3bceb6eb287275e123cb92c487696c6c7325096
22:49.68 INFO: First bad revision: 1fb2d6e0aa2d82c2db246ecd75f7225fecc449ed
22:49.68 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e3bceb6eb287275e123cb92c487696c6c7325096&tochange=1fb2d6e0aa2d82c2db246ecd75f7225fecc449ed

Comment 4

[Andreas Pehrson [:pehrsons]]

Thanks, I'll take a look.

Comment 5

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

We can still take a fix for 59 - I'll track this so we remember to uplift any fixes.

Comment 6

[Andreas Pehrson [:pehrsons]]

Wow, this is a nice find.

So what happens is this fine order of events:
- The first gUM request comes in. It ends with dispatching a runnable.
- Then the reload() causes an OnNavigation event. Not sure why these happen out of order, but it's irrelevant to the bug. Had these come in order we'd just need different code to trigger the same thing.
- The OnNavigation event clears out some (window id based) state from the first gUM request in MediaManager.
- Then the second gUM request comes in, adds some state for the (same) window id.
- The first gUM request's runnable runs. Sees that the window id based state in MediaManager still exists (it's from the 2nd, alas the wrong, request) and assumes everything is jolly. But when it tries to operate on this state things go south.

I reproed this in a debug build and was hitting an assert rather than the crash mentioned. I imagine the root cause is the same for both modes though.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Is this something you still want to address in 59? We're down to one week of development before the RC builds.

Comment 8

[Andreas Pehrson [:pehrsons]]

I don't think 59 is that high prio with this being really rare. If I get it landed in time I could reconsider but for now let's mark it wontfix.

Comment 9

[Julien Cristau [:jcristau] (back April 22)]

not tracking based on low crash volume

Comment 10

[Andreas Pehrson [:pehrsons]]

This has been overtaken by events so the root cause has been fixed. But I still want to add crashtests.

Comment 11

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1429507 - Add crashtests.

Review commit: https://reviewboard.mozilla.org/r/248530/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/248530/

Comment 12

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8982547 [details]
Bug 1429507 - Add crashtests.

https://reviewboard.mozilla.org/r/248530/#review254834

lgtm

Comment 13

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8982547 [details]
Bug 1429507 - Add crashtests.

https://reviewboard.mozilla.org/r/248530/#review254836

::: dom/media/tests/crashtests/1429507_1.html:7
(Diff revision 1)
> +  <head>
> +    <script>
> +      try { o1 = window.open("") } catch (e) {}
> +      try { o1.location.reload() } catch (e) {}
> +      try { o2 = new RTCPeerConnection({iceServers: [{"url": "stun:d"}]}) } catch (e) {}
> +      try { o1.navigator.mediaDevices.getUserMedia({video: true, fake: true}).catch((error) => {}) } catch (e) {}

Actually, it would be nice if these cleaned up after themselves (track.stop and pc.close()) since leaving these resources open cause nondeterminism in following tests.


I know we track that for mochitests but not crashtests presumably, so maybe this is a lost cause.

Comment 14

[Andreas Pehrson [:pehrsons]]

tests in 59: https://hg.mozilla.org/try/rev/6bcdf687feff2f44bef56776275ae65c2da55c86
tests on m-c: https://hg.mozilla.org/try/rev/8f64026717998f12526c9689ff5ae5c0b320bb1c

Comment 15

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8982547 [details]
Bug 1429507 - Add crashtests.

https://reviewboard.mozilla.org/r/248530/#review254836

> Actually, it would be nice if these cleaned up after themselves (track.stop and pc.close()) since leaving these resources open cause nondeterminism in following tests.
> 
> 
> I know we track that for mochitests but not crashtests presumably, so maybe this is a lost cause.

How do you propose we do that?

With the `reload()` happening the test should already be cleaning itself up. And on navigation to the next test it should be cleaned up by MediaManager anyway.

I fear instead that cleaning up explicitly might affect the test outcome because it's a race between the `reload()` and the second `getUserMedia()` that caused problems in the first place.

Comment 16

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8982547 [details]
Bug 1429507 - Add crashtests.

https://reviewboard.mozilla.org/r/248530/#review254836

> How do you propose we do that?
> 
> With the `reload()` happening the test should already be cleaning itself up. And on navigation to the next test it should be cleaned up by MediaManager anyway.
> 
> I fear instead that cleaning up explicitly might affect the test outcome because it's a race between the `reload()` and the second `getUserMedia()` that caused problems in the first place.

You're right.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0e20e5a9570
Add crashtests. r=jib

Comment 18

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/d0e20e5a9570