GNOME Bugzilla – Bug 699637
rtpgstpay: uses an already-unreffed event in its sink_event handler
Last modified: 2013-05-04 10:17:34 UTC
By checking out git HEAD of gst-plugins-good and gst-rtsp-server and running the RTSP server's unit tests like so: make -C tests/check/ gst/rtspserver.valgrind I get the bogus log quoted below. I traced this back to gst_rtp_gst_pay_sink_event() in gst-plugins-good/gst/rtp/gstrtpgstpay.c accessing an unreffed event. The attached patch appears to fix the problem, and I believe it to be a correct patch. Running suite(s): rtspserver 0:00:17.845625879 32325 0xa2d9a40 ERROR rtspclient rtsp-client.c:508:find_media: client 0xa2dbfa0: no factory for uri 0:00:17.878471304 32325 0xa2d9a40 ERROR rtspclient rtsp-client.c:1505:handle_describe_request: client 0xa2dbfa0: no media 0:00:25.297796911 32342 0x8a0bd00 ERROR rtspclient rtsp-client.c:1325:handle_setup_request: client 0x8a08a10: media not found 0:00:32.139580338 32376 0x89fb190 ERROR rtspclient rtsp-client.c:966:handle_play_request: client 0xa2db260: no session 0:00:32.991345425 32377 0x8a17e00 ERROR rtspserver rtsp-server.c:877:gst_rtsp_server_create_socket:<GstRTSPServer@0x89fc760> failed to create socket 0:00:33.008538299 32377 0x8a17e00 ERROR rtspserver rtsp-server.c:1301:gst_rtsp_server_create_source:<GstRTSPServer@0x89fc760> failed to create socket 0:00:33.009169204 32377 0x8a17e00 ERROR rtspserver rtsp-server.c:1341:gst_rtsp_server_attach:<GstRTSPServer@0x89fc760> failed to create watch: Error binding to address: Address already in use 0:01:07.219939928 32441 0x8a297f0 ERROR rtspclient rtsp-client.c:1741:handle_request: client 0x8a1da00: session not found ==32489== Invalid read of size 4 ==32489== at 0xAA7C9E7: gst_rtp_gst_pay_sink_event (gstrtpgstpay.c:313) ==32489== by 0x50B5372: gst_rtp_base_payload_sink_event (gstrtpbasepayload.c:404) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x5790539: gst_base_transform_sink_eventfunc (gstbasetransform.c:1854) ==32489== by 0x5790183: gst_base_transform_sink_event (gstbasetransform.c:1794) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x577D014: gst_base_src_perform_seek (gstbasesrc.c:1566) ==32489== by 0x577EAE1: gst_base_src_default_event (gstbasesrc.c:1898) ==32489== by 0x577EDA2: gst_base_src_event (gstbasesrc.c:1950) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x57907E9: gst_base_transform_src_eventfunc (gstbasetransform.c:1906) ==32489== by 0x5790615: gst_base_transform_src_event (gstbasetransform.c:1873) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x6271797: event_forward_func (gstpad.c:2735) ==32489== by 0x62713EA: gst_pad_forward (gstpad.c:2689) ==32489== by 0x6271A57: gst_pad_event_default (gstpad.c:2786) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x6271797: event_forward_func (gstpad.c:2735) ==32489== by 0x62713EA: gst_pad_forward (gstpad.c:2689) ==32489== by 0x6271A57: gst_pad_event_default (gstpad.c:2786) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x6271797: event_forward_func (gstpad.c:2735) ==32489== by 0x62713EA: gst_pad_forward (gstpad.c:2689) ==32489== by 0x6271A57: gst_pad_event_default (gstpad.c:2786) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x6271797: event_forward_func (gstpad.c:2735) ==32489== by 0x62713EA: gst_pad_forward (gstpad.c:2689) ==32489== by 0x6271A57: gst_pad_event_default (gstpad.c:2786) ==32489== by 0xBBB7DA9: gst_rtp_session_event_send_rtp_src (gstrtpsession.c:1776) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x6271797: event_forward_func (gstpad.c:2735) ==32489== by 0x62713EA: gst_pad_forward (gstpad.c:2689) ==32489== by 0x6271A57: gst_pad_event_default (gstpad.c:2786) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== Address 0x8c30b10 is 64 bytes inside a block of size 96 free'd ==32489== at 0x4C27D4E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==32489== by 0x6BBA060: standard_free (gmem.c:98) ==32489== by 0x6BBA380: g_free (gmem.c:252) ==32489== by 0x6BE4CCA: g_slice_free1 (gslice.c:1111) ==32489== by 0x6246F8D: _gst_event_free (gstevent.c:232) ==32489== by 0x626421E: gst_mini_object_unref (gstminiobject.c:467) ==32489== by 0x626530F: gst_event_unref (gstevent.h:451) ==32489== by 0x6271A9D: gst_pad_event_default (gstpad.c:2796) ==32489== by 0x50B4FA4: gst_rtp_base_payload_sink_event_default (gstrtpbasepayload.c:346) ==32489== by 0xAA7C9DF: gst_rtp_gst_pay_sink_event (gstrtpgstpay.c:311) ==32489== by 0x50B5372: gst_rtp_base_payload_sink_event (gstrtpbasepayload.c:404) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x5790539: gst_base_transform_sink_eventfunc (gstbasetransform.c:1854) ==32489== by 0x5790183: gst_base_transform_sink_event (gstbasetransform.c:1794) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x577D014: gst_base_src_perform_seek (gstbasesrc.c:1566) ==32489== by 0x577EAE1: gst_base_src_default_event (gstbasesrc.c:1898) ==32489== by 0x577EDA2: gst_base_src_event (gstbasesrc.c:1950) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x57907E9: gst_base_transform_src_eventfunc (gstbasetransform.c:1906) ==32489== by 0x5790615: gst_base_transform_src_event (gstbasetransform.c:1873) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x6271797: event_forward_func (gstpad.c:2735) ==32489== by 0x62713EA: gst_pad_forward (gstpad.c:2689) ==32489== by 0x6271A57: gst_pad_event_default (gstpad.c:2786) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x6271797: event_forward_func (gstpad.c:2735) ==32489== by 0x62713EA: gst_pad_forward (gstpad.c:2689) ==32489== by 0x6271A57: gst_pad_event_default (gstpad.c:2786) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x6271797: event_forward_func (gstpad.c:2735) ==32489== by 0x62713EA: gst_pad_forward (gstpad.c:2689) ==32489== by 0x6271A57: gst_pad_event_default (gstpad.c:2786) ==32489== by 0x627C00F: gst_pad_send_event_unchecked (gstpad.c:4886) ==32489== by 0x627A8E0: gst_pad_push_event_unchecked (gstpad.c:4582) ==32489== by 0x627B0DD: gst_pad_push_event (gstpad.c:4705) ==32489== by 0x6271797: event_forward_func (gstpad.c:2735) ==32489== by 0x62713EA: gst_pad_forward (gstpad.c:2689)
Created attachment 243251 [details] [review] Proposed patch
Looks fine to me, thanks for the patch. The basertppay handler doesn't do anything for the events we care about, only difference is that they get forwarded before the rtp packet, but I can't imagine that mattering anywhere. commit 9532b04947baf701585a8643c90b1e1c48498be3 Author: Sebastian Rasmussen <sebras@gmail.com> Date: Fri May 3 23:43:26 2013 +0200 rtpgstpay: fix invalid memory access in event handler First process event in payloader, then hand it to the base class which takes ownership of the event. https://bugzilla.gnome.org/show_bug.cgi?id=699637