Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=192567144&repo=mozilla-central&lineNumber=4105 Task details: https://tools.taskcluster.net/groups/Ty8PoAHlScqn2ZXgc3bvIg/tasks/QD3QWOgWT1mT1wiyw3lk3w/details 16:37:50 INFO - REFTEST TEST-START | file:///Z:/task_1533659308/build/tests/reftest/tests/dom/media/test/crashtests/1411322.html 16:37:50 INFO - REFTEST TEST-LOAD | file:///Z:/task_1533659308/build/tests/reftest/tests/dom/media/test/crashtests/1411322.html | 608 / 3584 (16%) 16:37:50 INFO - ================================================================= 16:37:50 ERROR - ==7176==ERROR: AddressSanitizer: attempting to call malloc_usable_size() for pointer which is not owned: 0x1208926e6be0 16:37:50 INFO - REFTEST TEST-PASS | file:///Z:/task_1533659308/build/tests/reftest/tests/dom/media/test/crashtests/1411322.html | (LOAD ONLY) 16:37:50 INFO - REFTEST TEST-END | file:///Z:/task_1533659308/build/tests/reftest/tests/dom/media/test/crashtests/1411322.html 16:37:50 INFO - REFTEST TEST-START | file:///Z:/task_1533659308/build/tests/reftest/tests/dom/media/test/crashtests/1450845.html 16:37:50 INFO - REFTEST TEST-LOAD | file:///Z:/task_1533659308/build/tests/reftest/tests/dom/media/test/crashtests/1450845.html | 609 / 3584 (16%) 16:37:50 INFO - ==7176==WARNING: Failed to use and restart external symbolizer! 16:37:50 INFO - #0 0x7fffeab5e4c8 in __asan::AsanThread::ThreadStart Z:\task_1533162563\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_thread.cc:262 16:37:50 INFO - #1 0x7ffff9f02773 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x180012773) 16:37:50 INFO - #2 0x7fffeaa875c3 in patched_BaseThreadInitThunk z:\build\build\src\mozglue\build\WindowsDllBlocklist.cpp:670 16:37:50 INFO - #3 0x7ffffca00d60 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x180070d60) 16:37:50 INFO - 0x1208926e6be0 is located 32 bytes inside of 199-byte region [0x1208926e6bc0,0x1208926e6c87) 16:37:50 INFO - allocated by thread T128 here: 16:37:50 INFO - #0 0x7fffeab540d0 in malloc Z:\task_1533162563\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_malloc_win.cc:60 16:37:50 INFO - #1 0x7ffff9e0e251 in o_strncpy_s+0x91 (C:\Windows\System32\ucrtbase.dll+0x18001e251) 16:37:50 INFO - #2 0x7fffea940612 in av_malloc z:\build\build\src\media\ffvpx\libavutil\mem.c:90 16:37:50 INFO - #3 0x7fffdcfb12e6 in av_rdft_init z:\build\build\src\media\ffvpx\libavcodec\avfft.c:97 16:37:51 INFO - #4 0x7fffc93e67ec in mozilla::FFTBlock::PerformFFT z:\build\build\src\dom\media\webaudio\FFTBlock.h:83 16:37:51 INFO - #5 0x7fffc94c7d9c in mozilla::FFTBlock::PadAndMakeScaledDFT z:\build\build\src\obj-firefox\dist\include\mozilla\FFTBlock.h:172 16:37:51 INFO - #6 0x7fffc94d55c2 in WebCore::ReverbConvolverStage::ReverbConvolverStage z:\build\build\src\dom\media\webaudio\blink\ReverbConvolverStage.cpp:54 16:37:51 INFO - #7 0x7fffc94d36bd in WebCore::ReverbConvolver::ReverbConvolver z:\build\build\src\dom\media\webaudio\blink\ReverbConvolver.cpp:102 16:37:51 INFO - #8 0x7fffc94d2483 in WebCore::Reverb::initialize z:\build\build\src\dom\media\webaudio\blink\Reverb.cpp:136 16:37:51 INFO - #9 0x7fffc94d1b3d in WebCore::Reverb::Reverb z:\build\build\src\dom\media\webaudio\blink\Reverb.cpp:102 16:37:51 INFO - #10 0x7fffc947c281 in mozilla::dom::ConvolverNodeEngine::SetBuffer z:\build\build\src\dom\media\webaudio\ConvolverNode.cpp:83 16:37:51 INFO - #11 0x7fffc8e68f7f in mozilla::MediaStreamGraphImpl::RunMessagesInQueue z:\build\build\src\dom\media\MediaStreamGraph.cpp:1249 16:37:51 INFO - #12 0x7fffc8e6e9b0 in mozilla::MediaStreamGraphImpl::OneIteration z:\build\build\src\dom\media\MediaStreamGraph.cpp:1459 16:37:51 INFO - #13 0x7fffc8be143b in mozilla::AudioCallbackDriver::DataCallback z:\build\build\src\dom\media\GraphDriver.cpp:1000 16:37:51 INFO - #14 0x7fffcc5740cc in passthrough_resampler<float>::fill z:\build\build\src\media\libcubeb\src\cubeb_resampler.cpp:76 16:37:51 INFO - #15 0x7fffcc58e8d6 in `anonymous namespace'::refill z:\build\build\src\media\libcubeb\src\cubeb_wasapi.cpp:477 16:37:51 INFO - #16 0x7fffcc58ae78 in `anonymous namespace'::refill_callback_output z:\build\build\src\media\libcubeb\src\cubeb_wasapi.cpp:809 16:37:51 INFO - #17 0x7fffcc5916a9 in `anonymous namespace'::wasapi_stream_render_loop z:\build\build\src\media\libcubeb\src\cubeb_wasapi.cpp:961 16:37:51 INFO - #18 0x7ffff9e10368 in o__strtoui64+0x58 (C:\Windows\System32\ucrtbase.dll+0x180020368) 16:37:51 INFO - #19 0x7fffeab5e4c8 in __asan::AsanThread::ThreadStart Z:\task_1533162563\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_thread.cc:262 16:37:51 INFO - #20 0x7ffff9f02773 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x180012773) 16:37:51 INFO - #21 0x7fffeaa875c3 in patched_BaseThreadInitThunk z:\build\build\src\mozglue\build\WindowsDllBlocklist.cpp:670 16:37:51 INFO - #22 0x7ffffca00d60 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x180070d60) 16:37:51 INFO - Thread T129 created by T128 here: 16:37:51 INFO - #0 0x7fffeab5f600 in __asan_wrap_CreateThread Z:\task_1533162563\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_win.cc:146 16:37:51 INFO - #1 0x7ffff9e0fde6 in beginthreadex+0x56 (C:\Windows\System32\ucrtbase.dll+0x18001fde6) 16:37:51 INFO - #2 0x7fffe9afa3fd in _PR_MD_CREATE_THREAD z:\build\build\src\nsprpub\pr\src\md\windows\w95thred.c:151 16:37:51 INFO - #3 0x7fffe9b2a9ad in _PR_NativeCreateThread z:\build\build\src\nsprpub\pr\src\threads\combined\pruthr.c:1044 16:37:51 INFO - #4 0x7fffe9b2b3e1 in _PR_CreateThread z:\build\build\src\nsprpub\pr\src\threads\combined\pruthr.c:1162 16:37:51 INFO - #5 0x7fffe9b2bf0f in PR_CreateThread z:\build\build\src\nsprpub\pr\src\threads\combined\pruthr.c:1374 16:37:51 INFO - #6 0x7fffc1597fb9 in nsThread::Init z:\build\build\src\xpcom\threads\nsThread.cpp:727 16:37:51 INFO - #7 0x7fffc15a3c48 in nsThreadManager::NewNamedThread z:\build\build\src\xpcom\threads\nsThreadManager.cpp:518 16:37:51 INFO - #8 0x7fffc15a7f93 in NS_NewNamedThread z:\build\build\src\xpcom\threads\nsThreadUtils.cpp:142 16:37:51 INFO - #9 0x7fffc8bd876d in mozilla::ThreadedDriver::Start z:\build\build\src\dom\media\GraphDriver.cpp:253 16:37:51 INFO - #10 0x7fffc8bd7b1a in mozilla::GraphDriver::SwitchToNextDriver z:\build\build\src\dom\media\GraphDriver.cpp:117 16:37:51 INFO - #11 0x7fffc8be1ab8 in mozilla::AudioCallbackDriver::DataCallback z:\build\build\src\dom\media\GraphDriver.cpp:1048 16:37:51 INFO - #12 0x7fffcc5740cc in passthrough_resampler<float>::fill z:\build\build\src\media\libcubeb\src\cubeb_resampler.cpp:76 16:37:51 INFO - #13 0x7fffcc58e8d6 in `anonymous namespace'::refill z:\build\build\src\media\libcubeb\src\cubeb_wasapi.cpp:477 16:37:51 INFO - #14 0x7fffcc58ae78 in `anonymous namespace'::refill_callback_output z:\build\build\src\media\libcubeb\src\cubeb_wasapi.cpp:809 16:37:51 INFO - #15 0x7fffcc5916a9 in `anonymous namespace'::wasapi_stream_render_loop z:\build\build\src\media\libcubeb\src\cubeb_wasapi.cpp:961 16:37:51 INFO - #16 0x7ffff9e10368 in o__strtoui64+0x58 (C:\Windows\System32\ucrtbase.dll+0x180020368) 16:37:51 INFO - #17 0x7fffeab5e4c8 in __asan::AsanThread::ThreadStart Z:\task_1533162563\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_thread.cc:262 16:37:51 INFO - #18 0x7ffff9f02773 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x180012773) 16:37:51 INFO - #19 0x7fffeaa875c3 in patched_BaseThreadInitThunk z:\build\build\src\mozglue\build\WindowsDllBlocklist.cpp:670 16:37:51 INFO - #20 0x7ffffca00d60 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x180070d60) 16:37:51 INFO - Thread T128 created by T75 here: 16:37:51 INFO - #0 0x7fffeab5f600 in __asan_wrap_CreateThread Z:\task_1533162563\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_win.cc:146 16:37:51 INFO - #1 0x7ffff9e0fde6 in beginthreadex+0x56 (C:\Windows\System32\ucrtbase.dll+0x18001fde6) 16:37:51 INFO - #2 0x7fffcc582308 in `anonymous namespace'::wasapi_stream_start z:\build\build\src\media\libcubeb\src\cubeb_wasapi.cpp:1989 16:37:51 INFO - #3 0x7fffc8bdb9d8 in mozilla::AudioCallbackDriver::Init z:\build\build\src\dom\media\GraphDriver.cpp:727 16:37:51 INFO - #4 0x7fffc8bdadf8 in mozilla::AsyncCubebTask::Run z:\build\build\src\dom\media\GraphDriver.cpp:520 16:37:51 INFO - #5 0x7fffc15a9092 in nsThreadPool::Run z:\build\build\src\xpcom\threads\nsThreadPool.cpp:231 16:37:51 INFO - #6 0x7fffc159c514 in nsThread::ProcessNextEvent z:\build\build\src\xpcom\threads\nsThread.cpp:1235 16:37:51 INFO - #7 0x7fffc15a4f7e in NS_ProcessNextEvent z:\build\build\src\xpcom\threads\nsThreadUtils.cpp:519 16:37:51 INFO - #8 0x7fffc25fda01 in mozilla::ipc::MessagePumpForNonMainThreads::Run z:\build\build\src\ipc\glue\MessagePump.cpp:334 16:37:51 INFO - #9 0x7fffc255f29e in MessageLoop::RunHandler z:\build\build\src\ipc\chromium\src\base\message_loop.cc:318 16:37:51 INFO - #10 0x7fffc255f026 in MessageLoop::Run z:\build\build\src\ipc\chromium\src\base\message_loop.cc:298 16:37:51 INFO - #11 0x7fffc15946cf in nsThread::ThreadFunc z:\build\build\src\xpcom\threads\nsThread.cpp:467 16:37:51 INFO - #12 0x7fffe9b29d40 in _PR_NativeRunThread z:\build\build\src\nsprpub\pr\src\threads\combined\pruthr.c:397 16:37:51 INFO - #13 0x7fffe9afa5d4 in pr_root z:\build\build\src\nsprpub\pr\src\md\windows\w95thred.c:137 16:37:51 INFO - #14 0x7ffff9e10368 in o__strtoui64+0x58 (C:\Windows\System32\ucrtbase.dll+0x180020368) 16:37:51 INFO - #15 0x7fffeab5e4c8 in __asan::AsanThread::ThreadStart Z:\task_1533162563\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_thread.cc:262 16:37:51 INFO - #16 0x7ffff9f02773 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x180012773) 16:37:51 INFO - #17 0x7fffeaa875c3 in patched_BaseThreadInitThunk z:\build\build\src\mozglue\build\WindowsDllBlocklist.cpp:670 16:37:51 INFO - #18 0x7ffffca00d60 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x180070d60) 16:37:51 INFO - Thread T75 created by T0 here: 16:37:51 INFO - #0 0x7fffeab5f600 in __asan_wrap_CreateThread Z:\task_1533162563\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_win.cc:146 16:37:51 INFO - #1 0x7ffff9e0fde6 in beginthreadex+0x56 (C:\Windows\System32\ucrtbase.dll+0x18001fde6) 16:37:51 INFO - #2 0x7fffe9afa3fd in _PR_MD_CREATE_THREAD z:\build\build\src\nsprpub\pr\src\md\windows\w95thred.c:151 16:37:51 INFO - #3 0x7fffe9b2a9ad in _PR_NativeCreateThread z:\build\build\src\nsprpub\pr\src\threads\combined\pruthr.c:1044 16:37:51 INFO - #4 0x7fffe9b2b3e1 in _PR_CreateThread z:\build\build\src\nsprpub\pr\src\threads\combined\pruthr.c:1162 16:37:51 INFO - #5 0x7fffe9b2bf0f in PR_CreateThread z:\build\build\src\nsprpub\pr\src\threads\combined\pruthr.c:1374 16:37:51 INFO - #6 0x7fffc1597fb9 in nsThread::Init z:\build\build\src\xpcom\threads\nsThread.cpp:727 16:37:51 INFO - #7 0x7fffc15a3c48 in nsThreadManager::NewNamedThread z:\build\build\src\xpcom\threads\nsThreadManager.cpp:518 16:37:51 INFO - #8 0x7fffc15a774b in nsThreadPool::PutEvent z:\build\build\src\xpcom\threads\nsThreadPool.cpp:109 16:37:51 INFO - #9 0x7fffc15aa197 in nsThreadPool::Dispatch z:\build\build\src\xpcom\threads\nsThreadPool.cpp:280 16:37:51 INFO - #10 0x7fffc157632f in mozilla::SharedThreadPool::Dispatch z:\build\build\src\obj-firefox\dist\include\mozilla\SharedThreadPool.h:71 16:37:51 INFO - #11 0x7fffc8bde2bd in mozilla::AudioCallbackDriver::Start z:\build\build\src\dom\media\GraphDriver.cpp:764 16:37:51 INFO - #12 0x7fffc8e70848 in mozilla::MediaStreamGraphImpl::RunInStableState z:\build\build\src\dom\media\MediaStreamGraph.cpp:1803 16:37:51 INFO - #13 0x7fffc8e9ba75 in mozilla::`anonymous namespace'::MediaStreamGraphStableStateRunnable::Run z:\build\build\src\dom\media\MediaStreamGraph.cpp:1658 16:37:51 INFO - #14 0x7fffc13a10f0 in mozilla::CycleCollectedJSContext::ProcessStableStateQueue z:\build\build\src\xpcom\base\CycleCollectedJSContext.cpp:338 16:37:51 INFO - #15 0x7fffc13a39e6 in mozilla::CycleCollectedJSContext::AfterProcessTask z:\build\build\src\xpcom\base\CycleCollectedJSContext.cpp:403 16:37:51 INFO - #16 0x7fffc314f7bd in XPCJSContext::AfterProcessTask z:\build\build\src\js\xpconnect\src\XPCJSContext.cpp:1231 16:37:51 INFO - #17 0x7fffc159ce17 in nsThread::ProcessNextEvent z:\build\build\src\xpcom\threads\nsThread.cpp:1270 16:37:51 INFO - #18 0x7fffc15a4f7e in NS_ProcessNextEvent z:\build\build\src\xpcom\threads\nsThreadUtils.cpp:519 16:37:51 INFO - #19 0x7fffc25fc769 in mozilla::ipc::MessagePump::Run z:\build\build\src\ipc\glue\MessagePump.cpp:97 16:37:51 INFO - #20 0x7fffc255f29e in MessageLoop::RunHandler z:\build\build\src\ipc\chromium\src\base\message_loop.cc:318 16:37:51 INFO - #21 0x7fffc255f026 in MessageLoop::Run z:\build\build\src\ipc\chromium\src\base\message_loop.cc:298 16:37:51 INFO - #22 0x7fffca748e7a in nsBaseAppShell::Run z:\build\build\src\widget\nsBaseAppShell.cpp:158 16:37:51 INFO - #23 0x7fffca8cf997 in nsAppShell::Run z:\build\build\src\widget\windows\nsAppShell.cpp:415 16:37:51 INFO - #24 0x7fffcea46a2d in XRE_RunAppShell z:\build\build\src\toolkit\xre\nsEmbedFunctions.cpp:942 16:37:51 INFO - #25 0x7fffc255f29e in MessageLoop::RunHandler z:\build\build\src\ipc\chromium\src\base\message_loop.cc:318 16:37:51 INFO - #26 0x7fffc255f026 in MessageLoop::Run z:\build\build\src\ipc\chromium\src\base\message_loop.cc:298 16:37:51 INFO - #27 0x7fffcea45bb9 in XRE_InitChildProcess z:\build\build\src\toolkit\xre\nsEmbedFunctions.cpp:768 16:37:51 INFO - #28 0x7ff616972118 in Ordinal0+0x2118 (Z:\task_1533659308\build\application\firefox\firefox.exe+0x140002118) 16:37:51 INFO - #29 0x7ff616971529 in Ordinal0+0x1529 (Z:\task_1533659308\build\application\firefox\firefox.exe+0x140001529) 16:37:51 INFO - #30 0x7ff616a6b587 in TargetNtUnmapViewOfSection+0x27ce7 (Z:\task_1533659308\build\application\firefox\firefox.exe+0x1400fb587) 16:37:51 INFO - #31 0x7ffff9f02773 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x180012773) 16:37:51 INFO - #32 0x7ffffca00d60 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x180070d60) 16:37:51 INFO - SUMMARY: AddressSanitizer: bad-malloc_usable_size Z:\task_1533162563\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_thread.cc:262 in __asan::AsanThread::ThreadStart 16:37:51 INFO - ==7176==ABORTING

