GNOME Bugzilla – Bug 782648
heap-use-after-free in gst_debug_print_object() spotted by Asan
Last modified: 2018-11-03 12:41:12 UTC
Created attachment 351856 [details] heap-use-after-free in gstvalue.c Hi, Asan found a couple of use after free cases when we print an object name, mainly when debugging level is high enough. There's a possibility that the name is read after the old pointer is freed and before the new one is assigned in gst_object_set_name().
Created attachment 351858 [details] heap-use-after-free in gsttask.c
Created attachment 351859 [details] [review] gstvalue: fix a race when retrieving object name Possible fix for the access to the object's name in gstvalue
Created attachment 351860 [details] [review] gsttask: object debug function should lock the object first Possible fix for the access to the object's name in gsttask.
Review of attachment 351860 [details] [review]: ::: gst/gsttask.c @@ +689,1 @@ GST_DEBUG_OBJECT (task, "Changing task %p to state %d", task, state); Where is this printing the name of the object?
Review of attachment 351859 [details] [review]: ::: gst/gstvalue.c @@ +6436,3 @@ g_strdup_printf ("(%s) %s", G_OBJECT_TYPE_NAME (obj), GST_OBJECT_NAME (obj)); + GST_OBJECT_UNLOCK (obj); This should not be needed as the object name is considered immutable. As long as you have a reference to the object, it's not going to go away
Created attachment 351877 [details] [review] object: Warn if setting the object name if reference count is greater than 1 Various places in the code are accessing the object name without any locks and assume that it never changes once set and is valid as long as the object reference is.
Comment on attachment 351877 [details] [review] object: Warn if setting the object name if reference count is greater than 1 >+ if (GST_OBJECT_REFCOUNT (object) > 1) { >+ g_warning >+ ("Trying to set name on GstObject with reference count greater than 1"); >+ } Should maybe be GST_OBJECT_REFCOUNT_VALUE, so we do an atomic int get, otherwise asan will probably complain again ;)
Also it does not work because setting the name via property already increases the refcount :) Lots of tests are failing.
Not sure what else we can do here
(In reply to Sebastian Dröge (slomo) from comment #9) > Not sure what else we can do here Maybe have a "set_name_unchecked" function, called from both gst_object_set_name and gst_object_set_property, in gst_object_set_name check if refcount > 1 and in set_property if refcount > 2? That's pretty clunky but the best I can think of
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/236.