rtpbuffer: make sure header extension buffer is initialized

Based upon valgrind finding:

Conditional jump or move depends on uninitialised value(s)
   at 0x4AFF589: read_rtp_header_extensions (gstrtpbasedepayload.c:1197)
   by 0x4AFF9E5: gst_rtp_base_depayload_set_headers
(gstrtpbasedepayload.c:1298)
   by 0x4AFFEE0: gst_rtp_base_depayload_do_push
(gstrtpbasedepayload.c:1413)
   by 0x4AFFF53: gst_rtp_base_depayload_push
(gstrtpbasedepayload.c:1448)
   by 0x4AFDEBA: gst_rtp_base_depayload_handle_buffer
(gstrtpbasedepayload.c:801)
   by 0x4AFE41E: gst_rtp_base_depayload_chain_list
(gstrtpbasedepayload.c:899)
   by 0x48F262C: gst_pad_chain_data_unchecked (gstpad.c:4414)
   by 0x48F3333: gst_pad_push_data (gstpad.c:4655)
   by 0x48F3DF8: gst_pad_push_list (gstpad.c:4814)
   by 0x4AFAD87: gst_rtp_base_payload_push_list
(gstrtpbasepayload.c:1978)
   by 0x72B3154: gst_rtp_vp8_pay_handle_buffer (gstrtpvp8pay.c:672)
   by 0x4AF7031: gst_rtp_base_payload_chain (gstrtpbasepayload.c:868)
 Uninitialised value was created by a heap allocation
   at 0x483C77F: malloc (in
/usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x4B8BA78: g_malloc (gmem.c:106)
   by 0x4BA3A9D: g_slice_alloc (gslice.c:1069)
   by 0x488D777: _sysmem_new_block (gstallocator.c:413)
   by 0x488DB28: default_alloc (gstallocator.c:512)
   by 0x488D3E8: gst_allocator_alloc (gstallocator.c:310)
   by 0x4AE97E3: gst_rtp_buffer_set_extension_data (gstrtpbuffer.c:856)
   by 0x4AF9EC6: set_headers (gstrtpbasepayload.c:1757)
   by 0x489FE4D: gst_buffer_list_foreach (gstbufferlist.c:287)
   by 0x4AFA87A: gst_rtp_base_payload_prepare_push
(gstrtpbasepayload.c:1915)
   by 0x4AFAD06: gst_rtp_base_payload_push_list
(gstrtpbasepayload.c:1970)
   by 0x72B3154: gst_rtp_vp8_pay_handle_buffer (gstrtpvp8pay.c:672)

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/1075>

diff --git a/gst-libs/gst/rtp/gstrtpbasepayload.c b/gst-libs/gst/rtp/gstrtpbasepayload.c
index 8be9df3..dc4fdd9 100644 (file)
--- a/gst-libs/gst/rtp/gstrtpbasepayload.c
+++ b/gst-libs/gst/rtp/gstrtpbasepayload.c
@@ -1765,6 +1765,11 @@ set_headers (GstBuffer ** buffer, guint idx, gpointer user_data)
         (GFunc) write_header_extension, &hdrext);
 
     wordlen = hdrext.written_size / 4 + ((hdrext.written_size % 4) ? 1 : 0);
+
+    /* zero-fill the hdrext padding bytes */
+    memset (&hdrext.data[hdrext.written_size], 0,
+        wordlen * 4 - hdrext.written_size);
+
     gst_rtp_buffer_set_extension_data (&rtp, bit_pattern, wordlen);
   }
   GST_OBJECT_UNLOCK (data->payload);

diff --git a/gst-libs/gst/rtp/gstrtpbuffer.c b/gst-libs/gst/rtp/gstrtpbuffer.c
index ae519f3..739b7d9 100644 (file)
--- a/gst-libs/gst/rtp/gstrtpbuffer.c
+++ b/gst-libs/gst/rtp/gstrtpbuffer.c
@@ -856,15 +856,23 @@ gst_rtp_buffer_set_extension_data (GstRTPBuffer * rtp, guint16 bits,
     mem = gst_allocator_alloc (NULL, min_size, NULL);
 
     if (rtp->data[1]) {
-      /* copy old data */
+      /* copy old data & initialize the remainder of the new buffer */
       gst_memory_map (mem, &map, GST_MAP_WRITE);
       memcpy (map.data, rtp->data[1], rtp->size[1]);
+      if (min_size > rtp->size[1]) {
+        memset (map.data + rtp->size[1], 0, min_size - rtp->size[1]);
+      }
       gst_memory_unmap (mem, &map);
 
       /* unmap old */
       gst_buffer_unmap (rtp->buffer, &rtp->map[1]);
       gst_buffer_replace_memory (rtp->buffer, 1, mem);
     } else {
+      /* don't leak data from uninitialized memory via the padding */
+      gst_memory_map (mem, &map, GST_MAP_WRITE);
+      memset (map.data, 0, map.size);
+      gst_memory_unmap (mem, &map);
+
       /* we didn't have extension data, add */
       gst_buffer_insert_memory (rtp->buffer, 1, mem);
     }