codecparsers: h264: fix memory leak in GstH264PPS.

The gst_h264_parse_pps() function dynamically allocates the slice
group ids map array, so that needs to be cleared before parsing a
new PPS NAL unit again, or when it is no longer needed.

Likewise, a clean copy to the internal NAL parser state needs to be
performed so that to avoid a double-free corruption.

https://bugzilla.gnome.org/show_bug.cgi?id=707282

Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>

5 files changed, 63 insertions, 1 deletions

diff --git a/docs/libs/gst-plugins-bad-libs-sections.txt b/docs/libs/gst-plugins-bad-libs-sections.txt
index 30b0b3b5b..eb98f7aeb 100644
--- a/docs/libs/gst-plugins-bad-libs-sections.txt
+++ b/docs/libs/gst-plugins-bad-libs-sections.txt
@@ -40,6 +40,7 @@ gst_h264_nal_parser_new
 gst_h264_nal_parser_free
 gst_h264_parse_sps
 gst_h264_parse_pps
+gst_h264_pps_clear
 gst_h264_quant_matrix_8x8_get_zigzag_from_raster
 gst_h264_quant_matrix_8x8_get_raster_from_zigzag
 gst_h264_quant_matrix_4x4_get_zigzag_from_raster
diff --git a/gst-libs/gst/codecparsers/gsth264parser.c b/gst-libs/gst/codecparsers/gsth264parser.c
index a0988e6e6..e5047229a 100644
--- a/gst-libs/gst/codecparsers/gsth264parser.c
+++ b/gst-libs/gst/codecparsers/gsth264parser.c
@@ -216,6 +216,32 @@ gst_h264_parse_nalu_header (GstH264NalUnit * nalu)
   return TRUE;
 }
 
+/*
+ * gst_h264_pps_copy:
+ * @dst_pps: The destination #GstH264PPS to copy into
+ * @src_pps: The source #GstH264PPS to copy from
+ *
+ * Copies @src_pps into @dst_pps.
+ *
+ * Returns: %TRUE if everything went fine, %FALSE otherwise
+ */
+static gboolean
+gst_h264_pps_copy (GstH264PPS * dst_pps, const GstH264PPS * src_pps)
+{
+  g_return_val_if_fail (dst_pps != NULL, FALSE);
+  g_return_val_if_fail (src_pps != NULL, FALSE);
+
+  gst_h264_pps_clear (dst_pps);
+
+  *dst_pps = *src_pps;
+
+  if (src_pps->slice_group_id)
+    dst_pps->slice_group_id = g_memdup (src_pps->slice_group_id,
+        src_pps->pic_size_in_map_units_minus1 + 1);
+
+  return TRUE;
+}
+
 /****** Parsing functions *****/
 
 static gboolean
@@ -985,6 +1011,10 @@ gst_h264_nal_parser_new (void)
 void
 gst_h264_nal_parser_free (GstH264NalParser * nalparser)
 {
+  guint i;
+
+  for (i = 0; i < GST_H264_MAX_PPS_COUNT; i++)
+    gst_h264_pps_clear (&nalparser->pps[i]);
   g_slice_free (GstH264NalParser, nalparser);
 
   nalparser = NULL;
@@ -1439,6 +1469,10 @@ error:
  *
  * Parses @data, and fills the @pps structure.
  *
+ * The resulting @pps data structure shall be deallocated with the
+ * gst_h264_pps_clear() function when it is no longer needed, or prior
+ * to parsing a new PPS NAL unit.
+ *
  * Returns: a #GstH264ParserResult
  */
 GstH264ParserResult
@@ -1559,6 +1593,7 @@ done:
 error:
   GST_WARNING ("error parsing \"Picture parameter set\"");
   pps->valid = FALSE;
+  gst_h264_pps_clear (pps);
   return GST_H264_PARSER_ERROR;
 }
 
