Bug 1334445 - Abort AnnexB parsing on ByteWriter failure. r=jya, a=jcristau
authorGerald Squelart <gsquelart@mozilla.com>
Wed, 01 Feb 2017 19:17:29 +1100
changeset 359000 4803d9443a778cfc4ba0c77b64feea46f5329c42
parent 358999 cefad960ed80981dd2773e7bb0f95dd9765b51c0
child 359001 514a5268030faf927430313a96a9e470e5c14c11
push id10696
push userryanvm@gmail.com
push dateTue, 07 Feb 2017 16:37:44 +0000
treeherdermozilla-aurora@514a5268030f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya, jcristau
bugs1334445
milestone53.0a2
Bug 1334445 - Abort AnnexB parsing on ByteWriter failure. r=jya, a=jcristau All writes through a ByteWriter are now checked for success. Possible failures could be in three classes: A. Attempt to write a single unreasonably-large block of data. In this case we just don't allocate more memory, so Firefox should keep functioning but simply refuse to play this media. B. Accumulation of a lot of data into a single vector. In this case we return from the function, which should immediately free the large vector, and hopefully allow Firefox to continue unscathed; but there is the possibility that other memory allocations elsewhere will fail in the meantime. C. OOM across all of Firefox, and this write is just the final drop. Like previously we will still try to return and deallocate some memory, but this may not be enough to recover. So even though this patch cannot fix all possible OOM causes, I think it is worth adding, as it is simple enough, and will help in some cases where corrupted data could sensibly be handled without making Firefox crash. MozReview-Commit-ID: JPVC9hao4Uq
media/libstagefright/binding/AnnexB.cpp
media/libstagefright/binding/include/mp4_demuxer/ByteWriter.h
--- a/media/libstagefright/binding/AnnexB.cpp
+++ b/media/libstagefright/binding/AnnexB.cpp
@@ -40,21 +40,25 @@ AnnexB::ConvertSampleToAnnexB(mozilla::M
 
   mozilla::Vector<uint8_t> tmp;
   ByteWriter writer(tmp);
 
   while (reader.Remaining() >= 4) {
     uint32_t nalLen = reader.ReadU32();
     const uint8_t* p = reader.Read(nalLen);
 
-    writer.Write(kAnnexBDelimiter, ArrayLength(kAnnexBDelimiter));
+    if (!writer.Write(kAnnexBDelimiter, ArrayLength(kAnnexBDelimiter))) {
+      return false;
+    }
     if (!p) {
       break;
     }
-    writer.Write(p, nalLen);
+    if (!writer.Write(p, nalLen)) {
+      return false;
+    }
   }
 
   nsAutoPtr<MediaRawDataWriter> samplewriter(aSample->CreateWriter());
 
   if (!samplewriter->Replace(tmp.begin(), tmp.length())) {
     return false;
   }
 
@@ -183,39 +187,44 @@ FindStartCode(ByteReader& aBr, size_t& a
     if (aBr.ReadU8() == 0) {
       aStartSize = 4;
     }
   }
   aBr.Read(3);
   return true;
 }
 
