1738931 - Crash in [@ mozilla::WebrtcCallWrapper::UnsetRemoteSSRC], [@ mozilla::WebrtcVideoConduit::SetRemoteSSRCConfig]

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

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

• 1 installations reporting crashes with [@ mozilla::WebrtcCallWrapper::UnsetRemoteSSRC]: https://crash-stats.mozilla.org/report/index/17849873-de6a-47b5-9530-4f3bc0211102
• 2 installations reporting crashes with [@ std::_Rb_tree_increment] which has mozilla::WebrtcVideoConduit::SetRemoteSSRCConfig(unsigned int, unsigned int) as parent frame.

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll mozilla::WebrtcCallWrapper::UnsetRemoteSSRC dom/media/webrtc/libwebrtcglue/WebrtcCallWrapper.cpp:55
1 xul.dll mozilla::WebrtcVideoConduit::SetRemoteSSRCConfig dom/media/webrtc/libwebrtcglue/VideoConduit.cpp:982
2 xul.dll mozilla::WebrtcVideoConduit::OnControlConfigChange dom/media/webrtc/libwebrtcglue/VideoConduit.cpp:826
3 xul.dll mozilla::detail::RunnableFunction<`lambda at /builds/worker/workspace/obj-build/dist/include/mozilla/StateWatching.h:248:34'>::Run xpcom/threads/nsThreadUtils.h:531
4 xul.dll mozilla::TaskQueue::DrainDirectTasks xpcom/threads/TaskQueue.cpp:272
5 xul.dll mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run xpcom/threads/TaskDispatcher.h:224
6 xul.dll mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/dom/media/webrtc/libwebrtcglue/TaskQueueWrapper.h:88:9'>::Run xpcom/threads/nsThreadUtils.h:531
7 xul.dll mozilla::TaskQueue::Runner::Run xpcom/threads/TaskQueue.cpp:208
8 xul.dll nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:305
9 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1169

Comment 1

[Andreas Pehrson [:pehrsons]]

WebrtcCallWrapper::UnsetRemoteSSRC loops over mConduits (std::set<MediaSessionConduit*>) and calls UnsetRemoteSSRC on them. This method for VideoConduit calls into VideoConduit::SetRemoteSSRCConfig which calls WebrtcCallWrapper::UnregisterConduit, which modifies the std::set that we're looping over higher up in the stack.

I think the safest bet here is to iterate over a copy of mConduits in WebrtcCallWrapper::UnsetRemoteSSRC. There's a theoretical lifetime issue there since mConduits contains raw pointers, but currently no UnsetRemoteSSRC path affects a conduit strong-ref, so this is fine for now.

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1738931 - Unset remote SSRC in a direct task (stable state) to avoid re-entrancy. r?bwc

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

crashes are UAFs

Comment 4

[Tyson Smith [:tsmith]]

The fuzzers hit just found this. I can attach a reduced test case when auto-reduction is complete if you are interested.

Comment 5

[Andreas Pehrson [:pehrsons]]

I'm interested in a test case as the path to hit this is not trivial.

Comment 6

[Andreas Pehrson [:pehrsons]]

FWIW https://jsfiddle.net/jib1/dy6rh1nw/ reproduces this reliably. Here it is in pernosco.

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1738931 - WatchManager: Allow manual notifications without a watch target present. r?#xpcom-reviewers

This is essentially a short-hand for adding a direct task to the current thread,
possible when a watch manager is already present.

Comment 8

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

Landed:
https://hg.mozilla.org/integration/autoland/rev/3964c080fafc194b712a03e30123a481954e6bb7
https://hg.mozilla.org/integration/autoland/rev/fbaeb0434e8afe0effa959be4ab68ce6247a6359

Backed out for causing mochitest failures on VideoConduit and WatchManager, at least on Android debug:
https://hg.mozilla.org/integration/autoland/rev/8722641a9f0d4d4d4985e67b7e9a4d064aecc57e

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=fbaeb0434e8afe0effa959be4ab68ce6247a6359&selectedTaskRun=FvwIgZr0Sviwarv9qVaBVw.0
Failure log: https://treeherder.mozilla.org/logviewer?job_id=357753971&repo=autoland

[task 2021-11-11T11:46:09.738Z] 11:46:09     INFO -  TEST-START | dom/media/webrtc/tests/mochitests/test_peerConnection_captureStream_canvas_2d_noSSRC.html
[task 2021-11-11T11:46:40.083Z] 11:46:40     INFO -  wait for org.mozilla.geckoview.test_runner complete; top activity=com.android.launcher3
[task 2021-11-11T11:46:40.084Z] 11:46:40     INFO -  runtestsremote.py | Application ran for: 0:00:44.866391
[task 2021-11-11T11:46:40.202Z] 11:46:40     INFO -  mozcrash Copy/paste: /builds/worker/fetches/minidump_stackwalk/minidump_stackwalk /tmp/tmpzodqdd66/79d7711e-0a24-d626-722e-0a5258d63f27.dmp /builds/worker/workspace/build/symbols https://symbols.mozilla.org/
[task 2021-11-11T11:46:44.197Z] 11:46:44     INFO -  mozcrash Saved minidump as /builds/worker/workspace/build/blobber_upload_dir/79d7711e-0a24-d626-722e-0a5258d63f27.dmp
[task 2021-11-11T11:46:44.197Z] 11:46:44     INFO -  mozcrash Saved app info as /builds/worker/workspace/build/blobber_upload_dir/79d7711e-0a24-d626-722e-0a5258d63f27.extra
[task 2021-11-11T11:46:44.204Z] 11:46:44  WARNING -  PROCESS-CRASH | dom/media/webrtc/tests/mochitests/test_peerConnection_captureStream_canvas_2d_noSSRC.html | application crashed [@ mozilla::WatchManager<mozilla::WebrtcVideoConduit>::PerCallbackWatcher::Notify()::{lambda()#1}::operator()() const]
[task 2021-11-11T11:46:44.205Z] 11:46:44     INFO -  Mozilla crash reason: MOZ_ASSERT(mCallThread->IsOnCurrentThread())
[task 2021-11-11T11:46:44.205Z] 11:46:44     INFO -  Crash dump filename: /tmp/tmpzodqdd66/79d7711e-0a24-d626-722e-0a5258d63f27.dmp
[task 2021-11-11T11:46:44.205Z] 11:46:44     INFO -  Operating system: Android
[task 2021-11-11T11:46:44.205Z] 11:46:44     INFO -                    0.0.0 Linux 3.10.0+ #260 SMP PREEMPT Fri May 19 12:48:14 PDT 2017 x86_64
[task 2021-11-11T11:46:44.205Z] 11:46:44     INFO -  CPU: amd64
[task 2021-11-11T11:46:44.206Z] 11:46:44     INFO -       family 6 model 6 stepping 3
[task 2021-11-11T11:46:44.206Z] 11:46:44     INFO -       4 CPUs
[task 2021-11-11T11:46:44.206Z] 11:46:44     INFO -  GPU: UNKNOWN
[task 2021-11-11T11:46:44.206Z] 11:46:44     INFO -  Crash reason:  SIGSEGV / SEGV_MAPERR
[task 2021-11-11T11:46:44.206Z] 11:46:44     INFO -  Crash address: 0x0
[task 2021-11-11T11:46:44.206Z] 11:46:44     INFO -  Process uptime: not available
[task 2021-11-11T11:46:44.207Z] 11:46:44     INFO -  Thread 44 tid 24402 (crashed) 0  libxul.so!mozilla::WebrtcVideoConduit::NotifyUnsetCurrentRemoteSSRC() [VideoConduit.cpp:fbaeb0434e8afe0effa959be4ab68ce6247a6359 : 980 + 0x29]
[task 2021-11-11T11:46:44.207Z] 11:46:44     INFO -      rax = 0x00007cb55dbf5583   rdx = 0x0000000000000004
[task 2021-11-11T11:46:44.207Z] 11:46:44     INFO -      rcx = 0x00007cb5794acc50   rbx = 0x00007cb54d57dd00
[task 2021-11-11T11:46:44.207Z] 11:46:44     INFO -      rsi = 0x00007cb54c9b8bd0   rdi = 0x000000000000001b
[task 2021-11-11T11:46:44.207Z] 11:46:44     INFO -      rbp = 0x00007cb54c9b9d00   rsp = 0x00007cb54c9b9cf0
[task 2021-11-11T11:46:44.208Z] 11:46:44     INFO -       r8 = 0x000000000000ffff    r9 = 0x0000000000000000
[task 2021-11-11T11:46:44.208Z] 11:46:44     INFO -      r10 = 0x00007cb57e8d13d0   r11 = 0x0000000000000246
[task 2021-11-11T11:46:44.208Z] 11:46:44     INFO -      r12 = 0x00007cb54c9b9de0   r13 = 0x00007cb54d710e78
[task 2021-11-11T11:46:44.208Z] 11:46:44     INFO -      r14 = 0x00007cb54d57dd00   r15 = 0xaaaaaaaaaaaaaaaa
[task 2021-11-11T11:46:44.208Z] 11:46:44     INFO -      rip = 0x00007cb558c0724d
[task 2021-11-11T11:46:44.208Z] 11:46:44     INFO -      Found by: given as instruction pointer in context
[task 2021-11-11T11:46:44.209Z] 11:46:44     INFO -   1  libxul.so!mozilla::WatchManager<mozilla::WebrtcVideoConduit>::PerCallbackWatcher::Notify()::{lambda()#1}::operator()() const [StateWatching.h:fbaeb0434e8afe0effa959be4ab68ce6247a6359 : 249 + 0x2e]
[task 2021-11-11T11:46:44.209Z] 11:46:44     INFO -      rbp = 0x00007cb54c9b9d20   rsp = 0x00007cb54c9b9d10
[task 2021-11-11T11:46:44.209Z] 11:46:44     INFO -      rip = 0x00007cb558c23cf7
[task 2021-11-11T11:46:44.209Z] 11:46:44     INFO -      Found by: previous frame's frame pointer
[task 2021-11-11T11:46:44.210Z] 11:46:44     INFO -   2  libxul.so!mozilla::detail::RunnableFunction<mozilla::WatchManager<mozilla::WebrtcVideoConduit>::PerCallbackWatcher::Notify()::{lambda()#1}>::Run() [nsThreadUtils.h:fbaeb0434e8afe0effa959be4ab68ce6247a6359 : 531 + 0x9]
[task 2021-11-11T11:46:44.210Z] 11:46:44     INFO -      rbp = 0x00007cb54c9b9d30   rsp = 0x00007cb54c9b9d30
[task 2021-11-11T11:46:44.210Z] 11:46:44     INFO -      rip = 0x00007cb558c23c6b
[task 2021-11-11T11:46:44.210Z] 11:46:44     INFO -      Found by: previous frame's frame pointer
[task 2021-11-11T11:46:44.211Z] 11:46:44     INFO -   3  libxul.so!mozilla::SimpleTaskQueue::DrainTasks() [TaskDispatcher.h:fbaeb0434e8afe0effa959be4ab68ce6247a6359 : 42 + 0x11]
[task 2021-11-11T11:46:44.211Z] 11:46:44     INFO -      rbp = 0x00007cb54c9b9d70   rsp = 0x00007cb54c9b9d40
[task 2021-11-11T11:46:44.211Z] 11:46:44     INFO -      rip = 0x00007cb556822663
[task 2021-11-11T11:46:44.211Z] 11:46:44     INFO -      Found by: previous frame's frame pointer

Comment 9

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1738931 - Make the TaskQueueWrapper's shutdown mutex recursive. r?bwc

Comment 10

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

Unset remote SSRC in a direct task (stable state) to avoid re-entrancy. r=bwc
https://hg.mozilla.org/integration/autoland/rev/80ef874e08519482b47b92ae1aa0afbf4d4f3452
https://hg.mozilla.org/mozilla-central/rev/80ef874e0851

Make the TaskQueueWrapper's shutdown mutex recursive. r=bwc
https://hg.mozilla.org/integration/autoland/rev/373a6f2fcd2bb1e3e65daa5d0e587434181cc03f
https://hg.mozilla.org/mozilla-central/rev/373a6f2fcd2b

Comment 11

[Release mgmt bot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 12

[Tyson Smith [:tsmith]]

(In reply to Andreas Pehrson [:pehrsons] from comment #5)

I'm interested in a test case as the path to hit this is not trivial.

Sorry I was unable to get a reduced test case for this issue.

Comment 13

[Andreas Pehrson [:pehrsons]]

(In reply to Tyson Smith [:tsmith] from comment #12)

(In reply to Andreas Pehrson [:pehrsons] from comment #5)

I'm interested in a test case as the path to hit this is not trivial.

Sorry I was unable to get a reduced test case for this issue.

That's fine, we have a unittest that triggers this artificially.