From 86927c39e01d7d8ae1e520727db41b1402580e1a Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 10 May 2022 15:53:19 +0100 Subject: [PATCH 1/4] gresource: Assert that resource has correct refcount when unregistering This should fix a scan-build warning that `resource` is used-after-freeing in the final `g_resource_unref()` call in `g_static_resource_fini()`, as `g_resources_unregister_unlocked()` has already unreffed it. In reality, each resource has two strong refs on it while active, so the second unref is correct. Signed-off-by: Philip Withnall --- gio/gresource.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gio/gresource.c b/gio/gresource.c index 45ca92b1f..921a02910 100644 --- a/gio/gresource.c +++ b/gio/gresource.c @@ -1444,6 +1444,10 @@ g_static_resource_fini (GStaticResource *static_resource) resource = g_atomic_pointer_get (&static_resource->resource); if (resource) { + /* There should be at least two references to the resource now: one for + * static_resource->resource, and one in the registered_resources list. */ + g_assert (g_atomic_int_get (&resource->ref_count) >= 2); + g_atomic_pointer_set (&static_resource->resource, NULL); g_resources_unregister_unlocked (resource); g_resource_unref (resource); From 77679787255cc1d7cc881766a82f8fdd051507e6 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 10 May 2022 16:17:38 +0100 Subject: [PATCH 2/4] gdbusdaemon: Add an assertion to help static analysis of refcounts This should fix a scan-build warning about the final `name_unref()` here being a use-after-free. Signed-off-by: Philip Withnall --- gio/gdbusdaemon.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gio/gdbusdaemon.c b/gio/gdbusdaemon.c index 5dfe62cd3..2bb7b8816 100644 --- a/gio/gdbusdaemon.c +++ b/gio/gdbusdaemon.c @@ -874,7 +874,11 @@ client_free (Client *client) name_ref (name); if (name->owner && name->owner->client == client) - name_release_owner (name); + { + /* Help static analysers with the refcount at this point. */ + g_assert (name->refcount >= 2); + name_release_owner (name); + } name_unqueue_owner (name, client); From e36f2bb24364a9deebe9d2f7d3e81021e9f174bf Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 10 May 2022 16:23:46 +0100 Subject: [PATCH 3/4] gproxyaddressenumerator: Factor out type check This will help static analysers which think that the type of `priv->proxy_address` could potentially change between freeing `dest_hostname` and the `g_return_if_fail()` call below, leading to the code to continue through to `g_object_new()` and use `dest_hostname` after freeing it. This introduces no functional changes. Signed-off-by: Philip Withnall --- gio/gproxyaddressenumerator.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gio/gproxyaddressenumerator.c b/gio/gproxyaddressenumerator.c index 654baade5..e841a902f 100644 --- a/gio/gproxyaddressenumerator.c +++ b/gio/gproxyaddressenumerator.c @@ -325,6 +325,7 @@ return_result (GTask *task) { GProxyAddressEnumeratorPrivate *priv = g_task_get_task_data (task); GSocketAddress *result; + gboolean is_inet_socket_address; if (strcmp ("direct", priv->proxy_type) == 0) { @@ -356,12 +357,13 @@ return_result (GTask *task) } dest_protocol = g_uri_parse_scheme (priv->dest_uri); - if (!G_IS_INET_SOCKET_ADDRESS (priv->proxy_address)) + is_inet_socket_address = G_IS_INET_SOCKET_ADDRESS (priv->proxy_address); + if (!is_inet_socket_address) { g_free (dest_hostname); g_free (dest_protocol); } - g_return_if_fail (G_IS_INET_SOCKET_ADDRESS (priv->proxy_address)); + g_return_if_fail (is_inet_socket_address); inetsaddr = G_INET_SOCKET_ADDRESS (priv->proxy_address); inetaddr = g_inet_socket_address_get_address (inetsaddr); From e0fe616dbc45ebf5270e788b3b344d04a1157a70 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 10 May 2022 16:29:22 +0100 Subject: [PATCH 4/4] gvariant: Factor out type check This will help static analysers, similarly to with the previous commit. This introduces no functional changes. Signed-off-by: Philip Withnall --- glib/gvariant.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/glib/gvariant.c b/glib/gvariant.c index 42ffc9a87..6384174ca 100644 --- a/glib/gvariant.c +++ b/glib/gvariant.c @@ -800,12 +800,13 @@ g_variant_new_array (const GVariantType *child_type, for (i = 0; i < n_children; i++) { - if G_UNLIKELY (!g_variant_is_of_type (children[i], child_type)) + gboolean is_of_child_type = g_variant_is_of_type (children[i], child_type); + if G_UNLIKELY (!is_of_child_type) { while (i != 0) g_variant_unref (my_children[--i]); g_free (my_children); - g_return_val_if_fail (g_variant_is_of_type (children[i], child_type), NULL); + g_return_val_if_fail (is_of_child_type, NULL); } my_children[i] = g_variant_ref_sink (children[i]); trusted &= g_variant_is_trusted (children[i]);