Closed Bug 1064117 Opened 5 years ago Closed 5 years ago

intermittent AddressSanitizer: heap-use-after-free content/media/../../dist/include/nsAutoPtr.h:1017 get

Categories

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

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
e10s + ---
firefox32 --- unaffected
firefox33 --- unaffected
firefox34 --- fixed
firefox35 + fixed
firefox-esr31 --- unaffected

People

(Reporter: karlt, Assigned: padenot)

References

Details

(4 keywords, Whiteboard: [asan])

Attachments

(1 file)

https://tbpl.mozilla.org/php/getParsedLog.php?id=47575268&full=1&branch=mozilla-inbound#error0

==1770==ERROR: AddressSanitizer: heap-use-after-free on address 0x60b0004e1230 at pc 0x7f5658e80f90 bp 0x7f5600057630 sp 0x7f5600057628
READ of size 8 at 0x60b0004e1230 thread T3134 (MediaStreamGrph)
    #0 0x7f5658e80f8f in get /builds/slave/m-in-l64-asan-0000000000000000/build/obj-firefox/content/media/../../dist/include/nsAutoPtr.h:1017
    #1 0x7f5658e80f8f in operator mozilla::GraphDriver * /builds/slave/m-in-l64-asan-0000000000000000/build/obj-firefox/content/media/../../dist/include/nsAutoPtr.h:1030
    #2 0x7f5658e80f8f in mozilla::ThreadedDriver::RunThread() /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/GraphDriver.cpp:297
    #3 0x7f5658e9bc02 in mozilla::MediaStreamGraphInitThreadRunnable::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/GraphDriver.cpp:214
    #4 0x7f5654c5c9b1 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/threads/nsThread.cpp:823
    #5 0x7f5654cb9cea in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/glue/nsThreadUtils.cpp:265
    #6 0x7f56554ae3a7 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/ipc/glue/MessagePump.cpp:326
    #7 0x7f565545db00 in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/ipc/chromium/src/base/message_loop.cc:229
    #8 0x7f565545db00 in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/ipc/chromium/src/base/message_loop.cc:222
    #9 0x7f565545db00 in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/ipc/chromium/src/base/message_loop.cc:196
    #10 0x7f5654c596f5 in nsThread::ThreadFunc(void*) /builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/threads/nsThread.cpp:350
    #11 0x7f566b14d405 in _pt_root /builds/slave/m-in-l64-asan-0000000000000000/build/nsprpub/pr/src/pthreads/ptthread.c:212
    #12 0x7f566e88ae99 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7e99)
    #13 0x7f566d99bdbc (/lib/x86_64-linux-gnu/libc.so.6+0xf3dbc)
0x60b0004e1230 is located 80 bytes inside of 104-byte region [0x60b0004e11e0,0x60b0004e1248)
freed by thread T0 here:
    #0 0x470d21 in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64
    #1 0x7f5658ee624e in Release /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/GraphDriver.h:77
    #2 0x7f5658ee624e in ~nsRefPtr /builds/slave/m-in-l64-asan-0000000000000000/build/obj-firefox/content/media/../../dist/include/nsAutoPtr.h:852
    #3 0x7f5658ee624e in mozilla::MediaStreamGraphImpl::~MediaStreamGraphImpl() /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/MediaStreamGraph.cpp:74
    #4 0x7f5658ee726d in mozilla::MediaStreamGraphImpl::~MediaStreamGraphImpl() /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/MediaStreamGraph.cpp:69
    #5 0x7f5658effabc in mozilla::MediaStreamGraphImpl::Release() /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/MediaStreamGraph.cpp:2822
    #6 0x7f5658f223f7 in ~nsRefPtr /builds/slave/m-in-l64-asan-0000000000000000/build/obj-firefox/content/media/../../dist/include/nsAutoPtr.h:852
    #7 0x7f5658f223f7 in ~MediaStreamGraphShutDownRunnable /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/MediaStreamGraph.cpp:1471
    #8 0x7f5658f223f7 in mozilla::(anonymous namespace)::MediaStreamGraphShutDownRunnable::~MediaStreamGraphShutDownRunnable() /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/MediaStreamGraph.cpp:1471
previously allocated by thread T0 here:
    #0 0x470f21 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x7f5667631bed in moz_xmalloc /builds/slave/m-in-l64-asan-0000000000000000/build/memory/mozalloc/mozalloc.cpp:52
    #2 0x7f5658eff2cb in operator new /builds/slave/m-in-l64-asan-0000000000000000/build/obj-firefox/content/media/../../dist/include/mozilla/mozalloc.h:201
    #3 0x7f5658eff2cb in mozilla::MediaStreamGraphImpl::MediaStreamGraphImpl(bool, int, unsigned char, mozilla::dom::AudioChannel) /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/MediaStreamGraph.cpp:2735
    #4 0x7f5658eff6eb in mozilla::MediaStreamGraph::CreateNonRealtimeInstance(int) /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/MediaStreamGraph.cpp:2798
    #5 0x7f5658feafff in mozilla::dom::AudioDestinationNode::AudioDestinationNode(mozilla::dom::AudioContext*, bool, mozilla::dom::AudioChannel, unsigned int, unsigned int, float) /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/webaudio/AudioDestinationNode.cpp:324
