tracers: leaks: fix potentially invalid memory access when trying to detect object type

The is_gst_mini_object_check would sometimes detect a proper GObject
as a mini object, and then bad things happen.

We know whether a pointer is a proper GObject or a MiniObject here
though, so just pass that information to the right code paths and
avoid the heuristics altogether.

There are probably more cases where the check should be eliminated.

Fixes #1334, maybe

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2846>
This commit is contained in:
Tim-Philipp Müller 2022-08-03 12:10:02 +01:00
parent 1fff4ef635
commit 5948da1a1f

View file

@ -114,6 +114,12 @@ static guint gst_leaks_tracer_signals[LAST_SIGNAL] = { 0 };
G_LOCK_DEFINE_STATIC (instances); G_LOCK_DEFINE_STATIC (instances);
typedef enum
{
GOBJECT,
MINI_OBJECT,
} ObjectKind;
typedef struct typedef struct
{ {
gboolean reffed; gboolean reffed;
@ -323,16 +329,23 @@ object_is_gst_mini_object (gpointer obj)
} }
static ObjectLog * static ObjectLog *
object_log_new (gpointer obj) object_log_new (gpointer obj, ObjectKind kind)
{ {
ObjectLog *o = g_new (ObjectLog, 1); ObjectLog *o = g_new (ObjectLog, 1);
o->object = obj; o->object = obj;
if (object_is_gst_mini_object (obj)) switch (kind) {
o->type_name = g_type_name (GST_MINI_OBJECT_TYPE (obj)); case GOBJECT:
else o->type_name = G_OBJECT_TYPE_NAME (obj);
o->type_name = G_OBJECT_TYPE_NAME (obj); break;
case MINI_OBJECT:
o->type_name = g_type_name (GST_MINI_OBJECT_TYPE (obj));
break;
default:
g_assert_not_reached ();
break;
}
return o; return o;
} }
@ -344,7 +357,8 @@ object_log_free (ObjectLog * obj)
} }
static void static void
handle_object_destroyed (GstLeaksTracer * self, gpointer object) handle_object_destroyed (GstLeaksTracer * self, gpointer object,
ObjectKind kind)
{ {
GST_OBJECT_LOCK (self); GST_OBJECT_LOCK (self);
if (self->done) { if (self->done) {
@ -356,7 +370,7 @@ handle_object_destroyed (GstLeaksTracer * self, gpointer object)
g_hash_table_remove (self->objects, object); g_hash_table_remove (self->objects, object);
if (self->removed) if (self->removed)
g_hash_table_add (self->removed, object_log_new (object)); g_hash_table_add (self->removed, object_log_new (object, kind));
out: out:
GST_OBJECT_UNLOCK (self); GST_OBJECT_UNLOCK (self);
} }
@ -366,7 +380,7 @@ object_weak_cb (gpointer data, GObject * object)
{ {
GstLeaksTracer *self = data; GstLeaksTracer *self = data;
handle_object_destroyed (self, object); handle_object_destroyed (self, object, GOBJECT);
} }
static void static void
@ -374,16 +388,16 @@ mini_object_weak_cb (gpointer data, GstMiniObject * object)
{ {
GstLeaksTracer *self = data; GstLeaksTracer *self = data;
handle_object_destroyed (self, object); handle_object_destroyed (self, object, MINI_OBJECT);
} }
static void static void
handle_object_created (GstLeaksTracer * self, gpointer object, GType type, handle_object_created (GstLeaksTracer * self, gpointer object, GType type,
gboolean gobject) gboolean gobject)
{ {
ObjectKind kind = gobject ? GOBJECT : MINI_OBJECT; // FIXME: change arg to pass directly
ObjectRefingInfos *infos; ObjectRefingInfos *infos;
if (!should_handle_object_type (self, type)) if (!should_handle_object_type (self, type))
return; return;
@ -401,7 +415,7 @@ handle_object_created (GstLeaksTracer * self, gpointer object, GType type,
g_hash_table_insert (self->objects, object, infos); g_hash_table_insert (self->objects, object, infos);
if (self->added) if (self->added)
g_hash_table_add (self->added, object_log_new (object)); g_hash_table_add (self->added, object_log_new (object, kind));
GST_OBJECT_UNLOCK (self); GST_OBJECT_UNLOCK (self);
} }