-static void
+static bool
 ParseNALUnits(ByteWriter& aBw, ByteReader& aBr)
 {
   size_t startSize;
 
   bool rv = FindStartCode(aBr, startSize);
   if (rv) {
     size_t startOffset = aBr.Offset();
     while (FindStartCode(aBr, startSize)) {
       size_t offset = aBr.Offset();
       size_t sizeNAL = offset - startOffset - startSize;
       aBr.Seek(startOffset);
-      aBw.WriteU32(sizeNAL);
-      aBw.Write(aBr.Read(sizeNAL), sizeNAL);
+      if (!aBw.WriteU32(sizeNAL)
+          || !aBw.Write(aBr.Read(sizeNAL), sizeNAL)) {
+        return false;
+      }
       aBr.Read(startSize);
       startOffset = offset;
     }
   }
   size_t sizeNAL = aBr.Remaining();
   if (sizeNAL) {
-    aBw.WriteU32(sizeNAL);
-    aBw.Write(aBr.Read(sizeNAL), sizeNAL);
+    if (!aBw.WriteU32(sizeNAL)
+        || !aBw.Write(aBr.Read(sizeNAL), sizeNAL)) {
+      return false;
+    }
   }
+  return true;
 }
 
 bool
 AnnexB::ConvertSampleToAVCC(mozilla::MediaRawData* aSample)
 {
   if (IsAVCC(aSample)) {
     return ConvertSampleTo4BytesAVCC(aSample);
   }
@@ -223,17 +232,19 @@ AnnexB::ConvertSampleToAVCC(mozilla::Med
     // Not AnnexB, nothing to convert.
     return true;
   }
 
   mozilla::Vector<uint8_t> nalu;
   ByteWriter writer(nalu);
   ByteReader reader(aSample->Data(), aSample->Size());
 
-  ParseNALUnits(writer, reader);
+  if (!ParseNALUnits(writer, reader)) {
+    return false;
+  }
   nsAutoPtr<MediaRawDataWriter> samplewriter(aSample->CreateWriter());
   return samplewriter->Replace(nalu.begin(), nalu.length());
 }
 
 already_AddRefed<mozilla::MediaByteBuffer>
 AnnexB::ExtractExtraData(const mozilla::MediaRawData* aSample)
 {
   RefPtr<mozilla::MediaByteBuffer> extradata = new mozilla::MediaByteBuffer;
@@ -279,22 +290,26 @@ AnnexB::ExtractExtraData(const mozilla::
     uint8_t nalType = reader.PeekU8() & 0x1f;
     const uint8_t* p = reader.Read(nalLen);
     if (!p) {
       return extradata.forget();
     }
 
     if (nalType == 0x7) { /* SPS */
       numSps++;
-      spsw.WriteU16(nalLen);
-      spsw.Write(p, nalLen);
+      if (!spsw.WriteU16(nalLen)
+          || !spsw.Write(p, nalLen)) {
+        return extradata.forget();
+      }
     } else if (nalType == 0x8) { /* PPS */
       numPps++;
-      ppsw.WriteU16(nalLen);
-      ppsw.Write(p, nalLen);
+      if (!ppsw.WriteU16(nalLen)
+          || !ppsw.Write(p, nalLen)) {
+        return extradata.forget();
+      }
     }
   }
 
   if (numSps && sps.length() > 5) {
     extradata->AppendElement(1);        // version
     extradata->AppendElement(sps[3]);   // profile
     extradata->AppendElement(sps[4]);   // profile compat
     extradata->AppendElement(sps[5]);   // level
@@ -394,18 +409,20 @@ AnnexB::ConvertSampleTo4BytesAVCC(mozill
       case 2: nalLen = reader.ReadU16(); break;
       case 3: nalLen = reader.ReadU24(); break;
       case 4: nalLen = reader.ReadU32(); break;
     }
     const uint8_t* p = reader.Read(nalLen);
     if (!p) {
       return true;
     }
-    writer.WriteU32(nalLen);
-    writer.Write(p, nalLen);
+    if (!writer.WriteU32(nalLen)
+        || !writer.Write(p, nalLen)) {
+      return false;
+    }
   }
   nsAutoPtr<MediaRawDataWriter> samplewriter(aSample->CreateWriter());
   return samplewriter->Replace(dest.begin(), dest.length());
 }
 
 bool
 AnnexB::IsAVCC(const mozilla::MediaRawData* aSample)
 {
--- a/media/libstagefright/binding/include/mp4_demuxer/ByteWriter.h
+++ b/media/libstagefright/binding/include/mp4_demuxer/ByteWriter.h
@@ -17,59 +17,59 @@ public:
   explicit ByteWriter(mozilla::Vector<uint8_t>& aData)
     : mPtr(aData)
   {
   }
   ~ByteWriter()
   {
   }
 
-  void WriteU8(uint8_t aByte)
+  MOZ_MUST_USE bool WriteU8(uint8_t aByte)
   {
-    Write(&aByte, 1);
+    return Write(&aByte, 1);
   }
 
-  void WriteU16(uint16_t aShort)
+  MOZ_MUST_USE bool WriteU16(uint16_t aShort)
   {
     uint8_t c[2];
     mozilla::BigEndian::writeUint16(&c[0], aShort);
-    Write(&c[0], 2);
+    return Write(&c[0], 2);
   }
 
-  void WriteU32(uint32_t aLong)
+  MOZ_MUST_USE bool WriteU32(uint32_t aLong)
   {
     uint8_t c[4];
     mozilla::BigEndian::writeUint32(&c[0], aLong);
-    Write(&c[0], 4);
+    return Write(&c[0], 4);
   }
 
-  void Write32(int32_t aLong)
+  MOZ_MUST_USE bool Write32(int32_t aLong)
   {
     uint8_t c[4];
     mozilla::BigEndian::writeInt32(&c[0], aLong);
-    Write(&c[0], 4);
+    return Write(&c[0], 4);
   }
 
-  void WriteU64(uint64_t aLongLong)
+  MOZ_MUST_USE bool WriteU64(uint64_t aLongLong)
   {
     uint8_t c[8];
     mozilla::BigEndian::writeUint64(&c[0], aLongLong);
-    Write(&c[0], 8);
+    return Write(&c[0], 8);
   }
 
-  void Write64(int64_t aLongLong)
+  MOZ_MUST_USE bool Write64(int64_t aLongLong)
   {
     uint8_t c[8];
     mozilla::BigEndian::writeInt64(&c[0], aLongLong);
-    Write(&c[0], 8);
+    return Write(&c[0], 8);
   }
 
-  void Write(const uint8_t* aSrc, size_t aCount)
+  MOZ_MUST_USE bool Write(const uint8_t* aSrc, size_t aCount)
   {
-    MOZ_RELEASE_ASSERT(mPtr.append(aSrc, aCount));
+    return mPtr.append(aSrc, aCount);
   }
 
 private:
   mozilla::Vector<uint8_t>& mPtr;
 };
 
 } // namespace mp4_demuxer