From c87835a79ff31d8d9fe1f71726fb8d466136c21f Mon Sep 17 00:00:00 2001 From: Thomas Bluemel Date: Thu, 2 Oct 2014 10:37:57 -0600 Subject: [PATCH] hlsdemux: Fix accessing invalidated memory In gst_hls_demux_get_next_fragment() the next fragment URI gets stored in next_fragment_uri, but the gst_hls_demux_updates_loop() can at any time update the playlist, rendering this string invalid. Therefore, any data (like key, iv, URIs) that is taken from a GstM3U8Client needs to be copied. In addition, accessing the internals of a GstM3U8Client requires locking. https://bugzilla.gnome.org/show_bug.cgi?id=737793 --- ext/hls/gsthlsdemux.c | 107 ++++++++++++++++++++++++++++++++++---------------- ext/hls/gsthlsdemux.h | 4 +- ext/hls/m3u8.c | 47 ++++++++++++++-------- ext/hls/m3u8.h | 11 ++++-- 4 files changed, 113 insertions(+), 56 deletions(-) mode change 100644 => 100755 ext/hls/gsthlsdemux.c mode change 100644 => 100755 ext/hls/gsthlsdemux.h mode change 100644 => 100755 ext/hls/m3u8.c mode change 100644 => 100755 ext/hls/m3u8.h diff --git a/ext/hls/gsthlsdemux.c b/ext/hls/gsthlsdemux.c old mode 100644 new mode 100755 index e32858b..68144c1 --- a/ext/hls/gsthlsdemux.c +++ b/ext/hls/gsthlsdemux.c @@ -588,7 +588,7 @@ gst_hls_demux_sink_event (GstPad * pad, GstObject * parent, GstEvent * event) switch (event->type) { case GST_EVENT_EOS:{ - gchar *playlist = NULL; + gchar *uri, *playlist = NULL; if (demux->playlist == NULL) { GST_WARNING_OBJECT (demux, "Received EOS without a playlist."); @@ -602,7 +602,7 @@ gst_hls_demux_sink_event (GstPad * pad, GstObject * parent, GstEvent * event) ret = gst_pad_peer_query (demux->sinkpad, query); if (ret) { gboolean permanent; - gchar *uri, *redirect_uri; + gchar *redirect_uri; gst_query_parse_uri (query, &uri); gst_query_parse_uri_redirection (query, &redirect_uri); @@ -618,16 +618,17 @@ gst_hls_demux_sink_event (GstPad * pad, GstObject * parent, GstEvent * event) } gst_query_unref (query); + uri = gst_m3u8_client_get_uri (demux->client); gst_element_post_message (GST_ELEMENT_CAST (demux), gst_message_new_element (GST_OBJECT_CAST (demux), gst_structure_new (STATISTICS_MESSAGE_NAME, - "manifest-uri", G_TYPE_STRING, - gst_m3u8_client_get_uri (demux->client), "uri", G_TYPE_STRING, - gst_m3u8_client_get_uri (demux->client), + "manifest-uri", G_TYPE_STRING, uri, + "uri", G_TYPE_STRING, uri, "manifest-download-start", GST_TYPE_CLOCK_TIME, GST_CLOCK_TIME_NONE, "manifest-download-stop", GST_TYPE_CLOCK_TIME, gst_util_get_timestamp (), NULL))); + g_free (uri); playlist = gst_hls_src_buf_to_utf8_playlist (demux->playlist); demux->playlist = NULL; @@ -694,9 +695,14 @@ gst_hls_demux_src_query (GstPad * pad, GstObject * parent, GstQuery * query) } case GST_QUERY_URI: if (hlsdemux->client) { + gchar *uri; + /* FIXME: Do we answer with the variant playlist, with the current * playlist or the the uri of the least downlowaded fragment? */ - gst_query_set_uri (query, gst_m3u8_client_get_uri (hlsdemux->client)); + uri = gst_m3u8_client_get_uri (hlsdemux->client); + gst_query_set_uri (query, uri); + g_free (uri); + ret = TRUE; } break; @@ -1405,8 +1411,14 @@ gst_hls_demux_reset (GstHLSDemux * demux, gboolean dispose) if (demux->pending_buffer) gst_buffer_unref (demux->pending_buffer); demux->pending_buffer = NULL; - demux->current_key = NULL; - demux->current_iv = NULL; + if (demux->current_key) { + g_free (demux->current_key); + demux->current_key = NULL; + } + if (demux->current_iv) { + g_free (demux->current_iv); + demux->current_iv = NULL; + } gst_hls_demux_decrypt_end (demux); demux->current_download_rate = -1; @@ -1575,25 +1587,28 @@ gst_hls_demux_update_playlist (GstHLSDemux * demux, gboolean update, GstBuffer *buf; gchar *playlist; gboolean main_checked = FALSE, updated = FALSE; - const gchar *uri; + gchar *uri, *main_uri; retry: uri = gst_m3u8_client_get_current_uri (demux->client); + main_uri = gst_m3u8_client_get_uri (demux->client); download = gst_uri_downloader_fetch_uri (demux->downloader, uri, - demux->client->main ? demux->client->main->uri : NULL, TRUE, TRUE, TRUE, - err); + main_uri, TRUE, TRUE, TRUE, err); + g_free (main_uri); if (download == NULL) { if (update && !main_checked && gst_m3u8_client_has_variant_playlist (demux->client) - && demux->client->main) { + && gst_m3u8_client_has_main (demux->client)) { GError *err2 = NULL; + main_uri = gst_m3u8_client_get_uri (demux->client); GST_INFO_OBJECT (demux, "Updating playlist %s failed, attempt to refresh variant playlist %s", - uri, demux->client->main->uri); + uri, main_uri); download = gst_uri_downloader_fetch_uri (demux->downloader, - demux->client->main->uri, NULL, TRUE, TRUE, TRUE, &err2); + main_uri, NULL, TRUE, TRUE, TRUE, &err2); + g_free (main_uri); g_clear_error (&err2); if (download != NULL) { gchar *base_uri; @@ -1607,6 +1622,7 @@ retry: return FALSE; } + g_free (uri); if (download->redirect_permanent && download->redirect_uri) { uri = download->redirect_uri; base_uri = NULL; @@ -1627,13 +1643,17 @@ retry: main_checked = TRUE; goto retry; } else { + g_free (uri); return FALSE; } } else { + g_free (uri); return FALSE; } } + g_free (uri); + /* Set the base URI of the playlist to the redirect target if any */ GST_M3U8_CLIENT_LOCK (demux->client); g_free (demux->client->current->uri); @@ -1667,26 +1687,30 @@ retry: return FALSE; } + uri = gst_m3u8_client_get_current_uri(demux->client); + main_uri = gst_m3u8_client_get_uri (demux->client); gst_element_post_message (GST_ELEMENT_CAST (demux), gst_message_new_element (GST_OBJECT_CAST (demux), gst_structure_new (STATISTICS_MESSAGE_NAME, - "manifest-uri", G_TYPE_STRING, - gst_m3u8_client_get_uri (demux->client), "uri", G_TYPE_STRING, - gst_m3u8_client_get_current_uri (demux->client), + "manifest-uri", G_TYPE_STRING, main_uri, + "uri", G_TYPE_STRING, uri, "manifest-download-start", GST_TYPE_CLOCK_TIME, download->download_start_time, "manifest-download-stop", GST_TYPE_CLOCK_TIME, download->download_stop_time, NULL))); + g_free (main_uri); + g_free (uri); g_object_unref (download); + GST_M3U8_CLIENT_LOCK (demux->client); + /* If it's a live source, do not let the sequence number go beyond * three fragments before the end of the list */ if (update == FALSE && demux->client->current && - gst_m3u8_client_is_live (demux->client)) { + GST_M3U8_CLIENT_IS_LIVE (demux->client)) { gint64 last_sequence; - GST_M3U8_CLIENT_LOCK (demux->client); last_sequence = GST_M3U8_MEDIA_FILE (g_list_last (demux->client->current-> files)->data)->sequence; @@ -1697,8 +1721,7 @@ retry: demux->need_segment = TRUE; demux->client->sequence = last_sequence - 3; } - GST_M3U8_CLIENT_UNLOCK (demux->client); - } else if (demux->client->current && !gst_m3u8_client_is_live (demux->client)) { + } else if (demux->client->current && !GST_M3U8_CLIENT_IS_LIVE (demux->client)) { GstClockTime current_pos, target_pos; guint sequence = 0; GList *walk; @@ -1707,7 +1730,6 @@ retry: * playlists, so get the correct fragment here based on the current * position */ - GST_M3U8_CLIENT_LOCK (demux->client); current_pos = 0; target_pos = demux->segment.position; for (walk = demux->client->current->files; walk; walk = walk->next) { @@ -1725,9 +1747,10 @@ retry: sequence++; demux->client->sequence = sequence; demux->client->sequence_position = current_pos; - GST_M3U8_CLIENT_UNLOCK (demux->client); } + GST_M3U8_CLIENT_UNLOCK (demux->client); + return updated; } @@ -1769,13 +1792,17 @@ retry_failover_protection: demux->new_playlist = TRUE; if (gst_hls_demux_update_playlist (demux, FALSE, NULL)) { + gchar *uri, *main_uri; + uri = gst_m3u8_client_get_current_uri (demux->client); + main_uri = gst_m3u8_client_get_uri (demux->client); gst_element_post_message (GST_ELEMENT_CAST (demux), gst_message_new_element (GST_OBJECT_CAST (demux), gst_structure_new (STATISTICS_MESSAGE_NAME, - "manifest-uri", G_TYPE_STRING, - gst_m3u8_client_get_uri (demux->client), "uri", G_TYPE_STRING, - gst_m3u8_client_get_current_uri (demux->client), "bitrate", - G_TYPE_INT, new_bandwidth, NULL))); + "manifest-uri", G_TYPE_STRING, main_uri, + "uri", G_TYPE_STRING, uri, + "bitrate", G_TYPE_INT, new_bandwidth, NULL))); + g_free (uri); + g_free (main_uri); } else { GList *failover = NULL; @@ -2090,13 +2117,13 @@ static gboolean gst_hls_demux_get_next_fragment (GstHLSDemux * demux, gboolean * end_of_playlist, GError ** err) { - const gchar *next_fragment_uri; + gchar *uri, *next_fragment_uri; GstClockTime duration; GstClockTime timestamp; gboolean discont; gint64 range_start, range_end; - const gchar *key = NULL; - const guint8 *iv = NULL; + gchar *key = NULL; + guint8 *iv = NULL; GstClockTime download_start_time = 0; GstFlowReturn ret = GST_FLOW_OK; @@ -2120,7 +2147,11 @@ gst_hls_demux_get_next_fragment (GstHLSDemux * demux, demux->current_duration = duration; demux->starting_fragment = TRUE; demux->reset_crypto = TRUE; + if (demux->current_key) + g_free (demux->current_key); demux->current_key = key; + if (demux->current_iv) + g_free (demux->current_iv); demux->current_iv = iv; /* Reset last flow return */ @@ -2129,15 +2160,19 @@ gst_hls_demux_get_next_fragment (GstHLSDemux * demux, g_clear_error (&demux->last_error); g_mutex_unlock (&demux->fragment_download_lock); + GST_M3U8_CLIENT_LOCK (demux->client); if (!gst_hls_demux_update_source (demux, next_fragment_uri, demux->client->main ? demux->client->main->uri : NULL, FALSE, demux->client->current ? demux->client->current->allowcache : TRUE)) { + GST_M3U8_CLIENT_UNLOCK (demux->client); *err = g_error_new (GST_CORE_ERROR, GST_CORE_ERROR_MISSING_PLUGIN, "Missing plugin to handle URI: '%s'", next_fragment_uri); + g_free (next_fragment_uri); return FALSE; } + GST_M3U8_CLIENT_UNLOCK (demux->client); gst_hls_demux_configure_src_pad (demux); @@ -2155,6 +2190,7 @@ gst_hls_demux_get_next_fragment (GstHLSDemux * demux, "Source element can't handle range requests"); demux->last_ret = GST_FLOW_ERROR; g_mutex_unlock (&demux->fragment_download_lock); + g_free (next_fragment_uri); return FALSE; } } @@ -2207,23 +2243,26 @@ gst_hls_demux_get_next_fragment (GstHLSDemux * demux, if (demux->segment.rate > 0) demux->segment.position += demux->current_duration; + uri = gst_m3u8_client_get_uri (demux->client); gst_element_post_message (GST_ELEMENT_CAST (demux), gst_message_new_element (GST_OBJECT_CAST (demux), gst_structure_new (STATISTICS_MESSAGE_NAME, - "manifest-uri", G_TYPE_STRING, - gst_m3u8_client_get_uri (demux->client), "uri", G_TYPE_STRING, - next_fragment_uri, "fragment-start-time", - GST_TYPE_CLOCK_TIME, download_start_time, + "manifest-uri", G_TYPE_STRING, uri, + "uri", G_TYPE_STRING, next_fragment_uri, + "fragment-start-time", GST_TYPE_CLOCK_TIME, download_start_time, "fragment-stop-time", GST_TYPE_CLOCK_TIME, gst_util_get_timestamp (), "fragment-size", G_TYPE_UINT64, demux->download_total_bytes, "fragment-download-time", GST_TYPE_CLOCK_TIME, demux->download_total_time * GST_USECOND, NULL))); + g_free (uri); } /* clear the ghostpad eos state */ gst_hls_demux_stream_clear_eos_state (demux); + g_free (next_fragment_uri); + if (ret != GST_FLOW_OK) return FALSE; diff --git a/ext/hls/gsthlsdemux.h b/ext/hls/gsthlsdemux.h old mode 100644 new mode 100755 index 8241df1..4e790f6 --- a/ext/hls/gsthlsdemux.h +++ b/ext/hls/gsthlsdemux.h @@ -139,8 +139,8 @@ struct _GstHLSDemux #else gcry_cipher_hd_t aes_ctx; #endif - const gchar *current_key; - const guint8 *current_iv; + gchar *current_key; + guint8 *current_iv; GstAdapter *adapter; /* used to accumulate 16 bytes multiple chunks */ GstBuffer *pending_buffer; /* decryption scenario: * the last buffer can only be pushed when diff --git a/ext/hls/m3u8.c b/ext/hls/m3u8.c old mode 100644 new mode 100755 index 7fa9699..c737b3d --- a/ext/hls/m3u8.c +++ b/ext/hls/m3u8.c @@ -932,9 +932,9 @@ find_next_fragment (GstM3U8Client * client, GList * l, gboolean forward) gboolean gst_m3u8_client_get_next_fragment (GstM3U8Client * client, - gboolean * discontinuity, const gchar ** uri, GstClockTime * duration, + gboolean * discontinuity, gchar ** uri, GstClockTime * duration, GstClockTime * timestamp, gint64 * range_start, gint64 * range_end, - const gchar ** key, const guint8 ** iv, gboolean forward) + gchar ** key, guint8 ** iv, gboolean forward) { GList *l; GstM3U8MediaFile *file; @@ -964,7 +964,7 @@ gst_m3u8_client_get_next_fragment (GstM3U8Client * client, if (discontinuity) *discontinuity = client->sequence != file->sequence || file->discont; if (uri) - *uri = file->uri; + *uri = g_strdup (file->uri); if (duration) *duration = file->duration; if (range_start) @@ -972,9 +972,11 @@ gst_m3u8_client_get_next_fragment (GstM3U8Client * client, if (range_end) *range_end = file->size != -1 ? file->offset + file->size - 1 : -1; if (key) - *key = file->key; - if (iv) - *iv = file->iv; + *key = g_strdup (file->key); + if (iv) { + *iv = g_new (guint8, sizeof (file->iv)); + memcpy (*iv, file->iv, sizeof (file->iv)); + } client->sequence = file->sequence; @@ -1054,33 +1056,49 @@ gst_m3u8_client_get_target_duration (GstM3U8Client * client) return duration; } -const gchar * +gchar * gst_m3u8_client_get_uri (GstM3U8Client * client) { - const gchar *uri; + gchar *uri; g_return_val_if_fail (client != NULL, NULL); GST_M3U8_CLIENT_LOCK (client); - uri = client->main->uri; + uri = client->main ? g_strdup (client->main->uri) : NULL; GST_M3U8_CLIENT_UNLOCK (client); return uri; } -const gchar * +gchar * gst_m3u8_client_get_current_uri (GstM3U8Client * client) { - const gchar *uri; + gchar *uri; g_return_val_if_fail (client != NULL, NULL); GST_M3U8_CLIENT_LOCK (client); - uri = client->current->uri; + uri = g_strdup (client->current->uri); GST_M3U8_CLIENT_UNLOCK (client); return uri; } gboolean +gst_m3u8_client_has_main(GstM3U8Client * client) +{ + gboolean ret; + + g_return_val_if_fail (client != NULL, FALSE); + + GST_M3U8_CLIENT_LOCK (client); + if (client->main) + ret = TRUE; + else + ret = FALSE; + GST_M3U8_CLIENT_UNLOCK (client); + return ret; +} + +gboolean gst_m3u8_client_has_variant_playlist (GstM3U8Client * client) { gboolean ret; @@ -1101,10 +1119,7 @@ gst_m3u8_client_is_live (GstM3U8Client * client) g_return_val_if_fail (client != NULL, FALSE); GST_M3U8_CLIENT_LOCK (client); - if (!client->current || client->current->endlist) - ret = FALSE; - else - ret = TRUE; + ret = GST_M3U8_CLIENT_IS_LIVE (client); GST_M3U8_CLIENT_UNLOCK (client); return ret; } diff --git a/ext/hls/m3u8.h b/ext/hls/m3u8.h old mode 100644 new mode 100755 index 2344b22..77b42d5 --- a/ext/hls/m3u8.h +++ b/ext/hls/m3u8.h @@ -35,6 +35,8 @@ typedef struct _GstM3U8Client GstM3U8Client; #define GST_M3U8_CLIENT_LOCK(c) g_mutex_lock (&c->lock); #define GST_M3U8_CLIENT_UNLOCK(c) g_mutex_unlock (&c->lock); +#define GST_M3U8_CLIENT_IS_LIVE(c) ((!(c)->current || (c)->current->endlist) ? FALSE : TRUE) + struct _GstM3U8 { gchar *uri; /* actually downloaded URI */ @@ -94,14 +96,15 @@ gboolean gst_m3u8_client_update (GstM3U8Client * client, gchar * data); gboolean gst_m3u8_client_update_variant_playlist (GstM3U8Client * client, gchar * data, const gchar * uri, const gchar * base_uri); void gst_m3u8_client_set_current (GstM3U8Client * client, GstM3U8 * m3u8); gboolean gst_m3u8_client_get_next_fragment (GstM3U8Client * client, - gboolean * discontinuity, const gchar ** uri, GstClockTime * duration, + gboolean * discontinuity, gchar ** uri, GstClockTime * duration, GstClockTime * timestamp, gint64 * range_start, gint64 * range_end, - const gchar ** key, const guint8 ** iv, gboolean forward); + gchar ** key, guint8 ** iv, gboolean forward); void gst_m3u8_client_advance_fragment (GstM3U8Client * client, gboolean forward); GstClockTime gst_m3u8_client_get_duration (GstM3U8Client * client); GstClockTime gst_m3u8_client_get_target_duration (GstM3U8Client * client); -const gchar *gst_m3u8_client_get_uri(GstM3U8Client * client); -const gchar *gst_m3u8_client_get_current_uri(GstM3U8Client * client); +gchar *gst_m3u8_client_get_uri(GstM3U8Client * client); +gchar *gst_m3u8_client_get_current_uri(GstM3U8Client * client); +gboolean gst_m3u8_client_has_main(GstM3U8Client * client); gboolean gst_m3u8_client_has_variant_playlist(GstM3U8Client * client); gboolean gst_m3u8_client_is_live(GstM3U8Client * client); GList * gst_m3u8_client_get_playlist_for_bitrate (GstM3U8Client * client, -- 2.7.4