1351123 - Stagefright: Assertion failure in [@ mp4_demuxer::AnnexB::ConvertSPSOrPPS(mp4_demuxer::ByteReader &,unsigned char,mozilla::MediaByteBuffer *)]

Status: RESOLVED WONTFIX

(Core :: Audio/Video: Playback, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: log.txt

Assertion failure: false, at /home/worker/workspace/build/src/media/libstagefright/binding/include/mp4_demuxer/ByteReader.h:195

Found with mozilla-central asan debug buildID=20170327212148

Looks like this could possibly trigger an invalid read, marking s-s

==64853==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fe7117bf777 bp 0x7fe6dcd48110 sp 0x7fe6dcd48110 T54)
==64853==The signal is caused by a WRITE memory access.
==64853==Hint: address points to the zero page.
    #0 0x7fe7117bf776 in mp4_demuxer::ByteReader::PeekU8() /home/worker/workspace/build/src/media/libstagefright/binding/include/mp4_demuxer/ByteReader.h:195:7
    #1 0x7fe7117c7429 in mp4_demuxer::H264::DecodePPSDataSetFromExtraData(mozilla::MediaByteBuffer const*, AutoTArray<mp4_demuxer::SPSData, 32ul> const&, AutoTArray<mp4_demuxer::PPSData, 256ul>&) /home/worker/workspace/build/src/media/libstagefright/binding/H264.cpp:737:17
    #2 0x7fe7117c6d58 in mp4_demuxer::H264::DecodeSPSFromExtraData(mozilla::MediaByteBuffer const*, mp4_demuxer::SPSData&) /home/worker/workspace/build/src/media/libstagefright/binding/H264.cpp:616:7
    #3 0x7fe715f3e710 in mozilla::AccumulateSPSTelemetry(mozilla::MediaByteBuffer const*) /home/worker/workspace/build/src/dom/media/fmp4/MP4Demuxer.cpp:84:7
    #4 0x7fe715f402c3 in mozilla::MP4TrackDemuxer::MP4TrackDemuxer(mozilla::MP4Demuxer*, mozilla::UniquePtr<mozilla::TrackInfo, mozilla::DefaultDelete<mozilla::TrackInfo> >&&, mp4_demuxer::IndiceWrapper const&) /home/worker/workspace/build/src/dom/media/fmp4/MP4Demuxer.cpp:288:28
    #5 0x7fe715f3efe6 in mozilla::MP4Demuxer::Init() /home/worker/workspace/build/src/dom/media/fmp4/MP4Demuxer.cpp:173:35
    #6 0x7fe715ac41a5 in mozilla::MediaFormatReader::DemuxerProxy::Init()::$_10::operator()() const /home/worker/workspace/build/src/dom/media/MediaFormatReader.cpp:1008:47
...
see log.txt

Comment 1

[Tyson Smith [:tsmith]]

Attachment: test_case.mp4

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

It's just a `MOZ_ASSERT(false)`, which does `*((volatile int*) NULL) = line;` (i.e., writing the line number at 0x0) in debug builds to force a crash.
So this is not a sec issue.

The problem is in H264.cpp:737, reader->ReadU32() probably goes too far.
There doesn't seem to be any size checks before Peeks&Reads!

We need more `reader->Remaining() >= n` or `!reader->Peek(n)` checks.
Or functions like ReadU32() could return Maybe<...>. That would be much more work to rework all calls, but would give better peace of mind. (Alternatively, we could add ByteReader::TryU32(), so we can limit changes for now.)

Jean-Yves, you wrote most of this last year; any recommendations regarding solutions?

Comment 3

[Jean-Yves Avenard [:jya]]

Yes...

Remove all the PPS parsing related code, we don't use it.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Still reproducible on trunk.

Comment 5

[Jean-Yves Avenard [:jya]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #4)
> Still reproducible on trunk.

Doubtful it has the same stack trace, seeing that this code no longer exist...

DecodePPSDataSetFromExtraData Was completely removed.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Stack from m-c tip:

Assertion failure: false, at z:/build/build/src/media/libstagefright/binding/AnnexB.cpp:128
#01: mp4_demuxer::AnnexB::ConvertSPSOrPPS(mp4_demuxer::ByteReader &,unsigned char,mozilla::MediaByteBuffer *) [media/libstagefright/binding/AnnexB.cpp:128]
#02: mp4_demuxer::AnnexB::ConvertExtraDataToAnnexB(mozilla::MediaByteBuffer const *) [media/libstagefright/binding/AnnexB.cpp:111]
#03: mp4_demuxer::AnnexB::ConvertSampleToAnnexB(mozilla::MediaRawData *,bool) [media/libstagefright/binding/AnnexB.cpp:68]
#04: mozilla::H264Converter::Decode(mozilla::MediaRawData *) [dom/media/platforms/wrappers/H264Converter.cpp:123]
#05: mozilla::MediaFormatReader::DecoderFactory::Wrapper::Decode(mozilla::MediaRawData *) [dom/media/MediaFormatReader.cpp:625]
#06: mozilla::MediaFormatReader::DecodeDemuxedSamples(mozilla::TrackInfo::TrackType,mozilla::MediaRawData *) [dom/media/MediaFormatReader.cpp:2016]
#07: mozilla::MediaFormatReader::HandleDemuxedSamples(mozilla::TrackInfo::TrackType,mozilla::FrameStatistics::AutoNotifyDecoded &) [dom/media/MediaFormatReader.cpp:2132]
#08: mozilla::MediaFormatReader::Update(mozilla::TrackInfo::TrackType) [dom/media/MediaFormatReader.cpp:2492]
#09: mozilla::detail::RunnableMethodImpl<mozilla::MediaFormatReader * const,void ( mozilla::MediaFormatReader::*)(mozilla::TrackInfo::TrackType),1,0,mozilla::TrackInfo::TrackType>::Run() [xpcom/threads/nsThreadUtils.h:1195]
#10: mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() [xpcom/threads/TaskDispatcher.h:212]
#11: mozilla::TaskQueue::Runner::Run() [xpcom/threads/TaskQueue.cpp:247]
#12: nsThreadPool::Run() [xpcom/threads/nsThreadPool.cpp:230]
#13: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:1038]
#14: NS_ProcessNextEvent(nsIThread *,bool) [xpcom/threads/nsThreadUtils.cpp:524]
#15: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:338]
#16: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:326]
#17: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:320]
#18: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:300]
#19: nsThread::ThreadFunc(void *) [xpcom/threads/nsThread.cpp:427]
#20: _PR_NativeRunThread [nsprpub/pr/src/threads/combined/pruthr.c:397]
#21: pr_root [nsprpub/pr/src/md/windows/w95thred.c:95]
#22: ucrtbase.DLL + 0x3d5ef
#23: kernel32.dll + 0x53c45
#24: patched_BaseThreadInitThunk [mozglue/build/WindowsDllBlocklist.cpp:824]
#25: ntdll.dll + 0x637f5
#26: ntdll.dll + 0x637c8

Comment 7

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

libstagefright is gone.