core: unexport dbus-objects on dispose

When the D-Bus name is already taken, NM crashes in the following
way. That's because disposed object are not unexported when quitting
and so they linger in the bus-manager's list of exported objects,
causing an invalid access when a neighboring item is accessed. Instead
of just clearing the path, fully unexport the object.

The behavior of not forcefully exporting objects on quit was added in
f9ee20a7b2 ("core: explicitly unexport objects when we're done with
them"), but such behavior doesn't seem to be needed by the stated
goal.

 <error> [1524062008.1886] bus-manager: fatal failure to acquire D-Bus service "org.freedesktop.NetworkManager" (3). Service already taken
 <trace> [1524062008.2327] config: state: success writing state file "/var/lib/NetworkManager/NetworkManager.state"
 <trace> [1524062008.2338] dns-mgr: stopping...
 <info>  [1524062008.2344] exiting (error)
 <debug> [1524062008.2628] disposing NMManager singleton (0xce587e0)
 <trace> [1524062008.2640] dns-mgr: disposing
 <debug> [1524062008.2651] disposing NMDnsManager singleton (0xceb8b50)
 <debug> [1524062008.2666] disposing NMFirewallManager singleton (0xceb62b0)
 <debug> [1524062008.2709] disposing NMHostnameManager singleton (0xce7b370)
 <trace> [1524062008.2722] dbus-object[0xce70f40]: unexport: "/org/freedesktop/NetworkManager/AgentManager"
 ==16381== Invalid write of size 8
 ==16381==    at 0x42F511: c_list_unlink_stale (c-list.h:158)
 ==16381==    by 0x42F511: c_list_unlink (c-list.h:171)
 ==16381==    by 0x42F511: _nm_dbus_manager_obj_unexport (nm-dbus-manager.c:1135)
 ==16381==    by 0x4C5E35: nm_dbus_object_unexport (nm-dbus-object.c:165)
 ==16381==    by 0x5C01E9: dispose (nm-agent-manager.c:1634)
 ==16381==    by 0x6636F37: g_object_unref (gobject.c:3303)
 ==16381==    by 0x4BDC89: _nm_singleton_instance_destroy (nm-core-utils.c:138)
 ==16381==    by 0x400FA85: _dl_fini (in /usr/lib64/ld-2.27.so)
 ==16381==    by 0x7F806AB: __run_exit_handlers (in /usr/lib64/libc-2.27.so)
 ==16381==    by 0x7F807DB: exit (in /usr/lib64/libc-2.27.so)
 ==16381==    by 0x41DA34: main (main.c:463)
 ==16381==  Address 0xce706a0 is 48 bytes inside a block of size 176 free'd
 ==16381==    at 0x4C2EDAC: free (vg_replace_malloc.c:530)
 ==16381==    by 0x6ACA3E1: g_free (gmem.c:194)
 ==16381==    by 0x6AE2572: g_slice_free1 (gslice.c:1136)
 ==16381==    by 0x66550AE: g_type_free_instance (gtype.c:1943)
 ==16381==    by 0x4505F8: dispose (nm-manager.c:6867)
 ==16381==    by 0x6636F37: g_object_unref (gobject.c:3303)
 ==16381==    by 0x4BDC89: _nm_singleton_instance_destroy (nm-core-utils.c:138)
 ==16381==    by 0x400FA85: _dl_fini (in /usr/lib64/ld-2.27.so)
 ==16381==    by 0x7F806AB: __run_exit_handlers (in /usr/lib64/libc-2.27.so)
 ==16381==    by 0x7F807DB: exit (in /usr/lib64/libc-2.27.so)
 ==16381==    by 0x41DA34: main (main.c:463)
 ==16381==  Block was alloc'd at
 ==16381==    at 0x4C2DBAB: malloc (vg_replace_malloc.c:299)
 ==16381==    by 0x6ACA2D5: g_malloc (gmem.c:99)
 ==16381==    by 0x6AE1E36: g_slice_alloc (gslice.c:1025)
 ==16381==    by 0x6AE247C: g_slice_alloc0 (gslice.c:1051)
 ==16381==    by 0x6654E09: g_type_create_instance (gtype.c:1848)
 ==16381==    by 0x66376C7: g_object_new_internal (gobject.c:1799)
 ==16381==    by 0x6638E14: g_object_new_with_properties (gobject.c:1967)
 ==16381==    by 0x66399D0: g_object_new (gobject.c:1639)
 ==16381==    by 0x5D6F18: nm_settings_new (nm-settings.c:1897)
 ==16381==    by 0x4514B4: constructed (nm-manager.c:6489)
 ==16381==    by 0x66378FA: g_object_new_internal (gobject.c:1839)
 ==16381==    by 0x6638E14: g_object_new_with_properties (gobject.c:1967)

https://github.com/NetworkManager/NetworkManager/pull/96
This commit is contained in:
Beniamino Galvani
2018-04-17 16:33:41 +02:00
parent 9967f099dd
commit 21d3f16809

View File

@@ -290,16 +290,10 @@ dispose (GObject *object)
/* Objects should have already been unexported by their owner, unless
* we are quitting, where many objects stick around until exit.
*/
if (!quitting) {
if (self->internal.path) {
if (self->internal.path) {
if (!quitting)
g_warn_if_reached ();
nm_dbus_object_unexport (self);
}
} else if (self->internal.path) {
/* FIXME: do a proper, coordinate shutdown, so that no objects stay
* alive nor exported. */
_emit_exported_changed (self);
nm_clear_g_free (&self->internal.path);
nm_dbus_object_unexport (self);
}
G_OBJECT_CLASS (nm_dbus_object_parent_class)->dispose (object);