Changeset 211495 in webkit

Timestamp:

Feb 1, 2017 10:22:21 AM (7 years ago)

Author:

jer.noble@apple.com

Message:

NULL-deref crash in TextTrack::removeCue()
​https://bugs.webkit.org/show_bug.cgi?id=167615

Reviewed by Eric Carlson.

Source/WebCore:

Test: http/tests/media/track-in-band-hls-metadata-crash.html

Follow-up to r211401. When passing around a reference to an object, the assumption is that
the caller is retaining the underlying object. This breaks down for
InbandDataTextTrack::removeDataCue(), which releases its own ownership of the cue object,
then passes the reference to that object to its superclass to do further remove steps. The
retain count of the cue can thus drop to zero within the scope of
InbandTextTrack::removeCue(). Use "take" semantics to remove the cue from the
m_incompleteCueMap without releasing ownership, and pass a reference to that retained object
on to removeCue(), guaranteeing that the cue will not be destroyed until after the
romeveDataCue() method returns.

• html/track/InbandDataTextTrack.cpp:

(WebCore::InbandDataTextTrack::removeDataCue):

LayoutTests:

• http/tests/media/track-in-band-hls-metadata-crash-expected.txt: Added.
• http/tests/media/track-in-band-hls-metadata-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/media/track-in-band-hls-metadata-crash-expected.txt (added)
• LayoutTests/http/tests/media/track-in-band-hls-metadata-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/track/InbandDataTextTrack.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-02-01  Jer Noble  <jer.noble@apple.com>
+
+        NULL-deref crash in TextTrack::removeCue()
+        https://bugs.webkit.org/show_bug.cgi?id=167615
+
+        Reviewed by Eric Carlson.
+
+        * http/tests/media/track-in-band-hls-metadata-crash-expected.txt: Added.
+        * http/tests/media/track-in-band-hls-metadata-crash.html: Added.
+
 2017-02-01  Nan Wang  <n_wang@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-02-01  Jer Noble  <jer.noble@apple.com>
+
+        NULL-deref crash in TextTrack::removeCue()
+        https://bugs.webkit.org/show_bug.cgi?id=167615
+
+        Reviewed by Eric Carlson.
+
+        Test: http/tests/media/track-in-band-hls-metadata-crash.html
+
+        Follow-up to r211401. When passing around a reference to an object, the assumption is that
+        the caller is retaining the underlying object. This breaks down for
+        InbandDataTextTrack::removeDataCue(), which releases its own ownership of the cue object,
+        then passes the reference to that object to its superclass to do further remove steps. The
+        retain count of the cue can thus drop to zero within the scope of
+        InbandTextTrack::removeCue(). Use "take" semantics to remove the cue from the
+        m_incompleteCueMap without releasing ownership, and pass a reference to that retained object
+        on to removeCue(), guaranteeing that the cue will not be destroyed until after the
+        romeveDataCue() method returns.
+
+        * html/track/InbandDataTextTrack.cpp:
+        (WebCore::InbandDataTextTrack::removeDataCue):
+
 2017-02-01  Nan Wang  <n_wang@apple.com>
 

trunk/Source/WebCore/html/track/InbandDataTextTrack.cpp

@@ -101 +101 @@
 void InbandDataTextTrack::removeDataCue(const MediaTime&, const MediaTime&, SerializedPlatformRepresentation& platformValue)
 {
-    if (auto* cue = m_incompleteCueMap.get(&platformValue)) {
+    if (auto cue = m_incompleteCueMap.take(&platformValue)) {
         LOG(Media, "InbandDataTextTrack::removeDataCue removing cue: start=%s, end=%s\n", toString(cue->startTime()).utf8().data(), toString(cue->endTime()).utf8().data());
-        removeCue(*cue);
+        InbandTextTrack::removeCue(*cue);
     }
 }