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.

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.

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

(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.

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"

FWIW, this appears to reproduce pretty reliably on ASAN e10s runs (hidden by default on TBPL). https://tbpl.mozilla.org/?showall=1&jobname=Ubuntu%20ASAN%20VM%2012.04%20x64%20mozilla-inbound%20opt%20test%20mochitest-e10s-1&tree=Mozilla-Inbound

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,

Comment on attachment 8487166 [details] [diff] [review] possible patch Sec-approval+ for trunk. Let's get this on Aurora as well.

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

Comment on attachment 8487166 [details] [diff] [review] possible patch Thanks, Mr. Jesup!

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.