libnm: fix dangling pointer in public API while destructing NMClient

While (and after) NMClient gets destroyed, nm_device_get_active_connection()
gives a dangling pointer. That can lead to a crash. This probably
affects all NMLDBusPropertyO type properties.

It's not clear how to fix that best. Usually, NMClient does updates in
two phases, first it processes the D-Bus events and tracks internal
data, then it emits all GObject signals and notifications.

When an object gets removed from the NMClient cache, then the second
phase is not fully processed, because the object is already removed
from the cache. Thus, the property was not properly cleared leaving
a dangling pointer.

A simple fix is to always clear the pointer during the first phase. Note that
effectively we do the same also for NMLDBusPropertyAO (by clearing the
"pr_ao->arr"), so at least this is consistent.

Somehow it seems that we should make sure that the "second" phase gets
full processed in this case too. But it's complicated, and it's not
clear how to do that. So this solution seems fine.

https://bugzilla.redhat.com/show_bug.cgi?id=2039331
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/896

2 files changed, 3 insertions, 2 deletions

diff --git a/src/libnm-client-impl/nm-client.c b/src/libnm-client-impl/nm-client.c
index 27382ec8b9..a2797f905a 100644
--- a/src/libnm-client-impl/nm-client.c
+++ b/src/libnm-client-impl/nm-client.c
@@ -1799,6 +1799,7 @@ nml_dbus_property_o_notify(NMClient               *self,
 
     if (pr_o->obj_watcher
         && (!dbus_path || !nm_streq(dbus_path, pr_o->obj_watcher->dbobj->dbus_path->str))) {
+        pr_o->nmobj = NULL;
         _dbobjs_obj_watcher_unregister(self, g_steal_pointer(&pr_o->obj_watcher));
         changed = TRUE;
     }
diff --git a/src/libnm-client-impl/tests/test-nm-client.c b/src/libnm-client-impl/tests/test-nm-client.c
index ff821d6be4..7e67d2a004 100644
--- a/src/libnm-client-impl/tests/test-nm-client.c
+++ b/src/libnm-client-impl/tests/test-nm-client.c
@@ -811,7 +811,7 @@ _dev_eth0_1_state_changed_cb(NMDevice           *device,
     g_assert(arr);
     g_assert_cmpint(arr->len, ==, 0);
 
-    // g_assert(!nm_device_get_active_connection(device));
+    g_assert(!nm_device_get_active_connection(device));
 }
 
 static void
@@ -920,7 +920,7 @@ test_activate_virtual(void)
             g_assert(arr);
             g_assert_cmpint(arr->len, ==, 0);
 
-            // g_assert(!nm_device_get_active_connection(dev_eth0_1));
+            g_assert(!nm_device_get_active_connection(dev_eth0_1));
 
             nm_clear_g_signal_handler(dev_eth0_1, &sig_id);