Add guards against invalid GSources to public GSource API

Especially check for a valid reference count. This is possible now in
all cases because of the addition of the dispose function and makes
usage of already finalized/finalizing GSources more obvious than the
use-after-free that would otherwise happen.
This commit is contained in:
Sebastian Dröge 2019-10-20 11:21:07 +03:00
parent 0adf5cae35
commit be1ec9004b

View File

@ -958,6 +958,7 @@ g_source_set_dispose_function (GSource *source,
{ {
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (source->priv->dispose == NULL); g_return_if_fail (source->priv->dispose == NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
source->priv->dispose = dispose; source->priv->dispose = dispose;
} }
@ -1220,6 +1221,8 @@ g_source_attach (GSource *source,
{ {
guint result = 0; guint result = 0;
g_return_val_if_fail (source != NULL, 0);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, 0);
g_return_val_if_fail (source->context == NULL, 0); g_return_val_if_fail (source->context == NULL, 0);
g_return_val_if_fail (!SOURCE_DESTROYED (source), 0); g_return_val_if_fail (!SOURCE_DESTROYED (source), 0);
@ -1317,6 +1320,7 @@ g_source_destroy (GSource *source)
GMainContext *context; GMainContext *context;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
context = source->context; context = source->context;
@ -1348,6 +1352,7 @@ g_source_get_id (GSource *source)
guint result; guint result;
g_return_val_if_fail (source != NULL, 0); g_return_val_if_fail (source != NULL, 0);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, 0);
g_return_val_if_fail (source->context != NULL, 0); g_return_val_if_fail (source->context != NULL, 0);
LOCK_CONTEXT (source->context); LOCK_CONTEXT (source->context);
@ -1377,6 +1382,8 @@ g_source_get_id (GSource *source)
GMainContext * GMainContext *
g_source_get_context (GSource *source) g_source_get_context (GSource *source)
{ {
g_return_val_if_fail (source != NULL, NULL);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, NULL);
g_return_val_if_fail (source->context != NULL || !SOURCE_DESTROYED (source), NULL); g_return_val_if_fail (source->context != NULL || !SOURCE_DESTROYED (source), NULL);
return source->context; return source->context;
@ -1408,6 +1415,7 @@ g_source_add_poll (GSource *source,
GMainContext *context; GMainContext *context;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
g_return_if_fail (fd != NULL); g_return_if_fail (fd != NULL);
g_return_if_fail (!SOURCE_DESTROYED (source)); g_return_if_fail (!SOURCE_DESTROYED (source));
@ -1444,6 +1452,7 @@ g_source_remove_poll (GSource *source,
GMainContext *context; GMainContext *context;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
g_return_if_fail (fd != NULL); g_return_if_fail (fd != NULL);
g_return_if_fail (!SOURCE_DESTROYED (source)); g_return_if_fail (!SOURCE_DESTROYED (source));
@ -1494,7 +1503,9 @@ g_source_add_child_source (GSource *source,
GMainContext *context; GMainContext *context;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
g_return_if_fail (child_source != NULL); g_return_if_fail (child_source != NULL);
g_return_if_fail (g_atomic_int_get (&child_source->ref_count) > 0);
g_return_if_fail (!SOURCE_DESTROYED (source)); g_return_if_fail (!SOURCE_DESTROYED (source));
g_return_if_fail (!SOURCE_DESTROYED (child_source)); g_return_if_fail (!SOURCE_DESTROYED (child_source));
g_return_if_fail (child_source->context == NULL); g_return_if_fail (child_source->context == NULL);
@ -1555,7 +1566,9 @@ g_source_remove_child_source (GSource *source,
GMainContext *context; GMainContext *context;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
g_return_if_fail (child_source != NULL); g_return_if_fail (child_source != NULL);
g_return_if_fail (g_atomic_int_get (&child_source->ref_count) > 0);
g_return_if_fail (child_source->priv->parent_source == source); g_return_if_fail (child_source->priv->parent_source == source);
g_return_if_fail (!SOURCE_DESTROYED (source)); g_return_if_fail (!SOURCE_DESTROYED (source));
g_return_if_fail (!SOURCE_DESTROYED (child_source)); g_return_if_fail (!SOURCE_DESTROYED (child_source));
@ -1638,6 +1651,7 @@ g_source_set_callback_indirect (GSource *source,
GSourceCallbackFuncs *old_cb_funcs; GSourceCallbackFuncs *old_cb_funcs;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
g_return_if_fail (callback_funcs != NULL || callback_data == NULL); g_return_if_fail (callback_funcs != NULL || callback_data == NULL);
context = source->context; context = source->context;
@ -1700,6 +1714,7 @@ g_source_set_callback (GSource *source,
GSourceCallback *new_callback; GSourceCallback *new_callback;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
TRACE (GLIB_SOURCE_SET_CALLBACK (source, func, data, notify)); TRACE (GLIB_SOURCE_SET_CALLBACK (source, func, data, notify));
@ -1813,6 +1828,7 @@ g_source_set_priority (GSource *source,
GMainContext *context; GMainContext *context;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
g_return_if_fail (source->priv->parent_source == NULL); g_return_if_fail (source->priv->parent_source == NULL);
context = source->context; context = source->context;
@ -1836,6 +1852,7 @@ gint
g_source_get_priority (GSource *source) g_source_get_priority (GSource *source)
{ {
g_return_val_if_fail (source != NULL, 0); g_return_val_if_fail (source != NULL, 0);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, 0);
return source->priority; return source->priority;
} }
@ -1878,15 +1895,7 @@ g_source_set_ready_time (GSource *source,
GMainContext *context; GMainContext *context;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
/* We deliberately don't check for ref_count > 0 here, because that g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
* breaks cancellable_source_cancelled() in GCancellable: it has no
* way to find out that the last-unref has happened until the
* finalize() function is called, but that's too late, because the
* ref_count already has already reached 0 before that time.
* However, priv is only poisoned (set to NULL) after finalize(),
* so we can use this as a simple guard against use-after-free.
* See https://bugzilla.gnome.org/show_bug.cgi?id=791754 */
g_return_if_fail (source->priv != NULL);
context = source->context; context = source->context;
@ -1930,6 +1939,7 @@ gint64
g_source_get_ready_time (GSource *source) g_source_get_ready_time (GSource *source)
{ {
g_return_val_if_fail (source != NULL, -1); g_return_val_if_fail (source != NULL, -1);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, -1);
return source->priv->ready_time; return source->priv->ready_time;
} }
@ -1951,6 +1961,7 @@ g_source_set_can_recurse (GSource *source,
GMainContext *context; GMainContext *context;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
context = source->context; context = source->context;
@ -1979,6 +1990,7 @@ gboolean
g_source_get_can_recurse (GSource *source) g_source_get_can_recurse (GSource *source)
{ {
g_return_val_if_fail (source != NULL, FALSE); g_return_val_if_fail (source != NULL, FALSE);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, FALSE);
return (source->flags & G_SOURCE_CAN_RECURSE) != 0; return (source->flags & G_SOURCE_CAN_RECURSE) != 0;
} }
@ -2015,6 +2027,7 @@ g_source_set_name (GSource *source,
GMainContext *context; GMainContext *context;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
context = source->context; context = source->context;
@ -2050,6 +2063,7 @@ const char *
g_source_get_name (GSource *source) g_source_get_name (GSource *source)
{ {
g_return_val_if_fail (source != NULL, NULL); g_return_val_if_fail (source != NULL, NULL);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, NULL);
return source->name; return source->name;
} }
@ -2106,6 +2120,9 @@ GSource *
g_source_ref (GSource *source) g_source_ref (GSource *source)
{ {
g_return_val_if_fail (source != NULL, NULL); g_return_val_if_fail (source != NULL, NULL);
/* We allow ref_count == 0 here to allow the dispose function to resurrect
* the GSource if needed */
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) >= 0, NULL);
g_atomic_int_inc (&source->ref_count); g_atomic_int_inc (&source->ref_count);
@ -2171,6 +2188,8 @@ g_source_unref_internal (GSource *source,
if (source->source_funcs->finalize) if (source->source_funcs->finalize)
{ {
gint old_ref_count;
/* Temporarily increase the ref count again so that GSource methods /* Temporarily increase the ref count again so that GSource methods
* can be called from finalize(). */ * can be called from finalize(). */
g_atomic_int_inc (&source->ref_count); g_atomic_int_inc (&source->ref_count);
@ -2179,11 +2198,14 @@ g_source_unref_internal (GSource *source,
source->source_funcs->finalize (source); source->source_funcs->finalize (source);
if (context) if (context)
LOCK_CONTEXT (context); LOCK_CONTEXT (context);
g_atomic_int_add (&source->ref_count, -1); old_ref_count = g_atomic_int_add (&source->ref_count, -1);
g_warn_if_fail (old_ref_count == 1);
} }
if (old_cb_funcs) if (old_cb_funcs)
{ {
gint old_ref_count;
/* Temporarily increase the ref count again so that GSource methods /* Temporarily increase the ref count again so that GSource methods
* can be called from callback_funcs.unref(). */ * can be called from callback_funcs.unref(). */
g_atomic_int_inc (&source->ref_count); g_atomic_int_inc (&source->ref_count);
@ -2194,7 +2216,8 @@ g_source_unref_internal (GSource *source,
if (context) if (context)
LOCK_CONTEXT (context); LOCK_CONTEXT (context);
g_atomic_int_add (&source->ref_count, -1); old_ref_count = g_atomic_int_add (&source->ref_count, -1);
g_warn_if_fail (old_ref_count == 1);
} }
g_free (source->name); g_free (source->name);
@ -2238,6 +2261,7 @@ void
g_source_unref (GSource *source) g_source_unref (GSource *source)
{ {
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
g_source_unref_internal (source, source->context, FALSE); g_source_unref_internal (source, source->context, FALSE);
} }
@ -2541,6 +2565,7 @@ g_source_add_unix_fd (GSource *source,
GPollFD *poll_fd; GPollFD *poll_fd;
g_return_val_if_fail (source != NULL, NULL); g_return_val_if_fail (source != NULL, NULL);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, NULL);
g_return_val_if_fail (!SOURCE_DESTROYED (source), NULL); g_return_val_if_fail (!SOURCE_DESTROYED (source), NULL);
poll_fd = g_new (GPollFD, 1); poll_fd = g_new (GPollFD, 1);
@ -2594,6 +2619,7 @@ g_source_modify_unix_fd (GSource *source,
GPollFD *poll_fd; GPollFD *poll_fd;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
g_return_if_fail (g_slist_find (source->priv->fds, tag)); g_return_if_fail (g_slist_find (source->priv->fds, tag));
context = source->context; context = source->context;
@ -2631,6 +2657,7 @@ g_source_remove_unix_fd (GSource *source,
GPollFD *poll_fd; GPollFD *poll_fd;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0);
g_return_if_fail (g_slist_find (source->priv->fds, tag)); g_return_if_fail (g_slist_find (source->priv->fds, tag));
context = source->context; context = source->context;
@ -2679,6 +2706,7 @@ g_source_query_unix_fd (GSource *source,
GPollFD *poll_fd; GPollFD *poll_fd;
g_return_val_if_fail (source != NULL, 0); g_return_val_if_fail (source != NULL, 0);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, 0);
g_return_val_if_fail (g_slist_find (source->priv->fds, tag), 0); g_return_val_if_fail (g_slist_find (source->priv->fds, tag), 0);
poll_fd = tag; poll_fd = tag;
@ -3109,6 +3137,8 @@ g_main_current_source (void)
gboolean gboolean
g_source_is_destroyed (GSource *source) g_source_is_destroyed (GSource *source)
{ {
g_return_val_if_fail (source != NULL, TRUE);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, TRUE);
return SOURCE_DESTROYED (source); return SOURCE_DESTROYED (source);
} }
@ -4509,6 +4539,8 @@ g_source_get_time (GSource *source)
GMainContext *context; GMainContext *context;
gint64 result; gint64 result;
g_return_val_if_fail (source != NULL, 0);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, 0);
g_return_val_if_fail (source->context != NULL, 0); g_return_val_if_fail (source->context != NULL, 0);
context = source->context; context = source->context;