Thread T3134 (MediaStreamGrph) created by T0 here:
    #0 0x45d795 in __interceptor_pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:175
    #1 0x7f566b149d8d in _PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/nsprpub/pr/src/pthreads/ptthread.c:453
    #2 0x7f566b14990a in PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/nsprpub/pr/src/pthreads/ptthread.c:544
    #3 0x7f5654c5ac0b in nsThread::Init() /builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/threads/nsThread.cpp:455
    #4 0x7f5654c600fc in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) /builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/threads/nsThreadManager.cpp:269
    #5 0x7f5654cb941c in NS_NewThread(nsIThread**, nsIRunnable*, unsigned int) /builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/glue/nsThreadUtils.cpp:68
    #6 0x7f5658e7fd44 in NS_NewNamedThread<16> /builds/slave/m-in-l64-asan-0000000000000000/build/obj-firefox/content/media/../../dist/include/nsThreadUtils.h:74
    #7 0x7f5658e7fd44 in mozilla::ThreadedDriver::Start() /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/GraphDriver.cpp:226
    #8 0x7f5658ef7dd8 in mozilla::MediaStreamGraphImpl::RunInStableState(bool) /builds/slave/m-in-l64-asan-0000000000000000/build/content/media/MediaStreamGraph.cpp:1673
ASAN UAF -> sec bug.  'this' being freed out from under it in a processing loop; sec-high or sec-critical

In RunThread(), the main driver loop:
    if (mNextDriver && stillProcessing) {

'this' appears to have been freed out from under us.

It was freed at mozilla::MediaStreamGraphImpl::~MediaStreamGraphImpl():74 or :69, from the ShutdownRunnable, on Mainthread.
Group: core-security
Severity: normal → critical
Keywords: crash, sec-critical
Duplicate of this bug: 1064519
Attached patch possible patchSplinter Review
Maybe this ?

The ASAN report shows that the thread for the OfflineAudioContext still runs
while we are deleting the graph. At this point mForceShutdown should be true, so
we just have to wait for the driver thread to exit and continue.

It's not clear to me if nsThread::Shutdown does a sync wait. It seems to do so.
Attachment #8487166 - Flags: review?(rjesup)
Assignee: nobody → paul
Status: NEW → ASSIGNED
Comment on attachment 8487166 [details] [diff] [review]
possible patch

Review of attachment 8487166 [details] [diff] [review]:
-----------------------------------------------------------------

r+; but I'd like to see a Try run.
nsThread::Shutdown() is absolutely synchronous
Attachment #8487166 - Flags: review?(rjesup) → review+
(In reply to Paul Adenot (:padenot) from comment #3)
> It's not clear to me if nsThread::Shutdown does a sync wait. It seems to do
> so.

It won't return until the thread is shut down.

However, I wouldn't usually describe running main (current) thread events as a "wait".

The world may be quite different when Shutdown() returns.
Duplicate of this bug: 1064713
Ok, so that's what I want I think.
Yup - the target thread is shut down when it returned, which is what I meant by synchronous and "wait"
Keywords: csectype-uaf
Whiteboard: [asan]
Are we ready to land this?  Since it's in 34, we'll need security approval

This should only apply to 34 & 35 as it was due to bug 848954
Comment on attachment 8487166 [details] [diff] [review]
possible patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not easily, this is a quite rare race condition.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No.

Which older supported branches are affected by this flaw?
34, 35

If not all supported branches, which bug introduced the flaw?
bug 848954

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Yes, we have backports for all branches

How likely is this patch to cause regressions; how much testing does it need?
It's green on try,
Attachment #8487166 - Flags: sec-approval?
Flags: needinfo?(paul)
Comment on attachment 8487166 [details] [diff] [review]
possible patch

Sec-approval+ for trunk. Let's get this on Aurora as well.
Attachment #8487166 - Flags: sec-approval? → sec-approval+
Comment on attachment 8487166 [details] [diff] [review]
possible patch

Approval Request Comment
[Feature/regressing bug #]: bug 848954

[User impact if declined]: sec UAF

[Describe test coverage new/current, TBPL]: see s-a request

[Risks and why]: see s-a request

[String/UUID change made/needed]: none
Attachment #8487166 - Flags: approval-mozilla-aurora?
Comment on attachment 8487166 [details] [diff] [review]
possible patch

Thanks, Mr. Jesup!
Attachment #8487166 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/c6363ee84124
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Marking this as qe‑verify- as there's no STR/POC attached. Quickly talked to Paul via IRC to double check and see if there's anything QE can do here in terms of verification.
Flags: qe-verify-
Group: core-security
You need to log in before you can comment on or make changes to this bug.