Title: [211495] trunk
Revision
211495
Author
jer.no...@apple.com
Date
2017-02-01 10:22:21 -0800 (Wed, 01 Feb 2017)

Log 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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (211494 => 211495)


--- trunk/LayoutTests/ChangeLog	2017-02-01 18:20:41 UTC (rev 211494)
+++ trunk/LayoutTests/ChangeLog	2017-02-01 18:22:21 UTC (rev 211495)
@@ -1,3 +1,13 @@
+2017-02-01  Jer Noble  <jer.no...@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_w...@apple.com>
 
         AX: Incorrect range from index and length in text controls when there are newlines

Added: trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash-expected.txt (0 => 211495)


--- trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash-expected.txt	2017-02-01 18:22:21 UTC (rev 211495)
@@ -0,0 +1,18 @@
+
+Test that seeking HLS streams containing metadata tracks does not crash.
+
+
+** Set video.src, wait for media data to load
+RUN(video.src = '')
+
+EVENT(addtrack)
+RUN(track = video.textTracks[0])
+RUN(track.mode = 'hidden')
+RUN(video.play())
+EVENT(cuechange)
+
+** Seek, should not crash.
+RUN(video.currentTime = 5)
+EVENT(seeked)
+END OF TEST
+

Added: trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash.html (0 => 211495)


--- trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash.html	2017-02-01 18:22:21 UTC (rev 211495)
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
+
+        <script src=""
+        <script src=""
+
+        <script>
+            var track;
+
+            function addtrack(event)
+            {
+                tracks = event.target;
+                run("track = video.textTracks[0]");
+                run("track.mode = 'hidden'");
+                run("video.play()");
+                waitForEvent('cuechange', cuechange, false, true, track);
+            }
+
+            function cuechange()
+            {
+                consoleWrite("<br><em>** Seek, should not crash.</em>");
+                run("video.currentTime = 5"); 
+                waitForEventAndEnd("seeked");
+            }
+
+            function start()
+            {
+                consoleWrite("<br><em>** Set video.src, wait for media data to load</em>");
+                findMediaElement();
+                run("video.src = ''");
+
+                consoleWrite("");
+                waitForEvent('addtrack', addtrack, false, true, video.textTracks);
+            }
+        </script>
+    </head>
+    <body _onload_="start()">
+        <video controls></video>
+        <p>Test that seeking HLS streams containing metadata tracks does not crash.</p>
+    </body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (211494 => 211495)


--- trunk/Source/WebCore/ChangeLog	2017-02-01 18:20:41 UTC (rev 211494)
+++ trunk/Source/WebCore/ChangeLog	2017-02-01 18:22:21 UTC (rev 211495)
@@ -1,3 +1,25 @@
+2017-02-01  Jer Noble  <jer.no...@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_w...@apple.com>
 
         AX: Incorrect range from index and length in text controls when there are newlines

Modified: trunk/Source/WebCore/html/track/InbandDataTextTrack.cpp (211494 => 211495)


--- trunk/Source/WebCore/html/track/InbandDataTextTrack.cpp	2017-02-01 18:20:41 UTC (rev 211494)
+++ trunk/Source/WebCore/html/track/InbandDataTextTrack.cpp	2017-02-01 18:22:21 UTC (rev 211495)
@@ -100,9 +100,9 @@
 
 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);
     }
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to