diff options
author | Miloslav Trmač <mitr@redhat.com> | 2018-04-02 09:59:40 -0400 |
---|---|---|
committer | Ray Strode <rstrode@redhat.com> | 2018-04-03 14:16:04 -0400 |
commit | ddbbe5958b6560b97825efedb9a54a06eb9f90df (patch) | |
tree | 1d3be039a310100e37e66efae14ad6a9ca02ca34 | |
parent | 59d2fde33cf65e252f153545ee8a3083ee545948 (diff) | |
parent | 1458aaa10fed2c782717cf187b3d6ad92172cb8b (diff) | |
download | polkit-ddbbe5958b6560b97825efedb9a54a06eb9f90df.tar.gz |
Audit and fix GVariant reference counting
This patch series fixes a fair number of memory leaks revolving
around GVariant memory allocation and D-Bus calls. The first
patch in the series does not actually fix any leaks, only
simplifies the code; the rest are leak fixes, one or two per
patch.
-rw-r--r-- | src/polkit/polkitactiondescription.c | 26 | ||||
-rw-r--r-- | src/polkit/polkitauthority.c | 51 | ||||
-rw-r--r-- | src/polkit/polkitauthorizationresult.c | 18 | ||||
-rw-r--r-- | src/polkit/polkitdetails.c | 5 | ||||
-rw-r--r-- | src/polkit/polkitidentity.c | 6 | ||||
-rw-r--r-- | src/polkit/polkitsubject.c | 6 | ||||
-rw-r--r-- | src/polkit/polkittemporaryauthorization.c | 21 | ||||
-rw-r--r-- | src/polkitbackend/polkitbackendauthority.c | 28 | ||||
-rw-r--r-- | src/polkitbackend/polkitbackendinteractiveauthority.c | 28 |
9 files changed, 70 insertions, 119 deletions
diff --git a/src/polkit/polkitactiondescription.c b/src/polkit/polkitactiondescription.c index 4bd9604..ed0655e 100644 --- a/src/polkit/polkitactiondescription.c +++ b/src/polkit/polkitactiondescription.c @@ -352,10 +352,10 @@ polkit_action_description_new_for_gvariant (GVariant *value) return action_description; } +/* Note that this returns a floating value. */ GVariant * polkit_action_description_to_gvariant (PolkitActionDescription *action_description) { - GVariant *value; GVariantBuilder builder; GHashTableIter iter; const gchar *a_key; @@ -368,17 +368,15 @@ polkit_action_description_to_gvariant (PolkitActionDescription *action_descripti g_variant_builder_add (&builder, "{ss}", a_key, a_value); /* TODO: note 'foo ? : ""' is a gcc specific extension (it's a short-hand for 'foo ? foo : ""') */ - value = g_variant_new ("(ssssssuuua{ss})", - action_description->action_id ? : "", - action_description->description ? : "", - action_description->message ? : "", - action_description->vendor_name ? : "", - action_description->vendor_url ? : "", - action_description->icon_name ? : "", - action_description->implicit_any, - action_description->implicit_inactive, - action_description->implicit_active, - &builder); - - return value; + return g_variant_new ("(ssssssuuua{ss})", + action_description->action_id ? : "", + action_description->description ? : "", + action_description->message ? : "", + action_description->vendor_name ? : "", + action_description->vendor_url ? : "", + action_description->icon_name ? : "", + action_description->implicit_any, + action_description->implicit_inactive, + action_description->implicit_active, + &builder); } diff --git a/src/polkit/polkitauthority.c b/src/polkit/polkitauthority.c index 7c4db7b..71d527c 100644 --- a/src/polkit/polkitauthority.c +++ b/src/polkit/polkitauthority.c @@ -886,8 +886,6 @@ polkit_authority_check_authorization (PolkitAuthority *authority, GAsyncReadyCallback callback, gpointer user_data) { - GVariant *subject_value; - GVariant *details_value; CheckAuthData *data; g_return_if_fail (POLKIT_IS_AUTHORITY (authority)); @@ -896,11 +894,6 @@ polkit_authority_check_authorization (PolkitAuthority *authority, g_return_if_fail (details == NULL || POLKIT_IS_DETAILS (details)); g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable)); - subject_value = polkit_subject_to_gvariant (subject); - details_value = polkit_details_to_gvariant (details); - g_variant_ref_sink (subject_value); - g_variant_ref_sink (details_value); - data = g_new0 (CheckAuthData, 1); data->authority = g_object_ref (authority); data->simple = g_simple_async_result_new (G_OBJECT (authority), @@ -915,9 +908,9 @@ polkit_authority_check_authorization (PolkitAuthority *authority, g_dbus_proxy_call (authority->proxy, "CheckAuthorization", g_variant_new ("(@(sa{sv})s@a{ss}us)", - subject_value, + polkit_subject_to_gvariant (subject), /* A floating value */ action_id, - details_value, + polkit_details_to_gvariant (details), /* A floating value */ flags, data->cancellation_id != NULL ? data->cancellation_id : ""), G_DBUS_CALL_FLAGS_NONE, @@ -925,8 +918,6 @@ polkit_authority_check_authorization (PolkitAuthority *authority, cancellable, (GAsyncReadyCallback) check_authorization_cb, data); - g_variant_unref (subject_value); - g_variant_unref (details_value); } /** @@ -1058,20 +1049,16 @@ polkit_authority_register_authentication_agent (PolkitAuthority *authority, GAsyncReadyCallback callback, gpointer user_data) { - GVariant *subject_value; - g_return_if_fail (POLKIT_IS_AUTHORITY (authority)); g_return_if_fail (POLKIT_IS_SUBJECT (subject)); g_return_if_fail (locale != NULL); g_return_if_fail (g_variant_is_object_path (object_path)); g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable)); - subject_value = polkit_subject_to_gvariant (subject); - g_variant_ref_sink (subject_value); g_dbus_proxy_call (authority->proxy, "RegisterAuthenticationAgent", g_variant_new ("(@(sa{sv})ss)", - subject_value, + polkit_subject_to_gvariant (subject), /* A floating value */ locale, object_path), G_DBUS_CALL_FLAGS_NONE, @@ -1082,7 +1069,6 @@ polkit_authority_register_authentication_agent (PolkitAuthority *authority, callback, user_data, polkit_authority_register_authentication_agent)); - g_variant_unref (subject_value); } /** @@ -1375,19 +1361,15 @@ polkit_authority_unregister_authentication_agent (PolkitAuthority *authorit GAsyncReadyCallback callback, gpointer user_data) { - GVariant *subject_value; - g_return_if_fail (POLKIT_IS_AUTHORITY (authority)); g_return_if_fail (POLKIT_IS_SUBJECT (subject)); g_return_if_fail (g_variant_is_object_path (object_path)); g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable)); - subject_value = polkit_subject_to_gvariant (subject); - g_variant_ref_sink (subject_value); g_dbus_proxy_call (authority->proxy, "UnregisterAuthenticationAgent", g_variant_new ("(@(sa{sv})s)", - subject_value, + polkit_subject_to_gvariant (subject), /* A floating value */ object_path), G_DBUS_CALL_FLAGS_NONE, -1, @@ -1397,7 +1379,6 @@ polkit_authority_unregister_authentication_agent (PolkitAuthority *authorit callback, user_data, polkit_authority_unregister_authentication_agent)); - g_variant_unref (subject_value); } /** @@ -1511,7 +1492,6 @@ polkit_authority_authentication_agent_response (PolkitAuthority *authority, GAsyncReadyCallback callback, gpointer user_data) { - GVariant *identity_value; /* Note that in reality, this API is only accessible to root, and * only called from the setuid helper `polkit-agent-helper-1`. * @@ -1526,14 +1506,12 @@ polkit_authority_authentication_agent_response (PolkitAuthority *authority, g_return_if_fail (POLKIT_IS_IDENTITY (identity)); g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable)); - identity_value = polkit_identity_to_gvariant (identity); - g_variant_ref_sink (identity_value); g_dbus_proxy_call (authority->proxy, "AuthenticationAgentResponse2", g_variant_new ("(us@(sa{sv}))", (guint32)uid, cookie, - identity_value), + polkit_identity_to_gvariant (identity)), /* A floating value */ G_DBUS_CALL_FLAGS_NONE, -1, cancellable, @@ -1542,7 +1520,6 @@ polkit_authority_authentication_agent_response (PolkitAuthority *authority, callback, user_data, polkit_authority_authentication_agent_response)); - g_variant_unref (identity_value); } /** @@ -1653,18 +1630,14 @@ polkit_authority_enumerate_temporary_authorizations (PolkitAuthority *author GAsyncReadyCallback callback, gpointer user_data) { - GVariant *subject_value; - g_return_if_fail (POLKIT_IS_AUTHORITY (authority)); g_return_if_fail (POLKIT_IS_SUBJECT (subject)); g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable)); - subject_value = polkit_subject_to_gvariant (subject); - g_variant_ref_sink (subject_value); g_dbus_proxy_call (authority->proxy, "EnumerateTemporaryAuthorizations", g_variant_new ("(@(sa{sv}))", - subject_value), + polkit_subject_to_gvariant (subject)), /* A floating value */ G_DBUS_CALL_FLAGS_NONE, -1, cancellable, @@ -1673,7 +1646,6 @@ polkit_authority_enumerate_temporary_authorizations (PolkitAuthority *author callback, user_data, polkit_authority_enumerate_temporary_authorizations)); - g_variant_unref (subject_value); } /** @@ -1726,11 +1698,13 @@ polkit_authority_enumerate_temporary_authorizations_finish (PolkitAuthority *aut g_prefix_error (error, "Error serializing return value of EnumerateTemporaryAuthorizations: "); g_list_foreach (ret, (GFunc) g_object_unref, NULL); g_list_free (ret); - goto out; + ret = NULL; + goto out_array; } ret = g_list_prepend (ret, auth); } ret = g_list_reverse (ret); + out_array: g_variant_unref (array); g_variant_unref (value); @@ -1805,18 +1779,14 @@ polkit_authority_revoke_temporary_authorizations (PolkitAuthority *authority GAsyncReadyCallback callback, gpointer user_data) { - GVariant *subject_value; - g_return_if_fail (POLKIT_IS_AUTHORITY (authority)); g_return_if_fail (POLKIT_IS_SUBJECT (subject)); g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable)); - subject_value = polkit_subject_to_gvariant (subject); - g_variant_ref_sink (subject_value); g_dbus_proxy_call (authority->proxy, "RevokeTemporaryAuthorizations", g_variant_new ("(@(sa{sv}))", - subject_value), + polkit_subject_to_gvariant (subject)), /* A floating value */ G_DBUS_CALL_FLAGS_NONE, -1, cancellable, @@ -1825,7 +1795,6 @@ polkit_authority_revoke_temporary_authorizations (PolkitAuthority *authority callback, user_data, polkit_authority_revoke_temporary_authorizations)); - g_variant_unref (subject_value); } /** diff --git a/src/polkit/polkitauthorizationresult.c b/src/polkit/polkitauthorizationresult.c index dd3d761..877a9a6 100644 --- a/src/polkit/polkitauthorizationresult.c +++ b/src/polkit/polkitauthorizationresult.c @@ -290,19 +290,15 @@ polkit_authorization_result_new_for_gvariant (GVariant *value) return ret; } +/* Note that this returns a floating value. */ GVariant * polkit_authorization_result_to_gvariant (PolkitAuthorizationResult *authorization_result) { - GVariant *ret; - GVariant *details_gvariant; - - details_gvariant = polkit_details_to_gvariant (polkit_authorization_result_get_details (authorization_result)); - g_variant_ref_sink (details_gvariant); - ret = g_variant_new ("(bb@a{ss})", - polkit_authorization_result_get_is_authorized (authorization_result), - polkit_authorization_result_get_is_challenge (authorization_result), - details_gvariant); - g_variant_unref (details_gvariant); + PolkitDetails *details; - return ret; + details = polkit_authorization_result_get_details (authorization_result); + return g_variant_new ("(bb@a{ss})", + polkit_authorization_result_get_is_authorized (authorization_result), + polkit_authorization_result_get_is_challenge (authorization_result), + polkit_details_to_gvariant (details)); /* A floating value */ } diff --git a/src/polkit/polkitdetails.c b/src/polkit/polkitdetails.c index 07a6f63..b16aadc 100644 --- a/src/polkit/polkitdetails.c +++ b/src/polkit/polkitdetails.c @@ -195,10 +195,10 @@ polkit_details_get_keys (PolkitDetails *details) return ret; } +/* Note that this returns a floating value. */ GVariant * polkit_details_to_gvariant (PolkitDetails *details) { - GVariant *ret; GVariantBuilder builder; g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{ss}")); @@ -212,8 +212,7 @@ polkit_details_to_gvariant (PolkitDetails *details) while (g_hash_table_iter_next (&hash_iter, (gpointer) &key, (gpointer) &value)) g_variant_builder_add (&builder, "{ss}", key, value); } - ret = g_variant_builder_end (&builder); - return ret; + return g_variant_builder_end (&builder); } PolkitDetails * diff --git a/src/polkit/polkitidentity.c b/src/polkit/polkitidentity.c index 7813c2c..3aa1f7f 100644 --- a/src/polkit/polkitidentity.c +++ b/src/polkit/polkitidentity.c @@ -198,12 +198,12 @@ polkit_identity_from_string (const gchar *str, return identity; } +/* Note that this returns a floating value. */ GVariant * polkit_identity_to_gvariant (PolkitIdentity *identity) { GVariantBuilder builder; GVariant *dict; - GVariant *ret; const gchar *kind; kind = ""; @@ -233,8 +233,7 @@ polkit_identity_to_gvariant (PolkitIdentity *identity) } dict = g_variant_builder_end (&builder); - ret = g_variant_new ("(s@a{sv})", kind, dict); - return ret; + return g_variant_new ("(s@a{sv})", kind, dict); } static GVariant * @@ -267,6 +266,7 @@ lookup_asv (GVariant *dict, g_variant_get_type_string (value), type_string); g_free (type_string); + g_variant_unref (value); goto out; } ret = value; diff --git a/src/polkit/polkitsubject.c b/src/polkit/polkitsubject.c index df8e1aa..d4c1182 100644 --- a/src/polkit/polkitsubject.c +++ b/src/polkit/polkitsubject.c @@ -290,12 +290,12 @@ polkit_subject_from_string (const gchar *str, return subject; } +/* Note that this returns a floating value. */ GVariant * polkit_subject_to_gvariant (PolkitSubject *subject) { GVariantBuilder builder; GVariant *dict; - GVariant *ret; const gchar *kind; kind = ""; @@ -329,8 +329,7 @@ polkit_subject_to_gvariant (PolkitSubject *subject) } dict = g_variant_builder_end (&builder); - ret = g_variant_new ("(s@a{sv})", kind, dict); - return ret; + return g_variant_new ("(s@a{sv})", kind, dict); } static GVariant * @@ -363,6 +362,7 @@ lookup_asv (GVariant *dict, g_variant_get_type_string (value), type_string); g_free (type_string); + g_variant_unref (value); goto out; } ret = value; diff --git a/src/polkit/polkittemporaryauthorization.c b/src/polkit/polkittemporaryauthorization.c index b2c6003..5e07678 100644 --- a/src/polkit/polkittemporaryauthorization.c +++ b/src/polkit/polkittemporaryauthorization.c @@ -212,22 +212,15 @@ polkit_temporary_authorization_new_for_gvariant (GVariant *value, return authorization; } +/* Note that this returns a floating value. */ GVariant * polkit_temporary_authorization_to_gvariant (PolkitTemporaryAuthorization *authorization) { - GVariant *ret; - GVariant *subject_gvariant; - - subject_gvariant = polkit_subject_to_gvariant (authorization->subject); - g_variant_ref_sink (subject_gvariant); - ret = g_variant_new ("(ss@(sa{sv})tt)", - authorization->id, - authorization->action_id, - subject_gvariant, - authorization->time_obtained, - authorization->time_expires); - g_variant_unref (subject_gvariant); - - return ret; + return g_variant_new ("(ss@(sa{sv})tt)", + authorization->id, + authorization->action_id, + polkit_subject_to_gvariant (authorization->subject), /* A floating value */ + authorization->time_obtained, + authorization->time_expires); } diff --git a/src/polkitbackend/polkitbackendauthority.c b/src/polkitbackend/polkitbackendauthority.c index 64560e1..0d1fac4 100644 --- a/src/polkitbackend/polkitbackendauthority.c +++ b/src/polkitbackend/polkitbackendauthority.c @@ -645,11 +645,8 @@ server_handle_enumerate_actions (Server *server, for (l = actions; l != NULL; l = l->next) { PolkitActionDescription *ad = POLKIT_ACTION_DESCRIPTION (l->data); - GVariant *value; - value = polkit_action_description_to_gvariant (ad); - g_variant_ref_sink (value); - g_variant_builder_add_value (&builder, value); - g_variant_unref (value); + g_variant_builder_add_value (&builder, + polkit_action_description_to_gvariant (ad)); /* A floating value */ } g_dbus_method_invocation_return_value (invocation, g_variant_new ("(a(ssssssuuua{ss}))", &builder)); @@ -709,11 +706,9 @@ check_auth_cb (GObject *source_object, } else { - GVariant *value; - value = polkit_authorization_result_to_gvariant (result); - g_variant_ref_sink (value); - g_dbus_method_invocation_return_value (data->invocation, g_variant_new ("(@(bba{ss}))", value)); - g_variant_unref (value); + g_dbus_method_invocation_return_value (data->invocation, + g_variant_new ("(@(bba{ss}))", + polkit_authorization_result_to_gvariant (result))); /* A floating value */ g_object_unref (result); } @@ -956,6 +951,7 @@ server_handle_register_authentication_agent_with_options (Server g_dbus_method_invocation_return_value (invocation, g_variant_new ("()")); out: + g_variant_unref (subject_gvariant); if (options != NULL) g_variant_unref (options); if (subject != NULL) @@ -1007,6 +1003,7 @@ server_handle_unregister_authentication_agent (Server *server, g_dbus_method_invocation_return_value (invocation, g_variant_new ("()")); out: + g_variant_unref (subject_gvariant); if (subject != NULL) g_object_unref (subject); } @@ -1057,6 +1054,7 @@ server_handle_authentication_agent_response (Server *server, g_dbus_method_invocation_return_value (invocation, g_variant_new ("()")); out: + g_variant_unref (identity_gvariant); if (identity != NULL) g_object_unref (identity); } @@ -1107,6 +1105,7 @@ server_handle_authentication_agent_response2 (Server *server, g_dbus_method_invocation_return_value (invocation, g_variant_new ("()")); out: + g_variant_unref (identity_gvariant); if (identity != NULL) g_object_unref (identity); } @@ -1158,17 +1157,15 @@ server_handle_enumerate_temporary_authorizations (Server *server for (l = authorizations; l != NULL; l = l->next) { PolkitTemporaryAuthorization *a = POLKIT_TEMPORARY_AUTHORIZATION (l->data); - GVariant *value; - value = polkit_temporary_authorization_to_gvariant (a); - g_variant_ref_sink (value); - g_variant_builder_add_value (&builder, value); - g_variant_unref (value); + g_variant_builder_add_value (&builder, + polkit_temporary_authorization_to_gvariant (a)); /* A floating value */ } g_list_foreach (authorizations, (GFunc) g_object_unref, NULL); g_list_free (authorizations); g_dbus_method_invocation_return_value (invocation, g_variant_new ("(a(ss(sa{sv})tt))", &builder)); out: + g_variant_unref (subject_gvariant); if (subject != NULL) g_object_unref (subject); } @@ -1215,6 +1212,7 @@ server_handle_revoke_temporary_authorizations (Server *server, g_dbus_method_invocation_return_value (invocation, g_variant_new ("()")); out: + g_variant_unref (subject_gvariant); if (subject != NULL) g_object_unref (subject); } diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c index ccfd608..1cd60d3 100644 --- a/src/polkitbackend/polkitbackendinteractiveauthority.c +++ b/src/polkitbackend/polkitbackendinteractiveauthority.c @@ -1906,15 +1906,15 @@ authentication_agent_begin_cb (GDBusProxy *proxy, AuthenticationSession *session = user_data; gboolean gained_authorization; gboolean was_dismissed; + GVariant *result; GError *error; was_dismissed = FALSE; gained_authorization = FALSE; error = NULL; - if (!g_dbus_proxy_call_finish (proxy, - res, - &error)) + result = g_dbus_proxy_call_finish (proxy, res, &error); + if (result == NULL) { g_printerr ("Error performing authentication: %s (%s %d)\n", error->message, @@ -1926,6 +1926,7 @@ authentication_agent_begin_cb (GDBusProxy *proxy, } else { + g_variant_unref (result); gained_authorization = session->is_authenticated; g_debug ("Authentication complete, is_authenticated = %d", session->is_authenticated); } @@ -2299,7 +2300,6 @@ authentication_agent_initiate_challenge (AuthenticationAgent *agent, gchar *localized_message; gchar *localized_icon_name; PolkitDetails *localized_details; - GVariant *details_gvariant; GList *user_identities = NULL; GVariantBuilder identities_builder; GVariant *parameters; @@ -2397,28 +2397,21 @@ authentication_agent_initiate_challenge (AuthenticationAgent *agent, add_pid (localized_details, caller, "polkit.caller-pid"); add_pid (localized_details, subject, "polkit.subject-pid"); - details_gvariant = polkit_details_to_gvariant (localized_details); - g_variant_ref_sink (details_gvariant); - g_variant_builder_init (&identities_builder, G_VARIANT_TYPE ("a(sa{sv})")); for (l = user_identities; l != NULL; l = l->next) { PolkitIdentity *identity = POLKIT_IDENTITY (l->data); - GVariant *value; - value = polkit_identity_to_gvariant (identity); - g_variant_ref_sink (value); - g_variant_builder_add_value (&identities_builder, value); - g_variant_unref (value); + g_variant_builder_add_value (&identities_builder, + polkit_identity_to_gvariant (identity)); /* A floating value */ } parameters = g_variant_new ("(sss@a{ss}sa(sa{sv}))", action_id, localized_message, localized_icon_name, - details_gvariant, + polkit_details_to_gvariant (localized_details), /* A floating value */ session->cookie, &identities_builder); - g_variant_unref (details_gvariant); g_dbus_proxy_call (agent->proxy, "BeginAuthentication", @@ -2444,13 +2437,18 @@ authentication_agent_cancel_cb (GDBusProxy *proxy, GAsyncResult *res, gpointer user_data) { + GVariant *result; GError *error; + error = NULL; - if (!g_dbus_proxy_call_finish (proxy, res, &error)) + result = g_dbus_proxy_call_finish (proxy, res, &error); + if (result == NULL) { g_printerr ("Error cancelling authentication: %s\n", error->message); g_error_free (error); } + else + g_variant_unref (result); } static void |