colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libnm: fix dangling pointer in public API while destructing NMClient

Browse Source 
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
This commit is contained in:
Thomas Haller
2022-01-18 08:41:43 +01:00
parent bc9aa72c88
commit 2afecaf908
2 changed files with 3 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/libnm-client-impl/nm-client.c
View File

@@ -1799,6 +1799,7 @@ nml_dbus_property_o_notify(NMClient *self,
if (pr_o->obj_watcher if (pr_o->obj_watcher
&& (!dbus_path || !nm_streq(dbus_path, pr_o->obj_watcher->dbobj->dbus_path->str))) { && (!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)); _dbobjs_obj_watcher_unregister(self, g_steal_pointer(&pr_o->obj_watcher));
changed = TRUE; changed = TRUE;
} }

4
src/libnm-client-impl/tests/test-nm-client.c
View File

@@ -811,7 +811,7 @@ _dev_eth0_1_state_changed_cb(NMDevice *device,
g_assert(arr); g_assert(arr);
g_assert_cmpint(arr->len, ==, 0); g_assert_cmpint(arr->len, ==, 0);
// g_assert(!nm_device_get_active_connection(device)); g_assert(!nm_device_get_active_connection(device));
} }
static void static void
@@ -920,7 +920,7 @@ test_activate_virtual(void)
g_assert(arr); g_assert(arr);
g_assert_cmpint(arr->len, ==, 0); 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); nm_clear_g_signal_handler(dev_eth0_1, &sig_id);