Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
xhaakon authored and GStreamer Marge Bot committed Apr 3, 2021
1 parent abd99b8 commit 50c32a8
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
5 changes: 5 additions & 0 deletions gst-libs/gst/rtp/gstrtpbasepayload.c
Expand Up @@ -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);
Expand Down
10 changes: 9 additions & 1 deletion gst-libs/gst/rtp/gstrtpbuffer.c
Expand Up @@ -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);
}
Expand Down

0 comments on commit 50c32a8

Please sign in to comment.