From 3cb4b36261684aa3d2676f922c6d53bc31085153 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 30 Jul 2019 11:03:59 +0200 Subject: [PATCH 1/2] device: check platform link compatibility when setting nm-owned flag We set nm-owned to indicate whether a software device was created by NM or it was pre-existing. When checking the existence, we must verify also whether the link type is compatible with the device, otherwise it is possible to match unrelated interfaces. For example, when checking for the existence of an ovs-bridge (which is not compatible with any platform link) we could match a unrelated platform link with the same name. https://bugzilla.redhat.com/show_bug.cgi?id=1733709 --- src/devices/nm-device.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index feb5110d30..f6a3c3f8fa 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4153,13 +4153,14 @@ nm_device_create_and_realize (NMDevice *self, { nm_auto_nmpobj const NMPObject *plink_keep_alive = NULL; NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - const NMPlatformLink *plink = NULL; + const NMPlatformLink *plink; /* Must be set before device is realized */ - priv->nm_owned = !nm_platform_link_get_by_ifname (nm_device_get_platform (self), priv->iface); - + plink = nm_platform_link_get_by_ifname (nm_device_get_platform (self), priv->iface); + priv->nm_owned = !plink || !link_type_compatible (self, plink->type, NULL, NULL); _LOGD (LOGD_DEVICE, "create (is %snm-owned)", priv->nm_owned ? "" : "not "); + plink = NULL; /* Create any resources the device needs */ if (NM_DEVICE_GET_CLASS (self)->create_and_realize) { if (!NM_DEVICE_GET_CLASS (self)->create_and_realize (self, connection, parent, &plink, error)) From 57e3734b6cc1bb453216c7e2150a698114507a46 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 31 Jul 2019 11:40:35 +0200 Subject: [PATCH 2/2] device: fix releasing slaves Not all masters type have a platform link and so it's wrong to check for it to decide whether the slave should be really released. Move the check to master devices that need it (bond, bridge and team). OVS ports don't need the check because they don't call to platform to remove a slave. https://bugzilla.redhat.com/show_bug.cgi?id=1733709 --- src/devices/nm-device-bond.c | 6 ++++++ src/devices/nm-device-bridge.c | 6 ++++++ src/devices/nm-device.c | 7 +------ src/devices/team/nm-device-team.c | 6 ++++++ 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index fd79348d2c..50494526a9 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -414,6 +414,12 @@ release_slave (NMDevice *device, gboolean success; gs_free char *address = NULL; int ifindex_slave; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; ifindex_slave = nm_device_get_ip_ifindex (slave); diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index ade9eb0d8f..91d824b35b 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -626,6 +626,12 @@ release_slave (NMDevice *device, NMDeviceBridge *self = NM_DEVICE_BRIDGE (device); gboolean success; int ifindex_slave; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; ifindex_slave = nm_device_get_ip_ifindex (slave); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index f6a3c3f8fa..cf80774298 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4987,7 +4987,6 @@ nm_device_master_release_slaves (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMDeviceStateReason reason; - gboolean configure = TRUE; CList *iter, *safe; /* Don't release the slaves if this connection doesn't belong to NM. */ @@ -4998,14 +4997,10 @@ nm_device_master_release_slaves (NMDevice *self) if (priv->state == NM_DEVICE_STATE_FAILED) reason = NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED; - if ( priv->ifindex <= 0 - || !nm_platform_link_get (nm_device_get_platform (self), priv->ifindex)) - configure = FALSE; - c_list_for_each_safe (iter, safe, &priv->slaves) { SlaveInfo *info = c_list_entry (iter, SlaveInfo, lst_slave); - nm_device_master_release_one_slave (self, info->slave, configure, reason); + nm_device_master_release_one_slave (self, info->slave, TRUE, reason); } } diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index a60a9fda5d..4661a84034 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -775,6 +775,12 @@ release_slave (NMDevice *device, NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); gboolean success; int ifindex_slave; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; ifindex_slave = nm_device_get_ip_ifindex (slave);