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 776125 - fix an use after free in gst_base_sink_send_event()
fix an use after free in gst_base_sink_send_event()
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.8.3
Other Linux
: Normal normal
: 1.10.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-12-15 09:55 UTC by Fabrice Bellet
Modified: 2017-11-02 22:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Hold a reference on the event until the debug macro is called. (1.02 KB, patch)
2016-12-15 09:55 UTC, Fabrice Bellet
none Details | Review
basesink: fix a use after free case (1.19 KB, patch)
2016-12-16 22:41 UTC, Fabrice Bellet
committed Details | Review

Description Fabrice Bellet 2016-12-15 09:55:14 UTC
Created attachment 342006 [details] [review]
Hold a reference on the event until the debug macro is called.

Hi!

I noticed with the address sanitizer a case where the event may be disposed while being pushed. When this happens, the event cannot be used in the GST_DEBUG_OBJECT() macro a couple of line later.
Comment 1 Sebastian Dröge (slomo) 2016-12-15 10:59:31 UTC
Review of attachment 342006 [details] [review]:

::: libs/gst/base/gstbasesink.c
@@ +4504,2 @@
   GST_DEBUG_OBJECT (basesink, "handled event %p %" GST_PTR_FORMAT ": %d", event,
       event, result);

The nicer solution would be, to move this debug output before the gst_pad_push() (without the result of course, and saying "sending" instead of "handled"). And then having another here that just says "handled event: %", result

That way the event is possibly writeable by downstream and we don't keep another reference to it
Comment 2 Sebastian Dröge (slomo) 2016-12-16 12:17:09 UTC
Let me know if you want to update the patch, or if you'd prefer me or someone else to work on this. Thanks :)
Comment 3 Fabrice Bellet 2016-12-16 22:41:29 UTC
Created attachment 342088 [details] [review]
basesink: fix a use after free case

This is an update of my patch, that splits the GST_DEBUG_OBJECT() message in two parts as suggested. Thanks for the review.
Comment 4 Arun Raghavan 2016-12-17 04:13:52 UTC
Comment on attachment 342088 [details] [review]
basesink: fix a use after free case

Pushed this to master now, thanks!