This might be a regression from bug 1476231, which switched from libav to ffmpeg. In each case, av_rdft_init was in a different dll from libxul. It is now in the same dll as may have been used for video decoding. This is the allocation site: https://searchfox.org/mozilla-central/rev/2466b82b729765fb0a3ab62f812c1a96a7362478/media/ffvpx/libavcodec/avfft.c#97 https://searchfox.org/mozilla-central/rev/2466b82b729765fb0a3ab62f812c1a96a7362478/dom/media/webaudio/FFTBlock.h#238 If we can believe the line number from mem.c, _aligned_malloc was used. https://bugzilla.mozilla.org/attachment.cgi?id=8992570&action=diff#a/media/libav/libavutil/mem.c_sec3 may be the best indication of differences between libav and ffmpeg versions. HAVE_AVX should be defined from https://searchfox.org/mozilla-central/rev/2466b82b729765fb0a3ab62f812c1a96a7362478/media/ffvpx/config_win64.asm#42 https://hg.mozilla.org/integration/autoland/rev/beddb157da49 suggests that CONFIG_MEMALIGN_HACK was not set with libav.

David, do you have any feel for whether this is likely a bug in our code or Asan itself?

Can someone CC me on the duplicate bug so that I can check for any additional clues?

I don't have a good reason to lean for or against this being an ASan-internal bug. On the one hand, both times the pointer is "located 32 bytes inside of 199-byte region" allocated by av_rdft_init, which does put suspicion on the media code. On the other hand, the site of the actual complaint is __asan::AsanThread::ThreadStart which smells of the stack problems in bug 1479385 and friends. Also, I don't see any calls to malloc_usable_size from that function, so maybe the stack is a lie. It would be helpful to get more data points or even better, a memory dump or repro.

