From 08f5f630be0fa5f8ae7585352e804f75ca3c13a4 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 (cherry picked from commit 3cb4b36261684aa3d2676f922c6d53bc31085153) (cherry picked from commit cb20d0791a6daf20d64f4cd57d6bd4b60e35a9a0) (cherry picked from commit 511ef27d5eaf6fd0577b867d9d31de3bee0440fe) (cherry picked from commit 33309f7c9f125171d3239103316d88dbd7e606c0) (cherry picked from commit 1240565e482d02df19c34b845a1cd93433f01986) --- 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 4af51ea37f..6a7dac083b 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3848,13 +3848,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 426ac8329dc6015e398348cde1cecb2438649b7b 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 (cherry picked from commit 57e3734b6cc1bb453216c7e2150a698114507a46) (cherry picked from commit ec1b5fb019929441fdcdf6bf7c54a2856ad61976) (cherry picked from commit f6a90b899ab01d6dc2635faf6bc8d72839a9064c) (cherry picked from commit af2a126dc037f01d2172b4ca89907d463d3098c9) (cherry picked from commit f56853890466e0ffb64e086f950f3cc861f56790) --- src/devices/nm-device-bond.c | 6 ++++++ src/devices/nm-device-bridge.c | 6 ++++++ src/devices/nm-device.c | 6 +----- src/devices/team/nm-device-team.c | 6 ++++++ 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index f59ec9ff22..ca5f431013 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -436,6 +436,12 @@ release_slave (NMDevice *device, NMDeviceBond *self = NM_DEVICE_BOND (device); gboolean success, no_firmware = FALSE; gs_free char *address = NULL; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; if (configure) { /* When the last slave is released the bond MAC will be set to a random diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index c81a025338..c7edad60ba 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -403,6 +403,12 @@ release_slave (NMDevice *device, { NMDeviceBridge *self = NM_DEVICE_BRIDGE (device); gboolean success; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; if (configure) { success = nm_platform_link_release (nm_device_get_platform (device), diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 6a7dac083b..18883d49b2 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4580,7 +4580,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. */ @@ -4591,13 +4590,10 @@ nm_device_master_release_slaves (NMDevice *self) if (priv->state == NM_DEVICE_STATE_FAILED) reason = NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED; - if (!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 f83ba22b5d..027abb13c7 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -763,6 +763,12 @@ release_slave (NMDevice *device, NMDeviceTeam *self = NM_DEVICE_TEAM (device); NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); gboolean success, no_firmware = FALSE; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; if (configure) { success = nm_platform_link_release (nm_device_get_platform (device),