@@ -1570,6 +1605,10 @@ error:
  *
  * Parses @data, and fills the @pps structure.
  *
+ * The resulting @pps data structure shall be deallocated with the
+ * gst_h264_pps_clear() function when it is no longer needed, or prior
+ * to parsing a new PPS NAL unit.
+ *
  * Returns: a #GstH264ParserResult
  */
 GstH264ParserResult
@@ -1581,7 +1620,8 @@ gst_h264_parser_parse_pps (GstH264NalParser * nalparser,
   if (res == GST_H264_PARSER_OK) {
     GST_DEBUG ("adding picture parameter set with id: %d to array", pps->id);
 
-    nalparser->pps[pps->id] = *pps;
+    if (!gst_h264_pps_copy (&nalparser->pps[pps->id], pps))
+      return GST_H264_PARSER_ERROR;
     nalparser->last_pps = &nalparser->pps[pps->id];
   }
 
@@ -1589,6 +1629,23 @@ gst_h264_parser_parse_pps (GstH264NalParser * nalparser,
 }
 
 /**
+ * gst_h264_pps_clear:
+ * @pps: The #GstH264PPS to free
+ *
+ * Clears all @pps internal resources.
+ *
+ * Since: 1.4
+ */
+void
+gst_h264_pps_clear (GstH264PPS * pps)
+{
+  g_return_if_fail (pps != NULL);
+
+  g_free (pps->slice_group_id);
+  pps->slice_group_id = NULL;
+}
+
+/**
  * gst_h264_parser_parse_slice_hdr:
  * @nalparser: a #GstH264NalParser
  * @nalu: The #GST_H264_NAL_SLICE #GstH264NalUnit to parse
diff --git a/gst-libs/gst/codecparsers/gsth264parser.h b/gst-libs/gst/codecparsers/gsth264parser.h
index 65d535b2b..35dd55850 100644
--- a/gst-libs/gst/codecparsers/gsth264parser.h
+++ b/gst-libs/gst/codecparsers/gsth264parser.h
@@ -781,6 +781,8 @@ GstH264ParserResult gst_h264_parse_sps                (GstH264NalUnit *nalu,
 GstH264ParserResult gst_h264_parse_pps                (GstH264NalParser *nalparser,
                                                        GstH264NalUnit *nalu, GstH264PPS *pps);
 
+void                gst_h264_pps_clear                (GstH264PPS *pps);
+
 void    gst_h264_quant_matrix_8x8_get_zigzag_from_raster (guint8 out_quant[64],
                                                           const guint8 quant[64]);
 
diff --git a/gst/videoparsers/gsth264parse.c b/gst/videoparsers/gsth264parse.c
index dbee6a6cb..c14784a6f 100644
--- a/gst/videoparsers/gsth264parse.c
+++ b/gst/videoparsers/gsth264parse.c
@@ -581,6 +581,7 @@ gst_h264_parse_process_nal (GstH264Parse * h264parse, GstH264NalUnit * nalu)
       }
 
       gst_h264_parser_store_nal (h264parse, pps.id, nal_type, nalu);
+      gst_h264_pps_clear (&pps);
       break;
     case GST_H264_NAL_SEI:
       gst_h264_parse_process_sei (h264parse, nalu);
diff --git a/win32/common/libgstcodecparsers.def b/win32/common/libgstcodecparsers.def
index 45df377b1..31dc975d3 100644
--- a/win32/common/libgstcodecparsers.def
+++ b/win32/common/libgstcodecparsers.def
@@ -15,6 +15,7 @@ EXPORTS
 	gst_h264_parser_parse_sei
 	gst_h264_parser_parse_slice_hdr
 	gst_h264_parser_parse_sps
+	gst_h264_pps_clear
 	gst_h264_quant_matrix_4x4_get_raster_from_zigzag
 	gst_h264_quant_matrix_4x4_get_zigzag_from_raster
 	gst_h264_quant_matrix_8x8_get_raster_from_zigzag