Bug 1494181 indicates that the patch for bug 1477490 has not prevented this.

OK I think I've figured this out. The key piece of missing information was that this frame lacks symbols and is in fact ucrtbase!_aligned_malloc: 16:37:50 INFO - #1 0x7ffff9e0e251 in o_strncpy_s+0x91 (C:\Windows\System32\ucrtbase.dll+0x18001e251) av_rdft_init calls av_malloc(160) which in turn forwards to _aligned_malloc due to HAVE_ALIGNED_MALLOC: https://searchfox.org/mozilla-central/rev/ffe6eaf2f032e58ec3b0650a87df2c62ae4ca441/media/ffvpx/libavutil/mem.c#90 _aligned_malloc calls through to malloc with enough padding added to the size that it can achieve the desired alignmnent by offseting the result. But since av_malloc presents the post-offset value as a "malloc"-ed pointer, some code elsewhere (not clear where) tries to call malloc_usable_size on it, which is incorrect. As far as I can tell, there are two options: * Disable HAVE_ALIGNED_MALLOC to prevent the misrepresentation of this pointer * Find the code that calls malloc_usable_size and see if it can be avoided, or made aware of this trickery somehow. First that would require finding the call. I'm pretty sure it's not actually in __asan::AsanThread::ThreadStart and we're just seeing an incomplete stack walk there.

