device: gracefully handle device names exceeding IFNAMESIZ

Instead of truncating the device name when it exceeds IFNAMSIZ length;
let device_set_ifname return an error code and do not add the device
to the device list.
This avoids possible issues with device names becoming identical due the
truncation and as a result unexpected behavior.
Further let the different device types gracefully handle the error code
returned by device_init

Signed-off-by: Hans Dedecker <dedeckeh@gmail.com>

diff --git a/alias.c b/alias.c
index 3b14b87b2154b0011fdc2410ee3e7a76947d27be..951e046bb3f135b582ca4a371f01f3a647a282ab 100644 (file)
--- a/alias.c
+++ b/alias.c
@@ -124,7 +124,11 @@ alias_device_create(const char *name, struct device_type *devtype,
        strcpy(alias->name, name);
        alias->dev.set_state = alias_device_set_state;
        alias->dev.hidden = true;
-       device_init_virtual(&alias->dev, devtype, NULL);
+       if (device_init_virtual(&alias->dev, devtype, NULL) < 0) {
+               free(alias);
+               return NULL;
+       }
+
        alias->avl.key = alias->name;
        avl_insert(&aliases, &alias->avl);
        alias->dep.alias = true;

diff --git a/bridge.c b/bridge.c
index 96e0209141aea09df3627abe24c18b2a951c2119..887687162c09cd4f1e492353746d4f768b33744a 100644 (file)
--- a/bridge.c
+++ b/bridge.c
@@ -720,7 +720,13 @@ bridge_create(const char *name, struct device_type *devtype,
                return NULL;
 
        dev = &bst->dev;
-       device_init(dev, devtype, name);
+
+       if (device_init(dev, devtype, name) < 0) {
+               device_cleanup(dev);
+               free(bst);
+               return NULL;
+       }
+
        dev->config_pending = true;
        bst->retry.cb = bridge_retry_members;
 

diff --git a/device.c b/device.c
index 09998f4d529bb8fb17c4ad32432d69a4483a4db0..beac2c4b0aa0d0c8122b83b16663072526c759cf 100644 (file)
--- a/device.c
+++ b/device.c
@@ -470,7 +470,7 @@ int device_check_state(struct device *dev)
        return dev->type->check_state(dev);
 }
 
-void device_init_virtual(struct device *dev, struct device_type *type, const char *name)
+int device_init_virtual(struct device *dev, struct device_type *type, const char *name)
 {
        assert(dev);
        assert(type);
@@ -480,18 +480,27 @@ void device_init_virtual(struct device *dev, struct device_type *type, const cha
        INIT_SAFE_LIST(&dev->aliases);
        dev->type = type;
 
-       if (name)
-               device_set_ifname(dev, name);
+       if (name) {
+               int ret;
+
+               ret = device_set_ifname(dev, name);
+               if (ret < 0)
+                       return ret;
+       }
 
        if (!dev->set_state)
                dev->set_state = set_device_state;
+
+       return 0;
 }
 
 int device_init(struct device *dev, struct device_type *type, const char *ifname)
 {
        int ret;
 
-       device_init_virtual(dev, type, ifname);
+       ret = device_init_virtual(dev, type, ifname);
+       if (ret < 0)
+               return ret;
 
        dev->avl.key = dev->ifname;
 
@@ -520,7 +529,13 @@ device_create_default(const char *name, bool external)
 
        dev->external = external;
        dev->set_state = simple_device_set_state;
-       device_init(dev, &simple_device_type, name);
+
+       if (device_init(dev, &simple_device_type, name) < 0) {
+               device_cleanup(dev);
+               free(dev);
+               return NULL;
+       }
+
        dev->default_config = true;
        if (external)
                system_if_apply_settings(dev, &dev->settings, dev->settings.flags);
@@ -648,10 +663,13 @@ int device_set_ifname(struct device *dev, const char *name)
        if (!strcmp(dev->ifname, name))
                return 0;
 
+       if (strlen(name) > sizeof(dev->ifname) - 1)
+               return -1;
+
        if (dev->avl.key)
                avl_delete(&devices, &dev->avl);
 
-       strncpy(dev->ifname, name, IFNAMSIZ);
+       strcpy(dev->ifname, name);
 
        if (dev->avl.key)
                ret = avl_insert(&devices, &dev->avl);

diff --git a/device.h b/device.h
index 84acf52ba00bde10dcec01448008ef7d4c1c1369..63f937a7ae5d6753bb5ac390a37d5c48ec4474d8 100644 (file)
--- a/device.h
+++ b/device.h
@@ -250,7 +250,7 @@ device_apply_config(struct device *dev, struct device_type *type,
 void device_reset_config(void);
 void device_reset_old(void);
 
-void device_init_virtual(struct device *dev, struct device_type *type, const char *name);
+int device_init_virtual(struct device *dev, struct device_type *type, const char *name);
 int device_init(struct device *dev, struct device_type *type, const char *ifname);
 void device_cleanup(struct device *dev);
 struct device *device_find(const char *name);

diff --git a/macvlan.c b/macvlan.c
index 021d3944fb1ccbee81d9f141539a39d1a47ab258..092f1dd36c1d574a2e5fa110ae2b7a8d33de4838 100644 (file)
--- a/macvlan.c
+++ b/macvlan.c
@@ -239,7 +239,12 @@ macvlan_create(const char *name, struct device_type *devtype,
                return NULL;
 
        dev = &mvdev->dev;
-       device_init(dev, devtype, name);
+       if (device_init(dev, devtype, name) < 0) {
+               device_cleanup(dev);
+               free(mvdev);
+               return NULL;
+       }
+
        dev->config_pending = true;
 
        mvdev->set_state = dev->set_state;

diff --git a/tunnel.c b/tunnel.c
index 3fa3e2152ad1e63ed1e298684467cccc64ac6a69..1383384075cf57e8e95b3edf31602e57eb159639 100644 (file)
--- a/tunnel.c
+++ b/tunnel.c
@@ -72,7 +72,13 @@ tunnel_create(const char *name, struct device_type *devtype,
                return NULL;
 
        dev = &tun->dev;
-       device_init(dev, devtype, name);
+
+       if (device_init(dev, devtype, name) < 0) {
+               device_cleanup(dev);
+               free(tun);
+               return NULL;
+       }
+
        tun->set_state = dev->set_state;
        dev->set_state = tunnel_set_state;
        device_apply_config(dev, devtype, attr);

diff --git a/veth.c b/veth.c
index e109f273aedefa8b3872b8f9fb7bab7b640cc8a1..eafa4cf97bbbf5523e574a72e53ddc37f7c35d11 100644 (file)
--- a/veth.c
+++ b/veth.c
@@ -218,7 +218,12 @@ veth_create(const char *name, struct device_type *devtype,
                return NULL;
 
        dev = &veth->dev;
-       device_init(dev, devtype, name);
+       if (device_init(dev, devtype, name) < 0) {
+               device_cleanup(dev);
+               free(veth);
+               return NULL;
+       }
+
        dev->config_pending = true;
 
        veth->set_state = dev->set_state;

diff --git a/vlan.c b/vlan.c
index c3ab2d23f34c5d8f088aecad181ff560fc5fd3da..8748b3040d511f554ec335071f39b4b0a120df63 100644 (file)
--- a/vlan.c
+++ b/vlan.c
@@ -61,7 +61,7 @@ static int vlan_set_device_state(struct device *dev, bool up)
        return ret;
 }
 
-static void vlan_dev_set_name(struct vlan_device *vldev, struct device *dev)
+static int vlan_dev_set_name(struct vlan_device *vldev, struct device *dev)
 {
        char *name;
 
@@ -69,7 +69,7 @@ static void vlan_dev_set_name(struct vlan_device *vldev, struct device *dev)
        vldev->dev.hidden = dev->hidden;
        sprintf(name, "%s.%d", dev->ifname, vldev->id);
 
-       device_set_ifname(&vldev->dev, name);
+       return device_set_ifname(&vldev->dev, name);
 }
 
 static void vlan_dev_cb(struct device_user *dep, enum device_event ev)
@@ -85,7 +85,8 @@ static void vlan_dev_cb(struct device_user *dep, enum device_event ev)
                device_set_present(&vldev->dev, false);
                break;
        case DEV_EVENT_UPDATE_IFNAME:
-               vlan_dev_set_name(vldev, dep->dev);
+               if (vlan_dev_set_name(vldev, dep->dev) < 0)
+                       free_vlan_if(&vldev->dev);
                break;
        case DEV_EVENT_TOPO_CHANGE:
                /* Propagate topo changes */
@@ -106,9 +107,6 @@ static struct device *get_vlan_device(struct device *dev, int id, bool create)
        struct vlan_device *vldev;
        struct device_user *dep;
 
-       if (strlen(dev->ifname) > (IFNAMSIZ - 6))
-               return NULL;
-
        /* look for an existing interface before creating a new one */
        list_for_each_entry(dep, &dev->users.list, list.list) {
                if (dep->cb != vlan_dev_cb)
@@ -132,9 +130,12 @@ static struct device *get_vlan_device(struct device *dev, int id, bool create)
 
        vldev->id = id;
 
-       device_init(&vldev->dev, &vlan_type, NULL);
+       if (device_init(&vldev->dev, &vlan_type, NULL) < 0)
+               goto error;
+
+       if (vlan_dev_set_name(vldev, dev) < 0)
+               goto error;
 
-       vlan_dev_set_name(vldev, dev);
        vldev->dev.default_config = true;
 
        vldev->set_state = vldev->dev.set_state;
@@ -144,6 +145,11 @@ static struct device *get_vlan_device(struct device *dev, int id, bool create)
        device_add_user(&vldev->dep, dev);
 
        return &vldev->dev;
+
+error:
+       device_cleanup(&vldev->dev);
+       free(vldev);
+       return NULL;
 }
 
 static char *split_vlan(char *s)

diff --git a/vlandev.c b/vlandev.c
index 7e4625125297e2572bb4586606ef4a6ce46dfcc3..ceaeb3ef176b188b3aa9f14c7b9678a506040037 100644 (file)
--- a/vlandev.c
+++ b/vlandev.c
@@ -222,7 +222,13 @@ vlandev_create(const char *name, struct device_type *devtype,
                return NULL;
 
        dev = &mvdev->dev;
-       device_init(dev, devtype, name);
+
+       if (device_init(dev, devtype, name) < 0) {
+               device_cleanup(dev);
+               free(mvdev);
+               return NULL;
+       }
+
        dev->config_pending = true;
 
        mvdev->set_state = dev->set_state;