From 057131f23df3c40d8ee81fe5b160a6056408ff52 Mon Sep 17 00:00:00 2001

From: Wim Taymans <wtaymans@redhat.com>

Date: Tue, 12 Dec 2023 18:00:58 +0100

Subject: [PATCH] mxfdemux: Store GstMXFDemuxEssenceTrack in their own fixed

 allocation

Previously they were stored inline inside a GArray, but as references to

the tracks were stored in various other places although the array could

still be updated (and reallocated!), this could lead to dangling

references in various places.

Instead now store them in a GPtrArray in their own allocation so each

track's memory position stays fixed.

Fixes ZDI-CAN-22299

Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3055

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5638>

---

 gst/mxf/mxfdemux.c | 114 +++++++++++++++++++++------------------------

 gst/mxf/mxfdemux.h |   2 +-

 2 files changed, 53 insertions(+), 63 deletions(-)

diff --git a/gst/mxf/mxfdemux.c b/gst/mxf/mxfdemux.c

index 54ceec1df..6d76da9c1 100644

--- a/gst/mxf/mxfdemux.c

+++ b/gst/mxf/mxfdemux.c

@@ -153,10 +153,25 @@ gst_mxf_demux_partition_free (GstMXFDemuxPartition * partition)

 }

 static void

-gst_mxf_demux_reset_mxf_state (GstMXFDemux * demux)

+gst_mxf_demux_essence_track_free (GstMXFDemuxEssenceTrack * t)

 {

-  guint i;

+  if (t->offsets)

+    g_array_free (t->offsets, TRUE);

+

+  g_free (t->mapping_data);

+

+  if (t->tags)

+    gst_tag_list_unref (t->tags);

+

+  if (t->caps)

+    gst_caps_unref (t->caps);

+

+  g_free (t);

+}

+static void

+gst_mxf_demux_reset_mxf_state (GstMXFDemux * demux)

