GNOME Bugzilla – Bug 762702
gst_bus_add_watch will SEGV when process reaches fd limit
Last modified: 2016-02-26 10:18:02 UTC
Created attachment 322414 [details] Test program to generate issue gst_bus_add_watch will SEGV when process reaches fd limit When gst_bus_new is called. It will fail to init priv->poll correctly because the socketpair call fails. So when gst_bus_add_watch is called it will then fail to check the return the failing gst_bus_create_watch which will then derefence the null pointer.
+ Trace 236004
$1 = {object = {object = {g_type_instance = {g_class = 0x61ade0}, ref_count = 1, qdata = 0x0}, lock = {p = 0x1, i = {1, 0}}, name = 0x619800 "bus0", parent = 0x0, Python Exception <class 'TypeError'> iter() returned non-iterator of type '_iterator': flags = 0, control_bindings = 0x0, control_rate = 100000000, last_sync = 18446744073709551615, _gst_reserved = 0x0}, priv = 0x630010, _gst_reserved = {0x0, 0x0, 0x0, 0x0}} (gdb) p *bus.priv $2 = {queue = 0x62fc50, queue_lock = {p = 0x0, i = {0, 0}}, sync_handler = 0x0, sync_handler_data = 0x0, sync_handler_notify = 0x0, num_signal_watchers = 0, num_sync_message_emitters = 0, signal_watch = 0x0, enable_async = 1, poll = 0x0, pollfd = {fd = 0, events = 0, revents = 0}}
Created attachment 322415 [details] [review] Possible fix A possible fix which checks the pointer returns from gst_bus_create_watch logs an error and returns 0
Review of attachment 322415 [details] [review]: Thanks for the patch! Can you update it slightly? ::: gst/gstbus.c @@ +895,3 @@ + if (!source) { + GST_ERROR_OBJECT (bus, + "gst_bus_create_watch failed so gst_bus_add_watch_full_unlocked failed"); This should probably be a g_critical(). It's not like much is going to work from this point onwards :)
Created attachment 322416 [details] [review] Updated patch Sure i can use g_critical instead
I wonder if we should create an error message and deposit it on the bus in this case, and queue the watch callback with a one-shot g_idle_add() instead (or g_main_context_invooke) so the app gets the error message.
That would be a good idea. You want to write a patch for that on top of the other one? I think a g_critical() will additionally be also useful.
Creating an ERROR message is not trivial here though as messages have sources... we could use the bus as a source but that might break assumptions in existing code that error messages only come from elements. Let's get this patch here in for now, can be improved later. commit 5048155f575856b0a5a07d95dac72b90bee9d7bb Author: James Stevenson <james@stev.org> Date: Thu Feb 25 22:36:14 2016 +0000 bus: Prevent gst_bus_add_watch_full_unlocked from a segfault if priv->poll == NULL This happens if the process runs out of file descriptors. Better print a critical warning instead of just crashing. https://bugzilla.gnome.org/show_bug.cgi?id=762702