Bug 780040 – rtph264depay: segfault if empty sprop-parameters are provided in caps

After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.

Bug 780040 - rtph264depay: segfault if empty sprop-parameters are provided in caps


Summary:	rtph264depay: segfault if empty sprop-parameters are provided in caps


Status:	RESOLVED FIXED

Product:	GStreamer
Classification:	Platform
Component:	gst-plugins-good
Version:	1.x
Hardware:	Other Linux

Importance:	Normal critical
Target Milestone:	1.10.5
Assigned To:	GStreamer Maintainers
QA Contact:	GStreamer Maintainers

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2017-03-14 16:33 UTC by Nicola
Modified:	2017-03-16 11:50 UTC

See Also:
GNOME target:	---
GNOME version:	---

Attachments
proposed fix (3.42 KB, patch) 2017-03-14 16:33 UTC, Nicola	none	Details \| Review
gdp capture (422.39 KB, application/octet-stream) 2017-03-14 17:22 UTC, Nicola		Details
rtph264depay: fix crash with empty sprops-parameters (951 bytes, patch) 2017-03-16 00:49 UTC, Tim-Philipp Müller	committed	Details \| Review

Description Nicola 2017-03-14 16:33:17 UTC

Created attachment 347928 [details] [review]
proposed fix

a pipeline like this:

rtspsrc ! rtph264depay ! fakesink

segfaults with some rtsp stream, it seems that nal buffer can have NULL data,

the actual code segfault this way:

+ Trace 237263

#0 gst_rtp_h264_add_sps_pps
at gstrtph264depay.c line 510
#1 gst_rtp_h264_depay_add_sps_pps
at gstrtph264depay.c line 607
#2 gst_rtp_h264_depay_setcaps
at gstrtph264depay.c line 713
#3 gst_rtp_base_depayload_setcaps
at gstrtpbasedepayload.c line 326
#4 gst_rtp_base_depayload_handle_event
at gstrtpbasedepayload.c line 608
#5 gst_pad_send_event_unchecked
at gstpad.c line 5608
#6 gst_pad_push_event_unchecked
at gstpad.c line 5264
#7 push_sticky
at gstpad.c line 3807
#8 events_foreach
at gstpad.c line 604
#9 check_sticky
at gstpad.c line 3864
#10 gst_pad_push_event
at gstpad.c line 5395
#11 event_forward_func
at gstpad.c line 2992
#12 gst_pad_forward
at gstpad.c line 2946
#13 gst_pad_event_default
at gstpad.c line 3043
#14 gst_pad_send_event_unchecked
at gstpad.c line 5608
#15 gst_pad_push_event_unchecked
at gstpad.c line 5264
#16 push_sticky
at gstpad.c line 3807
#17 events_foreach
at gstpad.c line 604
#18 check_sticky
at gstpad.c line 3864
#19 gst_pad_push_event
at gstpad.c line 5395
#20 event_forward_func
at gstpad.c line 2992
#21 gst_pad_forward
at gstpad.c line 2946
#22 gst_pad_event_default
at gstpad.c line 3043
#23 gst_pad_send_event_unchecked
at gstpad.c line 5608
#24 gst_pad_push_event_unchecked
at gstpad.c line 5264
#25 push_sticky
at gstpad.c line 3807
#26 events_foreach
at gstpad.c line 604
#27 check_sticky
at gstpad.c line 3864
#28 gst_pad_push_data
at gstpad.c line 4435
#29 gst_pad_push
at gstpad.c line 4576
#30 gst_rtp_pt_demux_chain
#31 gst_pad_chain_data_unchecked
at gstpad.c line 4205
#32 gst_pad_push_data
at gstpad.c line 4457
#33 gst_pad_push
at gstpad.c line 4576
#34 pop_and_push_next
at gstrtpjitterbuffer.c line 3377
#35 handle_next_buffer
at gstrtpjitterbuffer.c line 3476
#36 gst_rtp_jitter_buffer_loop
at gstrtpjitterbuffer.c line 4022
#37 gst_task_func
at gsttask.c line 335
#38 0x00007ffff6a20b6e in
#39 0x00007ffff6a20175 in
#40 start_thread
#41 clone

Comment 1 Tim-Philipp Müller 2017-03-14 17:10:55 UTC

Any chance you could provide the capture of an input stream (pcap or gdp-protocol) that reproduces the issue?

The g_debug() thing has been removed in master.

map.data should never be NULL. You probably want to check the return value of gst_buffer_map() instead. But the question is still why the mapping fails here!

Comment 2 Nicola 2017-03-14 17:22:19 UTC

Created attachment 347931 [details]
gdp capture

using

filesrc ! gdpdepay ! rtph264depay ! fakesink

you can reproduce the issue,

gst_buffer_map does not fail it returns TRUE

Comment 3 Tim-Philipp Müller 2017-03-16 00:49:19 UTC

Created attachment 348049 [details] [review]
rtph264depay: fix crash with empty sprops-parameters

How about this?

Comment 4 Tim-Philipp Müller 2017-03-16 00:49:44 UTC

Also, what sends/creates this? Clearly broken.

Comment 5 Nicola 2017-03-16 08:04:18 UTC

(In reply to Tim-Philipp Müller from comment #3)
> Created attachment 348049 [details] [review] [review]
> rtph264depay: fix crash with empty sprops-parameters
> 
> How about this?

this fix the issue as well, thanks!

Comment 6 Nicola 2017-03-16 08:04:55 UTC

(In reply to Tim-Philipp Müller from comment #4)
> Also, what sends/creates this? Clearly broken.

another broken ganz dvr :)

Comment 7 Tim-Philipp Müller 2017-03-16 11:30:03 UTC

Okay, thanks. I'll push my version then since it fixes the issue closer to the source.

commit c8f094cf7de014c5cf72d782cd67e18c5c3deffa
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Thu Mar 16 00:41:44 2017 +0000

    rtph264depay: fix crash with empty sprops-parameters
    
    https://bugzilla.gnome.org/show_bug.cgi?id=780040