Blob Blame History Raw
From 602f73a2e838ea1055ef5e6913aec3d8a87ed610 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Thu, 13 May 2021 10:49:39 +0200
Subject: [PATCH 1/2] manager: fix active_connection_find()

Commit 33b9fa3a3caf ("manager: Keep volatile/external connections
while referenced by async_op_lst") changed active_connection_find() to
also return active connections that are not yet activating but are
waiting authorization.

This has side effect for other callers of the function. In particular,
_get_activatable_connections_filter() should exclude only ACs that are
really active, not those waiting for authorization.

Otherwise, in ensure_master_active_connection() all the ACs waiting
authorization are missed and we might fail to find the right master
AC.

Add an argument to active_connection_find to select whether include
ACs waiting authorization.

Fixes: 33b9fa3a3caf ('manager: Keep volatile/external connections while referenced by async_op_lst')

https://bugzilla.redhat.com/show_bug.cgi?id=1955101
(cherry picked from commit e694f2cec1a0e7bc188776c8573e07a4d57851dc)
(cherry picked from commit fc611f60470160fe98512c405d6785f2b24b98a1)
---
 src/core/nm-manager.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c
index 5a6e05a934..c751b2db50 100644
--- a/src/core/nm-manager.c
+++ b/src/core/nm-manager.c
@@ -363,6 +363,7 @@ static NMActiveConnection *active_connection_find(NMManager *             self,
                                                   NMSettingsConnection *  sett_conn,
                                                   const char *            uuid,
                                                   NMActiveConnectionState max_state,
+                                                  gboolean                also_waiting_auth,
                                                   GPtrArray **            out_all_matching);
 
 static NMConnectivity *concheck_get_mgr(NMManager *self);
@@ -833,6 +834,7 @@ _delete_volatile_connection_do(NMManager *self, NMSettingsConnection *connection
                                connection,
                                NULL,
                                NM_ACTIVE_CONNECTION_STATE_DEACTIVATED,
+                               TRUE,
                                NULL))
         return;
 
@@ -978,6 +980,7 @@ active_connection_find(
     NMSettingsConnection *  sett_conn,
     const char *            uuid,
     NMActiveConnectionState max_state /* candidates in state @max_state will be found */,
+    gboolean                also_waiting_auth /* return also ACs waiting authorization */,
     GPtrArray **            out_all_matching)
 {
     NMManagerPrivate *  priv = NM_MANAGER_GET_PRIVATE(self);
@@ -1017,6 +1020,9 @@ active_connection_find(
     if (!best_ac) {
         AsyncOpData *async_op_data;
 
+        if (!also_waiting_auth)
+            return NULL;
+
         c_list_for_each_entry (async_op_data, &priv->async_op_lst_head, async_op_lst) {
             NMSettingsConnection *ac_conn;
 
@@ -1078,6 +1084,7 @@ active_connection_find_by_connection(NMManager *             self,
                                   sett_conn,
                                   sett_conn ? NULL : nm_connection_get_uuid(connection),
                                   max_state,
+                                  FALSE,
                                   out_all_matching);
 }
 
@@ -1112,6 +1119,7 @@ _get_activatable_connections_filter(NMSettings *          settings,
                                    sett_conn,
                                    NULL,
                                    NM_ACTIVE_CONNECTION_STATE_ACTIVATED,
+                                   FALSE,
                                    NULL);
 }
 
@@ -2245,6 +2253,7 @@ connection_flags_changed(NMSettings *settings, NMSettingsConnection *connection,
                                connection,
                                NULL,
                                NM_ACTIVE_CONNECTION_STATE_DEACTIVATED,
+                               FALSE,
                                NULL)) {
         /* the connection still has an active-connection. It will be purged
          * when the active connection(s) get(s) removed. */
@@ -2564,6 +2573,7 @@ new_activation_allowed_for_connection(NMManager *self, NMSettingsConnection *con
                                    connection,
                                    NULL,
                                    NM_ACTIVE_CONNECTION_STATE_ACTIVATED,
+                                   FALSE,
                                    NULL);
 }
 
@@ -4134,6 +4144,7 @@ find_master(NMManager *            self,
                                                 master_connection,
                                                 NULL,
                                                 NM_ACTIVE_CONNECTION_STATE_DEACTIVATING,
+                                                FALSE,
                                                 NULL);
     }
 
@@ -4985,6 +4996,7 @@ _internal_activate_device(NMManager *self, NMActiveConnection *active, GError **
                                     sett_conn,
                                     NULL,
                                     NM_ACTIVE_CONNECTION_STATE_ACTIVATED,
+                                    FALSE,
                                     &all_ac_arr);
         if (ac) {
             n_all = all_ac_arr ? all_ac_arr->len : ((guint) 1);
-- 
2.26.3


From cf2f0e5a5180d2333ce33b925d54c8b7b925e094 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Fri, 30 Apr 2021 22:35:20 +0200
Subject: [PATCH 2/2] manager: fix assertion failure in
 active_connection_find()

Active-connections in the async_op_lst are not guaranteed to have a
settings-connection. In particular, the settings-connection for an
AddAndActivate() AC is set only after the authorization succeeds. Use
the non-asserting variant of the function to fix the following
failure:

 nm_active_connection_get_settings_connection: assertion 'sett_conn' failed

 1    _g_log_abort()
 2    g_logv()
 3    g_log()
 4    _nm_g_return_if_fail_warning.constprop.14()
 5    nm_active_connection_get_settings_connection()
 6    active_connection_find()
 7    _get_activatable_connections_filter()
 8    nm_settings_get_connections_clone()
 9    nm_manager_get_activatable_connections()
 10   auto_activate_device_cb()
 11   g_idle_dispatch()
 12   g_main_context_dispatch()
 13   g_main_context_iterate.isra.21()
 14   g_main_loop_run()
 15   main()

Fixes: 33b9fa3a3caf ('manager: Keep volatile/external connections while referenced by async_op_lst')

https://bugzilla.redhat.com/show_bug.cgi?id=1933719
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/834
(cherry picked from commit 23cc0bf3353ea43d95a906e27c9881b1b68e2bbe)
(cherry picked from commit d0b0c65905ae19145d1c1f2912aa580a3b0a36e0)
---
 src/core/nm-manager.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c
index c751b2db50..3405de86ee 100644
--- a/src/core/nm-manager.c
+++ b/src/core/nm-manager.c
@@ -1027,7 +1027,7 @@ active_connection_find(
             NMSettingsConnection *ac_conn;
 
             ac      = async_op_data->ac_auth.active;
-            ac_conn = nm_active_connection_get_settings_connection(ac);
+            ac_conn = _nm_active_connection_get_settings_connection(ac);
             if (sett_conn && sett_conn != ac_conn)
                 continue;
             if (uuid && !nm_streq0(uuid, nm_settings_connection_get_uuid(ac_conn)))
-- 
2.26.3