Also it's not clear if this is a problem in non-ASan builds. Maybe the malloc_usable_size call comes from ASan itself. Or maybe something didn't quite survive translation in the _msize/HeapSize interceptors. It's entirely possible, since I'm really not finding the call anywhere in our tree. We would really need a repro or time travel trace to be 100% sure. But maybe we should just disable HAVE_ALIGNED_MALLOC only for ASan builds?

Thank you very much for the hints, David. https://searchfox.org/mozilla-central/diff/032d3b87d73b4bf22a35948ac1f143e89c6108d8/media/ffvpx/libavutil/mem.c#64 would be what changed the message to "64 bytes inside of 231-byte region". dom/media/test/crashtests/1411322.html uses SpecialPowers.getMemoryReports(); Maximum sec-moderate based on "Client bugs that might have high or critical results but require the user perform unusual or complex actions to trigger." I don't know whether results would be high or not. It depends on the implementation of malloc_usable_size().

This probably dates back to https://hg.mozilla.org/mozilla-central/rev/248e6573f0810395ddfaec0d41b58b825ed40de4#l1.185 Marking geckoview unaffected because we don't yet set MOZ_LIBAV_FFT on arm.

Comment on attachment 9012445 [details] Bug 1481745 improve estimates of memory used by FFT contexts r?padenot Paul Adenot (:padenot) has approved the revision.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ba7d42459f512ac30bbfb8ed391a6607e99d4e8 https://hg.mozilla.org/mozilla-central/rev/4ba7d42459f5

