Commit f8643cc0 authored by Dan Williams's avatar Dan Williams

system-settings: fix PK Authority object lifetimes

It's a singleton, but PolicyKit didn't increment the reference count
when returning from polkit_authority_get() like we expected (which has
since been fixed upstream).  So for now, just don't unref the authority
at all.

Since we don't do that, there's a chance that some PolicyKit calls could
be outstanding when either the NMSysconfigSettings object or one of the
NMSysconfigConnection objects are around, so we make sure we cancel any
PolicyKit calls when the object gets disposed.  This is tricky, because
canceling them from the dispose may mean that the callback gets called
after the object is actually destroyed, so we have to be careful not to
access any private object data from the callbacks in that situation.
parent 4b2c810b
......@@ -42,6 +42,7 @@ G_DEFINE_TYPE_EXTENDED (NMSysconfigConnection, nm_sysconfig_connection, NM_TYPE_
typedef struct {
PolkitAuthority *authority;
GSList *pk_calls;
} NMSysconfigConnectionPrivate;
/**************************************************************/
......@@ -167,6 +168,7 @@ typedef struct {
DBusGMethodInvocation *context;
PolkitSubject *subject;
GCancellable *cancellable;
gboolean disposed;
/* Update */
NMConnection *connection;
......@@ -242,11 +244,26 @@ pk_update_cb (GObject *object, GAsyncResult *result, gpointer user_data)
{
PolkitCall *call = user_data;
NMSysconfigConnection *self = call->self;
NMSysconfigConnectionPrivate *priv = NM_SYSCONFIG_CONNECTION_GET_PRIVATE (self);
NMSysconfigConnectionPrivate *priv;
PolkitAuthorizationResult *pk_result;
GError *error = NULL;
GHashTable *settings;
/* If our NMSysconfigConnection is already gone, do nothing */
if (call->disposed) {
error = g_error_new_literal (NM_SYSCONFIG_SETTINGS_ERROR,
NM_SYSCONFIG_SETTINGS_ERROR_GENERAL,
"Request was canceled.");
dbus_g_method_return_error (call->context, error);
g_error_free (error);
polkit_call_free (call);
return;
}
priv = NM_SYSCONFIG_CONNECTION_GET_PRIVATE (self);
priv->pk_calls = g_slist_remove (priv->pk_calls, call);
pk_result = polkit_authority_check_authorization_finish (priv->authority,
result,
&error);
......@@ -321,6 +338,7 @@ dbus_update (NMExportedConnection *exported,
call->cancellable,
pk_update_cb,
call);
priv->pk_calls = g_slist_prepend (priv->pk_calls, call);
}
static void
......@@ -343,10 +361,25 @@ pk_delete_cb (GObject *object, GAsyncResult *result, gpointer user_data)
{
PolkitCall *call = user_data;
NMSysconfigConnection *self = call->self;
NMSysconfigConnectionPrivate *priv = NM_SYSCONFIG_CONNECTION_GET_PRIVATE (self);
NMSysconfigConnectionPrivate *priv;
PolkitAuthorizationResult *pk_result;
GError *error = NULL;
/* If our NMSysconfigConnection is already gone, do nothing */
if (call->disposed) {
error = g_error_new_literal (NM_SYSCONFIG_SETTINGS_ERROR,
NM_SYSCONFIG_SETTINGS_ERROR_GENERAL,
"Request was canceled.");
dbus_g_method_return_error (call->context, error);
g_error_free (error);
polkit_call_free (call);
return;
}
priv = NM_SYSCONFIG_CONNECTION_GET_PRIVATE (self);
priv->pk_calls = g_slist_remove (priv->pk_calls, call);
pk_result = polkit_authority_check_authorization_finish (priv->authority,
result,
&error);
......@@ -396,6 +429,7 @@ dbus_delete (NMExportedConnection *exported,
call->cancellable,
pk_delete_cb,
call);
priv->pk_calls = g_slist_prepend (priv->pk_calls, call);
}
static void
......@@ -419,10 +453,25 @@ pk_secrets_cb (GObject *object, GAsyncResult *result, gpointer user_data)
{
PolkitCall *call = user_data;
NMSysconfigConnection *self = call->self;
NMSysconfigConnectionPrivate *priv = NM_SYSCONFIG_CONNECTION_GET_PRIVATE (self);
NMSysconfigConnectionPrivate *priv;
PolkitAuthorizationResult *pk_result;
GError *error = NULL;
/* If our NMSysconfigConnection is already gone, do nothing */
if (call->disposed) {
error = g_error_new_literal (NM_SYSCONFIG_SETTINGS_ERROR,
NM_SYSCONFIG_SETTINGS_ERROR_GENERAL,
"Request was canceled.");
dbus_g_method_return_error (call->context, error);
g_error_free (error);
polkit_call_free (call);
return;
}
priv = NM_SYSCONFIG_CONNECTION_GET_PRIVATE (self);
priv->pk_calls = g_slist_remove (priv->pk_calls, call);
pk_result = polkit_authority_check_authorization_finish (priv->authority,
result,
&error);
......@@ -478,6 +527,7 @@ dbus_get_secrets (NMExportedConnection *exported,
call->cancellable,
pk_secrets_cb,
call);
priv->pk_calls = g_slist_prepend (priv->pk_calls, call);
}
/**************************************************************/
......@@ -501,10 +551,19 @@ nm_sysconfig_connection_init (NMSysconfigConnection *self)
static void
dispose (GObject *object)
{
NMSysconfigConnectionPrivate *priv = NM_SYSCONFIG_CONNECTION_GET_PRIVATE (object);
NMSysconfigConnection *self = NM_SYSCONFIG_CONNECTION (object);
NMSysconfigConnectionPrivate *priv = NM_SYSCONFIG_CONNECTION_GET_PRIVATE (self);
GSList *iter;
if (priv->authority)
g_object_unref (priv->authority);
/* Cancel PolicyKit requests */
for (iter = priv->pk_calls; iter; iter = g_slist_next (iter)) {
PolkitCall *call = iter->data;
call->disposed = TRUE;
g_cancellable_cancel (call->cancellable);
}
g_slist_free (priv->pk_calls);
priv->pk_calls = NULL;
G_OBJECT_CLASS (nm_sysconfig_connection_parent_class)->dispose (object);
}
......
......@@ -79,8 +79,12 @@ static void unmanaged_specs_changed (NMSystemConfigInterface *config, gpointer u
typedef struct {
PolkitAuthority *authority;
guint auth_changed_id;
char *config_file;
GSList *pk_calls;
GSList *permissions_calls;
GSList *plugins;
gboolean connections_loaded;
GHashTable *connections;
......@@ -516,6 +520,8 @@ typedef struct {
NMSysconfigSettings *self;
DBusGMethodInvocation *context;
PolkitSubject *subject;
GCancellable *cancellable;
gboolean disposed;
NMConnection *connection;
NMSettingsAddConnectionFunc callback;
......@@ -545,6 +551,7 @@ polkit_call_new (NMSysconfigSettings *self,
call = g_malloc0 (sizeof (PolkitCall));
call->self = self;
call->cancellable = g_cancellable_new ();
call->context = context;
if (connection)
call->connection = g_object_ref (connection);
......@@ -567,6 +574,7 @@ polkit_call_free (PolkitCall *call)
{
if (call->connection)
g_object_unref (call->connection);
g_object_unref (call->cancellable);
g_free (call->hostname);
g_object_unref (call->subject);
g_free (call);
......@@ -610,10 +618,25 @@ pk_add_cb (GObject *object, GAsyncResult *result, gpointer user_data)
{
PolkitCall *call = user_data;
NMSysconfigSettings *self = call->self;
NMSysconfigSettingsPrivate *priv = NM_SYSCONFIG_SETTINGS_GET_PRIVATE (self);
NMSysconfigSettingsPrivate *priv;
PolkitAuthorizationResult *pk_result;
GError *error = NULL, *add_error = NULL;
/* If NMSysconfigSettings is already gone, do nothing */
if (call->disposed) {
error = g_error_new_literal (NM_SYSCONFIG_SETTINGS_ERROR,
NM_SYSCONFIG_SETTINGS_ERROR_GENERAL,
"Request was canceled.");
dbus_g_method_return_error (call->context, error);
g_error_free (error);
polkit_call_free (call);
return;
}
priv = NM_SYSCONFIG_SETTINGS_GET_PRIVATE (self);
priv->pk_calls = g_slist_remove (priv->pk_calls, call);
pk_result = polkit_authority_check_authorization_finish (priv->authority,
result,
&error);
......@@ -680,9 +703,10 @@ add_connection (NMSettingsService *service,
NM_SYSCONFIG_POLICY_ACTION_CONNECTION_MODIFY,
NULL,
POLKIT_CHECK_AUTHORIZATION_FLAGS_ALLOW_USER_INTERACTION,
NULL,
call->cancellable,
pk_add_cb,
call);
priv->pk_calls = g_slist_append (priv->pk_calls, call);
}
static void
......@@ -690,12 +714,27 @@ pk_hostname_cb (GObject *object, GAsyncResult *result, gpointer user_data)
{
PolkitCall *call = user_data;
NMSysconfigSettings *self = call->self;
NMSysconfigSettingsPrivate *priv = NM_SYSCONFIG_SETTINGS_GET_PRIVATE (self);
NMSysconfigSettingsPrivate *priv;
PolkitAuthorizationResult *pk_result;
GError *error = NULL;
GSList *iter;
gboolean success = FALSE;
/* If our NMSysconfigConnection is already gone, do nothing */
if (call->disposed) {
error = g_error_new_literal (NM_SYSCONFIG_SETTINGS_ERROR,
NM_SYSCONFIG_SETTINGS_ERROR_GENERAL,
"Request was canceled.");
dbus_g_method_return_error (call->context, error);
g_error_free (error);
polkit_call_free (call);
return;
}
priv = NM_SYSCONFIG_SETTINGS_GET_PRIVATE (self);
priv->pk_calls = g_slist_remove (priv->pk_calls, call);
pk_result = polkit_authority_check_authorization_finish (priv->authority,
result,
&error);
......@@ -767,9 +806,10 @@ impl_settings_save_hostname (NMSysconfigSettings *self,
NM_SYSCONFIG_POLICY_ACTION_HOSTNAME_MODIFY,
NULL,
POLKIT_CHECK_AUTHORIZATION_FLAGS_ALLOW_USER_INTERACTION,
NULL,
call->cancellable,
pk_hostname_cb,
call);
priv->pk_calls = g_slist_append (priv->pk_calls, call);
}
static void
......@@ -783,7 +823,9 @@ pk_authority_changed_cb (GObject *object, gpointer user_data)
typedef struct {
PolkitCall *pk_call;
const char *pk_action;
GCancellable *cancellable;
NMSettingsSystemPermissions permission;
gboolean disposed;
} PermissionsCall;
static void
......@@ -792,10 +834,18 @@ permission_call_done (GObject *object, GAsyncResult *result, gpointer user_data)
PermissionsCall *call = user_data;
PolkitCall *pk_call = call->pk_call;
NMSysconfigSettings *self = pk_call->self;
NMSysconfigSettingsPrivate *priv = NM_SYSCONFIG_SETTINGS_GET_PRIVATE (self);
NMSysconfigSettingsPrivate *priv;
PolkitAuthorizationResult *pk_result;
GError *error = NULL;
/* If NMSysconfigSettings is gone, just skip to the end */
if (call->disposed)
goto done;
priv = NM_SYSCONFIG_SETTINGS_GET_PRIVATE (self);
priv->permissions_calls = g_slist_remove (priv->permissions_calls, call);
pk_result = polkit_authority_check_authorization_finish (priv->authority,
result,
&error);
......@@ -820,12 +870,21 @@ permission_call_done (GObject *object, GAsyncResult *result, gpointer user_data)
g_object_unref (pk_result);
done:
pk_call->permissions_calls--;
if (pk_call->permissions_calls == 0) {
/* All the permissions calls are done, return the full permissions
* bitfield back to the user.
*/
dbus_g_method_return (pk_call->context, pk_call->permissions);
if (call->disposed) {
error = g_error_new_literal (NM_SYSCONFIG_SETTINGS_ERROR,
NM_SYSCONFIG_SETTINGS_ERROR_GENERAL,
"Request was canceled.");
dbus_g_method_return_error (pk_call->context, error);
g_error_free (error);
} else {
/* All the permissions calls are done, return the full permissions
* bitfield back to the user.
*/
dbus_g_method_return (pk_call->context, pk_call->permissions);
}
polkit_call_free (pk_call);
}
......@@ -850,6 +909,7 @@ start_permission_check (NMSysconfigSettings *self,
call->pk_call = pk_call;
call->pk_action = pk_action;
call->permission = permission;
call->cancellable = g_cancellable_new ();
pk_call->permissions_calls++;
......@@ -858,9 +918,10 @@ start_permission_check (NMSysconfigSettings *self,
pk_action,
NULL,
0,
NULL,
call->cancellable,
permission_call_done,
call);
priv->permissions_calls = g_slist_append (priv->permissions_calls, call);
}
static void
......@@ -1261,6 +1322,41 @@ nm_sysconfig_settings_new (const char *config_file,
/***************************************************************/
static void
dispose (GObject *object)
{
NMSysconfigSettings *self = NM_SYSCONFIG_SETTINGS (object);
NMSysconfigSettingsPrivate *priv = NM_SYSCONFIG_SETTINGS_GET_PRIVATE (self);
GSList *iter;
if (priv->auth_changed_id) {
g_signal_handler_disconnect (priv->authority, priv->auth_changed_id);
priv->auth_changed_id = 0;
}
/* Cancel PolicyKit requests */
for (iter = priv->pk_calls; iter; iter = g_slist_next (iter)) {
PolkitCall *call = iter->data;
call->disposed = TRUE;
g_cancellable_cancel (call->cancellable);
}
g_slist_free (priv->pk_calls);
priv->pk_calls = NULL;
/* Cancel PolicyKit permissions requests */
for (iter = priv->permissions_calls; iter; iter = g_slist_next (iter)) {
PermissionsCall *call = iter->data;
call->disposed = TRUE;
g_cancellable_cancel (call->cancellable);
}
g_slist_free (priv->permissions_calls);
priv->permissions_calls = NULL;
G_OBJECT_CLASS (nm_sysconfig_settings_parent_class)->dispose (object);
}
static void
finalize (GObject *object)
{
......@@ -1274,9 +1370,6 @@ finalize (GObject *object)
g_slist_foreach (priv->plugins, (GFunc) g_object_unref, NULL);
g_slist_free (priv->plugins);
if (priv->authority)
g_object_unref (priv->authority);
g_free (priv->orig_hostname);
g_free (priv->config_file);
......@@ -1334,6 +1427,7 @@ nm_sysconfig_settings_class_init (NMSysconfigSettingsClass *class)
/* virtual methods */
object_class->notify = notify;
object_class->get_property = get_property;
object_class->dispose = dispose;
object_class->finalize = finalize;
ss_class->list_connections = list_connections;
ss_class->add_connection = add_connection;
......@@ -1379,9 +1473,12 @@ nm_sysconfig_settings_init (NMSysconfigSettings *self)
priv->connections = g_hash_table_new_full (g_direct_hash, g_direct_equal, g_object_unref, NULL);
priv->authority = polkit_authority_get ();
if (priv->authority)
g_signal_connect (priv->authority, "changed", G_CALLBACK (pk_authority_changed_cb), self);
else
if (priv->authority) {
priv->auth_changed_id = g_signal_connect (priv->authority,
"changed",
G_CALLBACK (pk_authority_changed_cb),
self);
} else
g_warning ("%s: failed to create PolicyKit authority.", __func__);
/* Grab hostname on startup and use that if no plugins provide one */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment