From abef55ece4d33b1a7e887bdf5452a48040d8ee8c Mon Sep 17 00:00:00 2001

From: Beniamino Galvani <bgalvani@redhat.com>

Date: Mon, 18 Dec 2017 17:25:10 +0100

Subject: [PATCH 1/1] settings: avoid assertion when deleting connections

If a volatile connection is deleted by user when it was already being

deleted internally because the device vanished, we may hit the

following failed assertion:

 file src/settings/nm-settings-connection.c: line 2196

 (nm_settings_connection_signal_remove): should not be reached

The @removed flag keeps track of whether we already signaled the

connection removal. Instead of throwing an assertion if we try to emit

the signal again, just return without action because this can happen

in the situation described above.

While at it, remove the @allow_reuse argument from

nm_settings_connection_signal_remove(): we should never emit the

signal twice. Instead, we should reset the @removed flag when the

connection is added.

Fixes: a9384452ed61ca3f1c6e1db175f499307da9c388

https://bugzilla.redhat.com/show_bug.cgi?id=1506552

(cherry picked from commit 98ac0f404ed8103324b7d03cf04c24946f4543ff)

(cherry picked from commit 39e1c65494d489b2cd9d63140c98d4fb4185003a)

---

 src/settings/nm-settings-connection.c              | 23 +++++++++++++++-------

 src/settings/nm-settings-connection.h              |  4 +++-

 src/settings/nm-settings.c                         |  2 ++

 .../plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c         |  4 ++--

 src/settings/plugins/ifnet/nms-ifnet-plugin.c      |  4 ++--

 src/settings/plugins/keyfile/nms-keyfile-plugin.c  |  2 +-

 6 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c

index 62f2de399..37a0b3a6c 100644

--- a/src/settings/nm-settings-connection.c

+++ b/src/settings/nm-settings-connection.c

@@ -794,7 +794,7 @@ nm_settings_connection_delete (NMSettingsConnection *self,

 	/* Remove connection from seen-bssids database file */

 	remove_entry_from_db (self, "seen-bssids");

-	nm_settings_connection_signal_remove (self, FALSE);

+	nm_settings_connection_signal_remove (self);

 	return TRUE;

 }

@@ -2188,15 +2188,24 @@ impl_settings_connection_clear_secrets (NMSettingsConnection *self,

 /*****************************************************************************/

 void

-nm_settings_connection_signal_remove (NMSettingsConnection *self, gboolean allow_reuse)

+nm_settings_connection_added (NMSettingsConnection *self)

 {

 	NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);

-	if (!allow_reuse) {

-		if (priv->removed)

-			g_return_if_reached ();

-		priv->removed = TRUE;

-	}

+	/* FIXME: we should always dispose connections that are removed

+	 * and not reuse them, but currently plugins keep alive unmanaged

+	 * (e.g. NM_CONTROLLED=no) connections. */

+	priv->removed = FALSE;

+}

+

+void

+nm_settings_connection_signal_remove (NMSettingsConnection *self)

+{

+	NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);

+

+	if (priv->removed)

+		return;

+	priv->removed = TRUE;

 	g_signal_emit_by_name (self, NM_SETTINGS_CONNECTION_REMOVED);

 }

diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h

index d7a999697..29ec05dd1 100644

--- a/src/settings/nm-settings-connection.h

+++ b/src/settings/nm-settings-connection.h

@@ -193,7 +193,9 @@ void nm_settings_connection_recheck_visibility (NMSettingsConnection *self);

 gboolean nm_settings_connection_check_permission (NMSettingsConnection *self,

                                                   const char *permission);

-void nm_settings_connection_signal_remove (NMSettingsConnection *self, gboolean allow_reuse);

+void nm_settings_connection_added (NMSettingsConnection *self);

+

+void nm_settings_connection_signal_remove (NMSettingsConnection *self);

 gboolean nm_settings_connection_get_unsaved (NMSettingsConnection *self);

diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c

index 21fdf9e06..d54305c13 100644

--- a/src/settings/nm-settings.c

+++ b/src/settings/nm-settings.c

@@ -1004,6 +1004,8 @@ claim_connection (NMSettings *self, NMSettingsConnection *connection)

 		/* Exported D-Bus signal */

 		g_signal_emit (self, signals[NEW_CONNECTION], 0, connection);

 	}

+

+	nm_settings_connection_added (connection);

 }

 static gboolean

diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c

index 2b388d1d9..0743fc9ff 100644

--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c

+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c

@@ -161,7 +161,7 @@ remove_connection (SettingsPluginIfcfg *self, NMIfcfgConnection *connection)

 	g_object_ref (connection);

 	g_hash_table_remove (priv->connections, nm_connection_get_uuid (NM_CONNECTION (connection)));

 	if (!unmanaged && !unrecognized)

-		nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection), FALSE);

+		nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection));

 	g_object_unref (connection);

 	/* Emit changes _after_ removing the connection */

@@ -331,7 +331,7 @@ update_connection (SettingsPluginIfcfg *self,

 					/* Unexport the connection by telling the settings service it's

 					 * been removed.

 					 */

-					nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection_by_uuid), TRUE);

+					nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection_by_uuid));

 					/* Remove the path so that claim_connection() doesn't complain later when

 					 * interface gets managed and connection is re-added. */

 					nm_connection_set_path (NM_CONNECTION (connection_by_uuid), NULL);

diff --git a/src/settings/plugins/ifnet/nms-ifnet-plugin.c b/src/settings/plugins/ifnet/nms-ifnet-plugin.c

index d89e988ba..8332358f9 100644

--- a/src/settings/plugins/ifnet/nms-ifnet-plugin.c

+++ b/src/settings/plugins/ifnet/nms-ifnet-plugin.c

@@ -262,7 +262,7 @@ reload_connections (NMSettingsPlugin *config)

 				                              NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)) {

 					nm_log_info (LOGD_SETTINGS, "Auto refreshing %s", conn_name);

-					nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (old), FALSE);

+					nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (old));

 					track_new_connection (self, new);

 					if (is_managed_plugin () && is_managed (conn_name))

 						g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_CONNECTION_ADDED, new);

@@ -305,7 +305,7 @@ reload_connections (NMSettingsPlugin *config)

 		 */

 		if (   nm_ifnet_connection_get_conn_name (NM_IFNET_CONNECTION (candidate))

 		    && !g_hash_table_lookup (new_connections, uuid)) {

-			nm_settings_connection_signal_remove (candidate, FALSE);

+			nm_settings_connection_signal_remove (candidate);

 			g_hash_table_iter_remove (&iter);

 		}

 	}

diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c

index 222768db4..cb5f2c9f0 100644

--- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c

+++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c

@@ -107,7 +107,7 @@ remove_connection (NMSKeyfilePlugin *self, NMSKeyfileConnection *connection)

 	g_signal_handlers_disconnect_by_func (connection, connection_removed_cb, self);

 	removed = g_hash_table_remove (NMS_KEYFILE_PLUGIN_GET_PRIVATE (self)->connections,

 	                               nm_connection_get_uuid (NM_CONNECTION (connection)));

-	nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection), FALSE);

+	nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection));

 	g_object_unref (connection);

 	g_return_if_fail (removed);

-- 

2.14.3