openSUSE / wicked

Framework for network configuration
https://en.opensuse.org/Portal:Wicked
GNU General Public License v2.0
98 stars 49 forks source link

client: fix crash in hierarchy traversing (bsc#1226664) #1023

Closed mtomaschewski closed 1 week ago

mtomaschewski commented 1 week ago

Added a loop guard to system and config interface worker hierarchy traversing logged as debug or in a --dry-run and fixed resolving of link reference names pointing to another netnsid and causing such a loop and wicked ifup crash in systemd-nspawn containers using macvlans.

cfconrad commented 1 week ago

I think we need another init here:

diff --git a/autoip4/device.c b/autoip4/device.c
index 6aa3d0a5..b8d44b98 100644
--- a/autoip4/device.c
+++ b/autoip4/device.c
@@ -39,6 +39,8 @@ ni_autoip_device_new(const char *ifname, const ni_linkinfo_t *link)
        dev = calloc(1, sizeof(*dev));
        ni_string_dup(&dev->ifname, ifname);
        dev->link.ifindex = link->ifindex;
+       ni_netdev_ref_init(&dev->link.lowerdev);
+       ni_netdev_ref_init(&dev->link.masterdev);
        dev->users = 1;
        dev->fsm.state = NI_AUTOIP_STATE_INIT;

@@ -139,6 +141,8 @@ ni_autoip_device_free(ni_autoip_device_t *dev)
        ni_string_free(&dev->devinfo.ifname);
        ni_string_free(&dev->ifname);
        dev->link.ifindex = 0;
+       ni_netdev_ref_destroy(&dev->link.lowerdev);
+       ni_netdev_ref_destroy(&dev->link.masterdev);

        for (pos = &ni_autoip_active; *pos; pos = &(*pos)->next) {
                if (*pos == dev) {
mtomaschewski commented 1 week ago

I think we need another init here:

A very good catch, thanks! Yes, it's a well hidden one, going to add the init + destroy.

Instead of resolving ifindex to netdev + refresh netdev->link, it (still) maintains a copy of the linkinfo :-1: -> a separate cleanup.

mtomaschewski commented 1 week ago

I've started to split the linkinfo init + destroy into separate functions in netinfo: add linkinfo init and destroy functions, but at the end it's not really needed: it's much better to get rid of the linkinfo copy / duplicate at all... and just lookup the netdev , refresh and use/pass directly in auto4 (as we do in dhcp4).

mtomaschewski commented 1 week ago

I've merged it to testing to appear at http://download.opensuse.org/repositories/network:/wicked:/testing/