948326 - Division by 0 in mozilla::MP3FrameParser::GetDuration with gstreamer

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Oleg Romashin (:romaxa)]

Got crash in MP3FrameParser, mSamplesPerSecond == 0 and we divide on it.

#0  0x41a70440 in raise (sig=8) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:46
#1  0x41da955c in __aeabi_ldiv0 () at ../../../gcc/config/arm/lib1funcs.asm:1266
#2  0x47b63f10 in mozilla::MP3FrameParser::GetDuration (this=<optimized out>)
    at mozilla-central/content/media/MP3FrameParser.cpp:517
#3  0x47baf1c0 in NotifyDataArrived (aOffset=5167604888239372448, aLength=16384, aBuffer=0x462977d8 "", this=0x495f7608)
    at mozilla-central/content/media/gstreamer/GStreamerReader.cpp:1125
#4  mozilla::GStreamerReader::NotifyDataArrived (this=0x495f7608, aBuffer=0x462977d8 "", aLength=16384, aOffset=0)
    at mozilla-central/content/media/gstreamer/GStreamerReader.cpp:1109
#5  0x47b70544 in mozilla::MediaDecoderStateMachine::NotifyDataArrived (this=<optimized out>, aBuffer=<optimized out>, aLength=<optimized out>, aOffset=0)
    at mozilla-central/content/media/MediaDecoderStateMachine.cpp:1617
#6  0x47b6f894 in mozilla::ChannelMediaResource::CopySegmentToCache (aInStream=<optimized out>, aClosure=0x4c14e998, aFromSegment=0x462977d8 "", 
    aToOffset=<optimized out>, aCount=16384, aWriteCount=0x4c14e954)
    at mozilla-central/content/media/MediaResource.cpp:475
#7  0x471478c4 in nsInputStreamTee::WriteSegmentFun (in=<optimized out>, closure=<optimized out>, fromSegment=0x462977d8 "", offset=<optimized out>, 
    count=16384, writeCount=0x4c14e954) at mozilla-central/xpcom/io/nsInputStreamTee.cpp:200
#8  0x471499dc in nsPipeInputStream::ReadSegments (this=0x50612c40, 
    writer=0x471478a0 <nsInputStreamTee::WriteSegmentFun(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*)>, 
    closure=0x58e64f80, count=196608, readCount=0x4c14e9a4) at mozilla-central/xpcom/io/nsPipe3.cpp:781
#9  0x47144b80 in ReadSegments (
    bytesRead=0x47b6f858 <mozilla::ChannelMediaResource::CopySegmentToCache(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*)>,---Type <return> to continue, or q <return> to quit---
 count=<optimized out>, closure=<optimized out>, writer=<optimized out>, this=<optimized out>)
    at mozilla-central/xpcom/io/nsInputStreamTee.cpp:257
#10 nsInputStreamTee::ReadSegments (this=<optimized out>, writer=<optimized out>, closure=<optimized out>, count=<optimized out>, bytesRead=0x4c14e9a4)
    at mozilla-central/xpcom/io/nsInputStreamTee.cpp:246
#11 0x47b76f80 in mozilla::ChannelMediaResource::OnDataAvailable (this=<optimized out>, aRequest=<optimized out>, aStream=0x58e64f80, 
    aCount=<optimized out>) at mozilla-central/content/media/MediaResource.cpp:513
#12 0x47b088b8 in OnDataAvailable (aCount=0, aOffset=5123806530035838128, aStream=<optimized out>, aContext=<optimized out>, aRequest=<optimized out>, 
    this=<optimized out>) at mozilla-central/content/html/content/src/HTMLMediaElement.cpp:386
#13 mozilla::dom::HTMLMediaElement::MediaLoadListener::OnDataAvailable (this=<optimized out>, aRequest=<optimized out>, aContext=<optimized out>, 
    aStream=<optimized out>, aOffset=0, aCount=196608)
    at mozilla-central/content/html/content/src/HTMLMediaElement.cpp:376
#14 0x471b6ae4 in nsStreamListenerTee::OnDataAvailable (this=0x5aaf80c8, request=0x589cecb0, context=0x0, input=<optimized out>, offset=0, count=196608)
    at mozilla-central/netwerk/base/src/nsStreamListenerTee.cpp:93
#15 0x4723b300 in OnDataAvailable (count=0, input=0x50612c40, request=<optimized out>, this=0x589cec80, ctxt=<optimized out>, offset=<optimized out>)
    at mozilla-central/netwerk/protocol/http/nsHttpChannel.cpp:5303
#16 mozilla::net::nsHttpChannel::OnDataAvailable (this=0x589cec80, request=<optimized out>, ctxt=<optimized out>, input=0x50612c40, offset=0, count=196608)
    at mozilla-central/netwerk/protocol/http/nsHttpChannel.cpp:5235
#17 0x471acd10 in nsInputStreamPump::OnStateTransfer (this=0x5980efe0)
    at mozilla-central/netwerk/base/src/nsInputStreamPump.cpp:593
#18 0x471b089c in nsInputStreamPump::OnInputStreamReady (this=0x5980efe0, stream=<optimized out>)
---Type <return> to continue, or q <return> to quit---
    at mozilla-central/netwerk/base/src/nsInputStreamPump.cpp:434
#19 0x4714d8b4 in nsInputStreamReadyEvent::Run (this=<optimized out>)

Comment 1

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Oh, damn it.

This was mentioned in review comments in bug 918135 but I pushed an outdated patch without them. >_<

Comment 2

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 948326.patch

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8347859 [details] [diff] [review]
948326.patch

Review of attachment 8347859 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/MP3FrameParser.cpp
@@ +497,5 @@
>  int64_t MP3FrameParser::GetDuration()
>  {
>    MutexAutoLock mon(mLock);
>  
> +  if (!ParsedHeaders() || !mSamplesPerSecond) {

Still 0-check mFrameCount and mTotalFrameSize here, as the only thing enforcing that mFrameCount is non-zero is client's good behaviour, and you shouldn't rely on that. It's still useful to assert that client is behaving correctly however (before the zero checks obviously).

Comment 4

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f67feb33a974

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f67feb33a974

Comment 6

[Sotaro Ikeda [:sotaro]]

Edwin, this needs to be applied also to b2g v1.3 to fix Bug 965415.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dde493494cf6