Closed Bug 1351094 Opened 7 years ago Closed 7 years ago

Stagefright: FPE crash in [@ stagefright::unitsToUs]

Categories

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

51 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox52 --- wontfix
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: tsmith, Assigned: mozbugz)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase)

Attachments

(4 files)

Attached file log.txt (deleted) —
Found with mozilla-central asan debug buildID=20170327212148

==59301==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000004edccf bp 0x7f25ed607490 sp 0x7f25ed607480 T55)
==59301==The signal is caused by a WRITE memory access.
==59301==Hint: address points to the zero page.
    #0 0x4edcce in mozalloc_abort(char const*) /home/worker/workspace/build/src/memory/mozalloc/mozalloc_abort.cpp:33:5
    #1 0x7f2612d0b9a5 in Abort(char const*) /home/worker/workspace/build/src/xpcom/base/nsDebugImpl.cpp:441:3
    #2 0x7f2612d0b67c in NS_DebugBreak /home/worker/workspace/build/src/xpcom/base/nsDebugImpl.cpp:428:7
    #3 0x7f261a89b8aa in fpehandler(int, siginfo*, void*) /home/worker/workspace/build/src/toolkit/xre/nsSigHandlers.cpp:156:5
    #4 0x7f26302a138f  (/lib/x86_64-linux-gnu/libpthread.so.0+0x1138f)
    #5 0x7f2612c5b664 in stagefright::unitsToUs(long, long) /home/worker/workspace/build/src/media/libstagefright/frameworks/av/media/libstagefright/MPEG4Extractor.cpp:55:32
    #6 0x7f2612c60f51 in stagefright::MPEG4Extractor::parseChunk(long*, int) /home/worker/workspace/build/src/media/libstagefright/frameworks/av/media/libstagefright/MPEG4Extractor.cpp:1847:35
    #7 0x7f2612c5eba7 in stagefright::MPEG4Extractor::parseChunk(long*, int) /home/worker/workspace/build/src/media/libstagefright/frameworks/av/media/libstagefright/MPEG4Extractor.cpp:807:32
    #8 0x7f2612c5eba7 in stagefright::MPEG4Extractor::parseChunk(long*, int) /home/worker/workspace/build/src/media/libstagefright/frameworks/av/media/libstagefright/MPEG4Extractor.cpp:807:32
    #9 0x7f2612c5c657 in stagefright::MPEG4Extractor::readMetaData() /home/worker/workspace/build/src/media/libstagefright/frameworks/av/media/libstagefright/MPEG4Extractor.cpp:407:15
    #10 0x7f2612c5c4c4 in stagefright::MPEG4Extractor::getMetaData() /home/worker/workspace/build/src/media/libstagefright/frameworks/av/media/libstagefright/MPEG4Extractor.cpp:342:16
    #11 0x7f2612c4c430 in mp4_demuxer::MP4MetadataStagefright::MP4MetadataStagefright(mp4_demuxer::Stream*) /home/worker/workspace/build/src/media/libstagefright/binding/MP4Metadata.cpp:491:47
    #12 0x7f2612c479ef in mozilla::detail::UniqueSelector<mp4_demuxer::MP4MetadataStagefright>::SingleObject mozilla::MakeUnique<mp4_demuxer::MP4MetadataStagefright, mp4_demuxer::Stream*&>(mp4_demuxer::Stream*&) /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:680:27
    #13 0x7f2612c4785f in mp4_demuxer::MP4Metadata::MP4Metadata(mp4_demuxer::Stream*) /home/worker/workspace/build/src/media/libstagefright/binding/MP4Metadata.cpp:231:17
    #14 0x7f26173b6d9b in mozilla::MP4Demuxer::Init() /home/worker/workspace/build/src/dom/media/fmp4/MP4Demuxer.cpp:138:28
...
see log.txt
Attached video test_case.mp4 (deleted) —
Assignee: nobody → twsmith
Flags: in-testsuite?
Assignee: twsmith → nobody
Div by 0; We should just add a simple `hz==0` test in unitsToUs.
Assignee: nobody → gsquelart
Comment on attachment 8851883 [details]
Bug 1351094 - Catch div/0 when hz==0 in MPEG4Extractor's unitsToUs -

https://reviewboard.mozilla.org/r/124116/#review126640
Attachment #8851883 - Flags: review?(ayang) → review+
Comment on attachment 8851884 [details]
Bug 1351094 - gtest -

https://reviewboard.mozilla.org/r/124118/#review126642
Attachment #8851884 - Flags: review?(ayang) → review+
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/95c849509574
Catch div/0 when hz==0 in MPEG4Extractor's unitsToUs - r=alfredo
https://hg.mozilla.org/integration/autoland/rev/bfbdd72a4445
gtest - r=alfredo
Thank you Alfredo for all the reviews today.
(In reply to Gerald Squelart [:gerald] from comment #9)
> Thank you Alfredo for all the reviews today.

No problem. :-)
https://hg.mozilla.org/mozilla-central/rev/95c849509574
https://hg.mozilla.org/mozilla-central/rev/bfbdd72a4445
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Is this something we should consider backporting to affected branches? AFAICT, this goes back to Fx51.
Blocks: 1301293
Flags: needinfo?(gsquelart)
Flags: in-testsuite?
Flags: in-testsuite+
Version: Trunk → 51 Branch
Sounds right.

Happy to request uplift, easy enough as the patch applies directly to esr52, beta, and aurora.
Flags: needinfo?(gsquelart)
Comment on attachment 8851883 [details]
Bug 1351094 - Catch div/0 when hz==0 in MPEG4Extractor's unitsToUs -

Uplifts request for *this* patch only (not the gtest).

Aurora 54, Beta 53:
Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: Possible crashes from malformed MP4 files.
[Is this code covered by automated tests?]: Yes, in Nightly.
[Has the fix been verified in Nightly?]: Yes, through new gtest with PoC test case.
[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?]: Just a 0-check with early error return.
[String changes made/needed]: None.

ESR 52:
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Possible crashes from malformed MP4 files.
Fix Landed on Version: 55.
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8851883 - Flags: approval-mozilla-esr52?
Attachment #8851883 - Flags: approval-mozilla-beta?
Attachment #8851883 - Flags: approval-mozilla-aurora?
Comment on attachment 8851883 [details]
Bug 1351094 - Catch div/0 when hz==0 in MPEG4Extractor's unitsToUs -

Fix a crash. Aurora54+ & Beta53+.
Attachment #8851883 - Flags: approval-mozilla-beta?
Attachment #8851883 - Flags: approval-mozilla-beta+
Attachment #8851883 - Flags: approval-mozilla-aurora?
Attachment #8851883 - Flags: approval-mozilla-aurora+
Comment on attachment 8851883 [details]
Bug 1351094 - Catch div/0 when hz==0 in MPEG4Extractor's unitsToUs -

crash fix for esr52
Attachment #8851883 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Setting qe-verify- based on Gerald's assessment on manual testing needs (see Comment 14) and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: