1481745 - Intermittent 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

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P2)

Description

[Natalia Csoregi [:nataliaCs]]

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

Comment 2

[Karl Tomlinson (:karlt)]

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.

Comment 3

[Karl Tomlinson (:karlt)]

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

Comment 4

[(Away)]

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

Comment 5

[(Away)]

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.

Comment 7

[Karl Tomlinson (:karlt)]

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

Comment 8

[(Away)]

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.

Comment 9

[(Away)]

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?

Comment 10

[Karl Tomlinson (:karlt)]

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

Comment 11

[Karl Tomlinson (:karlt)]

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 12

[Karl Tomlinson (:karlt)]

Attachment: Bug 1481745 improve estimates of memory used by FFT contexts r?padenot

Comment 13

[Paul Adenot (:padenot)]

Comment on attachment 9012445 [details]
Bug 1481745 improve estimates of memory used by FFT contexts r?padenot

Paul Adenot (:padenot) has approved the revision.

Comment 14

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

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

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Comment 16

[Karl Tomlinson (:karlt)]

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.

Comment 17

[Mike Hommey [:glandium]]

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

Comment 18

[(Away)]

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

Comment 19

[Mike Hommey [:glandium]]

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

Comment 20

[(Away)]

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

Comment 21

[Mike Hommey [:glandium]]

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

Comment 22

[Karl Tomlinson (:karlt)]

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

Comment 23

[Mike Hommey [:glandium]]

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

Comment 24

[Karl Tomlinson (:karlt)]

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

Comment 25

[Mike Hommey [:glandium]]

it uses libmozglue, like libxul does.

Comment 26

[Karl Tomlinson (:karlt)]

Attachment: Bug 1481745 exercise ConvolverNode memory reporting r?padenot

The bug is detected by only NT ASAN builds.

Comment 27

[Karl Tomlinson (:karlt)]

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 28

[Karl Tomlinson (:karlt)]

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.

Comment 29

[Karl Tomlinson (:karlt)]

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

Comment 30

[Mike Hommey [:glandium]]

urgh

Comment 31

[Mike Hommey [:glandium]]

Kind of related: bug 1134914

Comment 32

[Ryan VanderMeulen [:RyanVM]]

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

Comment 33

[Paul Adenot (:padenot)]

Comment on attachment 9013540 [details]
Bug 1481745 exercise ConvolverNode memory reporting r?padenot

Paul Adenot (:padenot) has approved the revision.

Comment 34

[Karl Tomlinson (:karlt)]

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

Comment 35

[Ryan VanderMeulen [:RyanVM]]

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

Comment 36

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/mozilla-central/rev/93aafd0255fe