From 0312711353b5cc924fccb5b94c34d4da8bf1f391 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 7 Jun 2022 23:15:47 +0200 Subject: [PATCH] device: fix memory leak l3cd instances must be removed from the old l3cfg before calling _cleanup_ip_pre(). Otherwise, _cleanup_ip_pre() unregisters them from the device, and later _dev_l3_register_l3cds(self, l3cfg_old, FALSE, FALSE) does nothing because the device doesn't have any l3cd. Previously the l3cds would linger in the l3cfg, keeping a reference to it and causing a memory leak; the leak was not detected by valgrind because the l3cfg was still referenced by the NMNetns. Fixes: 58287cbcc0c8 ('core: rework IP configuration in NetworkManager using layer 3 configuration') Fixes-test: @stable_mem_consumption2 https://bugzilla.redhat.com/show_bug.cgi?id=2083453 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1252 (cherry picked from commit f69a1cc874208a4d76bdfbdb55d223699aaba528) (cherry picked from commit 83ee0f0779960abf8f609750871fdc05d0dd40bd) (cherry picked from commit a0f34b3f9201802b8807ed788f00f6367df56778) --- src/core/devices/nm-device.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 264d75d936..539c0ec052 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -4018,7 +4018,6 @@ _set_ifindex(NMDevice *self, int ifindex, gboolean is_ip_ifindex) NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); gs_unref_object NML3Cfg *l3cfg_old = NULL; NML3CfgCommitTypeHandle *l3cfg_commit_type_old = NULL; - gboolean l3_changed; int ip_ifindex_new; int *p_ifindex; gboolean l3cfg_was_reset = FALSE; @@ -4059,6 +4058,10 @@ _set_ifindex(NMDevice *self, int ifindex, gboolean is_ip_ifindex) l3cfg_was_reset = TRUE; } } + + if (!priv->l3cfg && l3cfg_old) + _dev_l3_register_l3cds(self, l3cfg_old, FALSE, FALSE); + if (!priv->l3cfg && ip_ifindex_new > 0) { priv->l3cfg_ = nm_netns_l3cfg_acquire(priv->netns, ip_ifindex_new); @@ -4070,6 +4073,7 @@ _set_ifindex(NMDevice *self, int ifindex, gboolean is_ip_ifindex) _dev_l3_cfg_commit_type_reset(self); l3cfg_was_reset = TRUE; } + if (!priv->l3cfg) { _cleanup_ip_pre(self, AF_INET, CLEANUP_TYPE_KEEP, FALSE); _cleanup_ip_pre(self, AF_INET6, CLEANUP_TYPE_KEEP, FALSE); @@ -4110,11 +4114,7 @@ _set_ifindex(NMDevice *self, int ifindex, gboolean is_ip_ifindex) _notify(self, PROP_IP6_CONFIG); } - if (l3cfg_old != priv->l3cfg) { - l3_changed = FALSE; - if (_dev_l3_register_l3cds(self, l3cfg_old, FALSE, FALSE)) - l3_changed = TRUE; - + if (priv->l3cfg && l3cfg_old != priv->l3cfg) { /* Now it gets ugly. We changed the ip-ifindex, which determines the NML3Cfg instance. * But all the NML3ConfigData we currently track are still for the old ifindex. We * need to update them. @@ -4123,12 +4123,10 @@ _set_ifindex(NMDevice *self, int ifindex, gboolean is_ip_ifindex) * associated with one ifindex (and not the ifindex/ip-ifindex split). Or it * is not at all associated with an ifindex, but only a controlling device for * a real NMDevice (that has the ifindex). */ + _dev_l3_update_l3cds_ifindex(self); if (_dev_l3_register_l3cds(self, priv->l3cfg, TRUE, FALSE)) - l3_changed = TRUE; - - if (l3_changed) _dev_l3_cfg_commit(self, TRUE); } -- 2.36.1