From: Hans Dedecker <dedeckeh@gmail.com>
Date: Sun, 12 Aug 2018 20:08:22 +0000 (+0200)
Subject: device: gracefully handle device names exceeding IFNAMESIZ
X-Git-Url: http://git.openwrt.org/?p=project%2Fnetifd.git;a=commitdiff_plain;h=522456b9f3ab07a78de17bf693abead4a296b028

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 3b14b87..951e046 100644
--- 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 96e0209..8876871 100644
--- 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 09998f4..beac2c4 100644
--- 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 84acf52..63f937a 100644
--- 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 021d394..092f1dd 100644
--- 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 3fa3e21..1383384 100644
--- 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
--- 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
--- 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 7e46251..ceaeb3e 100644
--- 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;