Please nominate this for Beta/ESR60 approval when you get a chance.

I'm trying to assess the potential security impact of this bug. The bug is that malloc_usable_size() was being called on a pointer that did not necessarily come from malloc(). (The pointer is probably internal to an allocation from a dynamic library [outside libxul] but I suspect that may be irrelevant here.) The malloc_usable_size() call was made from within libxul and the function pointer is defined by MOZ_DEFINE_MALLOC_SIZE_OF at https://searchfox.org/mozilla-central/rev/819cd31a93fd50b7167979607371878c4d6f18e8/dom/media/MediaStreamGraphImpl.h#903 If that ends up using MozJemalloc::malloc_usable_size() then the pointer is validated and the symptoms will merely be that memory is not accounted correctly. Mike, do you know whether we ship any builds where MOZ_DEFINE_MALLOC_SIZE_OF will not use MozJemalloc::malloc_usable_size()? It does seem that ASAN builds override malloc_usable_size, but ASAN also seems to take care of security risks.

(In reply to Karl Tomlinson (:karlt) from comment #16) > I'm trying to assess the potential security impact of this bug. The bug is > that malloc_usable_size() was being called on a pointer that did not > necessarily come from malloc(). (The pointer is probably internal to an > allocation from a dynamic library [outside libxul] but I suspect that may > be irrelevant here.) That's not what comment 8 says. What seems to be happening is that the pointer is allocated with _aligned_malloc, which, in ASAN builds doesn't actually return a pointer to the beginning of an actual allocation. Then calling malloc_usable_size on that pointer doesn't work. I'll note that if asan builds are actively using malloc_usable_size, it means ASAN itself is providing it. As well as, presumably _aligned_malloc. If it doesn't provide _aligned_malloc, then there's a problem in ASAN, and it should. If it does, and malloc_usable_size doesn't handle its own allocations properly, then there's a problem in ASAN as well. > The malloc_usable_size() call was made from within libxul and the > function pointer is defined by MOZ_DEFINE_MALLOC_SIZE_OF at > https://searchfox.org/mozilla-central/rev/ > 819cd31a93fd50b7167979607371878c4d6f18e8/dom/media/MediaStreamGraphImpl.h#903 > > If that ends up using MozJemalloc::malloc_usable_size() then the pointer is > validated and the symptoms will merely be that memory is not accounted > correctly. This can't end up using MozJemalloc::* in ASAN builds, because ASAN builds have mozjemalloc disabled. > Mike, do you know whether we ship any builds where MOZ_DEFINE_MALLOC_SIZE_OF > will not use MozJemalloc::malloc_usable_size()? Any build where mozjemalloc is disabled, which is none of the normal builds. ASAN builds are an exception.

(In reply to Mike Hommey [:glandium] from comment #17) > If it doesn't provide > _aligned_malloc, then there's a problem in ASAN, and it should. Why? _aligned_malloc is a wrapper on top of malloc, which ASan already provides.

(In reply to David Major [:dmajor] from comment #18) > (In reply to Mike Hommey [:glandium] from comment #17) > > If it doesn't provide > > _aligned_malloc, then there's a problem in ASAN, and it should. > > Why? _aligned_malloc is a wrapper on top of malloc, which ASan already > provides. Because _aligned_malloc presumably doesn't return the pointer that it got from malloc. And using malloc_usable_size on that pointer would fail. ASan needs to handle that at some level. Also, because depending how things are linked, _aligned_malloc might actually be calling the CRT's malloc, not ASan's.

(In reply to Mike Hommey [:glandium] from comment #19) > Because _aligned_malloc presumably doesn't return the pointer that it got > from malloc. And using malloc_usable_size on that pointer would fail. ASan > needs to handle that at some level. _aligned_malloc and malloc_usable_size should not be mixed, with or without ASan. You're supposed to use _aligned_msize. > Also, because depending how things are linked, _aligned_malloc might > actually be calling the CRT's malloc, not ASan's. At least in the case I looked at, it's calling into ASan's malloc.

(In reply to David Major [:dmajor] from comment #20) > (In reply to Mike Hommey [:glandium] from comment #19) > > Because _aligned_malloc presumably doesn't return the pointer that it got > > from malloc. And using malloc_usable_size on that pointer would fail. ASan > > needs to handle that at some level. > > _aligned_malloc and malloc_usable_size should not be mixed, with or without > ASan. You're supposed to use _aligned_msize. Then that's arguably what the fix for this bug should have done. Now, one problem is that we don't have a wrapper for that for mozjemalloc (because, nothing actually uses it at the moment, which, considering how many calls there are to _aligned_malloc in the tree, is dubious).

Making assumptions about the source of pointers provided by third party libraries would be dubious. IIUC, MOZ_DEFINE_MALLOC_ENCLOSING_SIZE_OF() makes no such assumptions and so could be safely used here. I don't know whether or not that is preferable than a computed size. It produces a function that returns zero when MOZ_MEMORY is not defined. Even with the MOZ_MEMORY defined, the function would return zero if the DLL used a different heap. Is there reason why we should assume that the DLL uses the same heap? (The allocation associated with the pointer is very small and would not report most of the memory involved, and so this is not a major issue for this particular situation, but I assume there will be other cases of pointers associated with allocations from different DLLs.)

The allocations we're talking about here are from code in libxul.

How does the LGPL library use libxul to perform the allocations?

it uses libmozglue, like libxul does.

The bug is detected by only NT ASAN builds.

XP_WIN Gecko without MOZ_MEMORY uses _msize(), the docs for which are clear about requirements for the pointer: "The _msize function returns the size, in bytes, of the memory block allocated by a call to calloc, malloc, or realloc." Most likely, any potential security implications are limited to this non-shipping configuration on this platform. Linux Programmer's Manual says "The malloc_usable_size() function returns the number of usable bytes in the block pointed to by ptr, a pointer to a block of memory allocated by malloc(3) or a related function." malloc, free, calloc, realloc have the same man page in the Linux Programmer's Manual and so perhaps those are the "related function"s. https://www.gnu.org/software/libc/manual/html_node/Replacing-malloc.html also refers to those as "malloc-related", but that doesn't exclude others. |memptr| from posix_memalign can be passed to free, which may be a hint that posix_memalign is "related" to malloc. There was not consensus re malloc_usable_size with posix_memalign on https://github.com/google/sanitizers/issues/193 but asan was adjusted to accept this. XP_DARWIN Gecko uses malloc_size(ptr) instead of malloc_usable_size. The BSD Library Functions Manual does not make any explicit requirements re the ptr parameter beyond that it points to an "allocation". The name and "see also" field may imply an allocation from malloc (or related function). fwiw the same manual says the pointer from posix_memalign() "can be used as an argument in subsequent calls to realloc(3)".

Comment on attachment 9012445 [details] Bug 1481745 improve estimates of memory used by FFT contexts r?padenot The case for uplift is weak. Approval Request Comment [Feature/Bug causing the regression]: Bug 1157768. [User impact if declined]: The only shipped builds known to be affected are ASAN builds, which would safely crash when explicitly measuring memory use. Builds using --disable-jemalloc would be affected, with unknown consequences. Perhaps there are security implications, but these could only be triggered by an explicit user action (measuring memory use). [Is this code covered by automated tests?]: Intermittently. [Has the fix been verified in Nightly?]: No. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Limited to measuring memory use. [String changes made/needed]: None. [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Weak Fix Landed on Version: 64 Risk to taking this patch (and alternatives if risky): Risk is limited to measuring memory use.

https://searchfox.org/mozilla-central/rev/819cd31a93fd50b7167979607371878c4d6f18e8/build/docs/defining-binaries.rst#331 implies that SharedLibrary('mozavutil') does not use mozglue.

urgh

Kind of related: bug 1134914

Sounds like the security rating of this bug was set overly-high then? Can we maybe revise it to something more appropriate?

Comment on attachment 9013540 [details] Bug 1481745 exercise ConvolverNode memory reporting r?padenot Paul Adenot (:padenot) has approved the revision.

Ah, I see now we can drop it by one more level as an "unusual software configuration" for "mitigating circumstances that severely reduce the effectiveness of the exploit, then the exploit could be reduced by one level of severity."

Given the severity of the issue, let's just let this fix ride the trains.