From 50c32a8963b450a7031a935b9a8aa29247843f2d Mon Sep 17 00:00:00 2001 From: Jakub Adam Date: Thu, 18 Mar 2021 20:13:21 +0100 Subject: [PATCH] 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: --- gst-libs/gst/rtp/gstrtpbasepayload.c | 5 +++++ gst-libs/gst/rtp/gstrtpbuffer.c | 10 +++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/gst-libs/gst/rtp/gstrtpbasepayload.c b/gst-libs/gst/rtp/gstrtpbasepayload.c index 8be9df3..dc4fdd9 100644 --- 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 --- 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); } -- 2.7.4