+{

   GST_DEBUG_OBJECT (demux, "Resetting MXF state");

   g_list_foreach (demux->partitions, (GFunc) gst_mxf_demux_partition_free,

@@ -166,22 +181,7 @@ gst_mxf_demux_reset_mxf_state (GstMXFDemux * demux)

   demux->current_partition = NULL;

-  for (i = 0; i < demux->essence_tracks->len; i++) {

-    GstMXFDemuxEssenceTrack *t =

-        &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

-

-    if (t->offsets)

-      g_array_free (t->offsets, TRUE);

-

-    g_free (t->mapping_data);

-

-    if (t->tags)

-      gst_tag_list_unref (t->tags);

-

-    if (t->caps)

-      gst_caps_unref (t->caps);

-  }

-  g_array_set_size (demux->essence_tracks, 0);

+  g_ptr_array_set_size (demux->essence_tracks, 0);

 }

 static void

@@ -199,7 +199,7 @@ gst_mxf_demux_reset_linked_metadata (GstMXFDemux * demux)

   for (i = 0; i < demux->essence_tracks->len; i++) {

     GstMXFDemuxEssenceTrack *track =

-        &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

+        g_ptr_array_index (demux->essence_tracks, i);

     track->source_package = NULL;

     track->source_track = NULL;

@@ -707,8 +707,7 @@ gst_mxf_demux_update_essence_tracks (GstMXFDemux * demux)

       for (k = 0; k < demux->essence_tracks->len; k++) {

         GstMXFDemuxEssenceTrack *tmp =

-            &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack,

-            k);

+            g_ptr_array_index (demux->essence_tracks, k);

         if (tmp->track_number == track->parent.track_number &&

             tmp->body_sid == edata->body_sid) {

@@ -726,24 +725,23 @@ gst_mxf_demux_update_essence_tracks (GstMXFDemux * demux)

       }

       if (!etrack) {

-        GstMXFDemuxEssenceTrack tmp;

+        GstMXFDemuxEssenceTrack *tmp = g_new0 (GstMXFDemuxEssenceTrack, 1);

-        memset (&tmp, 0, sizeof (tmp));

-        tmp.body_sid = edata->body_sid;

-        tmp.index_sid = edata->index_sid;

-        tmp.track_number = track->parent.track_number;

-        tmp.track_id = track->parent.track_id;

-        memcpy (&tmp.source_package_uid, &package->parent.package_uid, 32);

+        tmp->body_sid = edata->body_sid;

+        tmp->index_sid = edata->index_sid;

+        tmp->track_number = track->parent.track_number;

+        tmp->track_id = track->parent.track_id;

+        memcpy (&tmp->source_package_uid, &package->parent.package_uid, 32);

         if (demux->current_partition->partition.body_sid == edata->body_sid &&

             demux->current_partition->partition.body_offset == 0)

-          tmp.position = 0;

+          tmp->position = 0;

         else

-          tmp.position = -1;

+          tmp->position = -1;

-        g_array_append_val (demux->essence_tracks, tmp);

+        g_ptr_array_add (demux->essence_tracks, tmp);

         etrack =

-            &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack,

+            g_ptr_array_index (demux->essence_tracks,

             demux->essence_tracks->len - 1);

         new = TRUE;

       }

@@ -869,13 +867,7 @@ gst_mxf_demux_update_essence_tracks (GstMXFDemux * demux)

     next:

       if (new) {

-        g_free (etrack->mapping_data);

-        if (etrack->tags)

-          gst_tag_list_unref (etrack->tags);

-        if (etrack->caps)

-          gst_caps_unref (etrack->caps);

-

-        g_array_remove_index (demux->essence_tracks,

+        g_ptr_array_remove_index (demux->essence_tracks,

             demux->essence_tracks->len - 1);

       }

     }

@@ -888,7 +880,7 @@ gst_mxf_demux_update_essence_tracks (GstMXFDemux * demux)

   for (i = 0; i < demux->essence_tracks->len; i++) {

     GstMXFDemuxEssenceTrack *etrack =

-        &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

+        g_ptr_array_index (demux->essence_tracks, i);

     if (!etrack->source_package || !etrack->source_track || !etrack->caps) {

       GST_ERROR_OBJECT (demux, "Failed to update essence track %u", i);

@@ -1055,7 +1047,7 @@ gst_mxf_demux_update_tracks (GstMXFDemux * demux)

     for (k = 0; k < demux->essence_tracks->len; k++) {

       GstMXFDemuxEssenceTrack *tmp =

-          &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, k);

+          g_ptr_array_index (demux->essence_tracks, k);

       if (tmp->source_package == source_package &&

           tmp->source_track == source_track) {

@@ -1533,8 +1525,7 @@ gst_mxf_demux_pad_set_component (GstMXFDemux * demux, GstMXFDemuxPad * pad,

   pad->current_essence_track = NULL;

   for (k = 0; k < demux->essence_tracks->len; k++) {

-    GstMXFDemuxEssenceTrack *tmp =

-        &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, k);

+    GstMXFDemuxEssenceTrack *tmp = g_ptr_array_index (demux->essence_tracks, k);

     if (tmp->source_package == source_package &&

         tmp->source_track == source_track) {

@@ -1657,7 +1648,7 @@ gst_mxf_demux_handle_generic_container_essence_element (GstMXFDemux * demux,

   for (i = 0; i < demux->essence_tracks->len; i++) {

     GstMXFDemuxEssenceTrack *tmp =

-        &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

+          g_ptr_array_index (demux->essence_tracks, i);

     if (tmp->body_sid == demux->current_partition->partition.body_sid &&

         (tmp->track_number == track_number || tmp->track_number == 0)) {

@@ -2505,7 +2496,7 @@ gst_mxf_demux_handle_klv_packet (GstMXFDemux * demux, const MXFUL * key,

       for (i = 0; i < demux->essence_tracks->len; i++) {

         GstMXFDemuxEssenceTrack *etrack =

-            &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

+                g_ptr_array_index (demux->essence_tracks, i);

         if (etrack->body_sid != demux->current_partition->partition.body_sid)

           continue;

@@ -2568,7 +2559,7 @@ gst_mxf_demux_handle_klv_packet (GstMXFDemux * demux, const MXFUL * key,

       guint i;

       for (i = 0; i < demux->essence_tracks->len; i++) {

         GstMXFDemuxEssenceTrack *etrack =

-            &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

+            g_ptr_array_index (demux->essence_tracks, i);

         if (etrack->body_sid != demux->current_partition->partition.body_sid)

           continue;

@@ -2763,7 +2754,7 @@ from_index:

     for (i = 0; i < demux->essence_tracks->len; i++) {

       GstMXFDemuxEssenceTrack *t =

-          &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

+          g_ptr_array_index (demux->essence_tracks, i);

       if (index_start_position != -1 && t == etrack)

         t->position = index_start_position;

@@ -2786,8 +2777,7 @@ from_index:

       if (ret == GST_FLOW_EOS) {

         for (i = 0; i < demux->essence_tracks->len; i++) {

           GstMXFDemuxEssenceTrack *t =

-              &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack,

-              i);

+              g_ptr_array_index (demux->essence_tracks, i);

           if (t->position > 0)

             t->duration = t->position;

@@ -2869,7 +2859,7 @@ gst_mxf_demux_pull_and_handle_klv_packet (GstMXFDemux * demux)

     for (i = 0; i < demux->essence_tracks->len; i++) {

       GstMXFDemuxEssenceTrack *t =

-          &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

+          g_ptr_array_index (demux->essence_tracks, i);

       if (t->position > 0)

         t->duration = t->position;

@@ -3472,8 +3462,8 @@ gst_mxf_demux_seek_push (GstMXFDemux * demux, GstEvent * event)

   }

   for (i = 0; i < demux->essence_tracks->len; i++) {

-    GstMXFDemuxEssenceTrack *t =

-        &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

+    GstMXFDemuxEssenceTrack *t = g_ptr_array_index (demux->essence_tracks, i);

+

     t->position = -1;

   }

@@ -3808,8 +3798,8 @@ gst_mxf_demux_seek_pull (GstMXFDemux * demux, GstEvent * event)

   }

   for (i = 0; i < demux->essence_tracks->len; i++) {

-    GstMXFDemuxEssenceTrack *t =

-        &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

+    GstMXFDemuxEssenceTrack *t = g_ptr_array_index (demux->essence_tracks, i);

+

     t->position = -1;

   }

@@ -4097,7 +4087,7 @@ gst_mxf_demux_sink_event (GstPad * pad, GstObject * parent, GstEvent * event)

       for (i = 0; i < demux->essence_tracks->len; i++) {

         GstMXFDemuxEssenceTrack *t =

-            &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i);

+            g_ptr_array_index (demux->essence_tracks, i);

         if (t->position > 0)

           t->duration = t->position;

@@ -4138,8 +4128,8 @@ gst_mxf_demux_sink_event (GstPad * pad, GstObject * parent, GstEvent * event)

           for (i = 0; i < demux->essence_tracks->len; i++) {

             GstMXFDemuxEssenceTrack *etrack =

-                &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack,

-                i);

+                g_ptr_array_index (demux->essence_tracks, i);

+

             etrack->position = -1;

           }

           ret = TRUE;

@@ -4163,8 +4153,8 @@ gst_mxf_demux_sink_event (GstPad * pad, GstObject * parent, GstEvent * event)

       for (i = 0; i < demux->essence_tracks->len; i++) {

         GstMXFDemuxEssenceTrack *t =

-            &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack,

-            i);

+            g_ptr_array_index (demux->essence_tracks, i);

+

         t->position = -1;

       }

       demux->current_partition = NULL;

@@ -4435,7 +4425,7 @@ gst_mxf_demux_finalize (GObject * object)

   g_ptr_array_free (demux->src, TRUE);

   demux->src = NULL;

-  g_array_free (demux->essence_tracks, TRUE);

+  g_ptr_array_free (demux->essence_tracks, TRUE);

   demux->essence_tracks = NULL;

   g_hash_table_destroy (demux->metadata);

@@ -4512,8 +4502,8 @@ gst_mxf_demux_init (GstMXFDemux * demux)

   g_rw_lock_init (&demux->metadata_lock);

   demux->src = g_ptr_array_new ();

-  demux->essence_tracks =

-      g_array_new (FALSE, FALSE, sizeof (GstMXFDemuxEssenceTrack));

+  demux->essence_tracks = g_ptr_array_new_with_free_func ((GDestroyNotify)

+      gst_mxf_demux_essence_track_free);

   gst_segment_init (&demux->segment, GST_FORMAT_TIME);

diff --git a/gst/mxf/mxfdemux.h b/gst/mxf/mxfdemux.h

index b4d7242a8..9b13488b0 100644

--- a/gst/mxf/mxfdemux.h

+++ b/gst/mxf/mxfdemux.h

@@ -160,7 +160,7 @@ struct _GstMXFDemux

   GList *partitions;

   GstMXFDemuxPartition *current_partition;

-  GArray *essence_tracks;

+  GPtrArray *essence_tracks;

   GList *pending_index_table_segments;

   GList *index_tables; /* one per BodySID / IndexSID */

-- 

2.43.0