device: gracefully handle device names exceeding IFNAMESIZ
authorHans Dedecker <dedeckeh@gmail.com>
Sun, 12 Aug 2018 20:08:22 +0000 (22:08 +0200)
committerHans Dedecker <dedeckeh@gmail.com>
Tue, 14 Aug 2018 15:06:38 +0000 (17:06 +0200)
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>
alias.c
bridge.c
device.c
device.h
macvlan.c
tunnel.c
veth.c
vlan.c
vlandev.c

diff --git a/alias.c b/alias.c
index 3b14b87..951e046 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;
index 96e0209..8876871 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;
 
index 09998f4..beac2c4 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);
index 84acf52..63f937a 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);
index 021d394..092f1dd 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;
index 3fa3e21..1383384 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 e109f27..eafa4cf 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 c3ab2d2..8748b30 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)
index 7e46251..ceaeb3e 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;