From: Christian Brauner <christian.brauner@ubuntu.com>
Date: Thu, 31 Aug 2017 22:23:30 +0000 (+0200)
Subject: network: use static memory for net device names
X-Git-Tag: lxc-2.1.0~9^2~17
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=de4855a8bc9cecfff180567889a784d938e4949b;p=thirdparty%2Flxc.git

network: use static memory for net device names

All network devices can only be of size < IFNAMSIZ. So let's spare the useless
heap allocations and use static memory.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index e66bae314..625616c69 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -483,7 +483,7 @@ static int set_config_net_link(const char *key, const char *value,
 	if (value[strlen(value) - 1] == '+' && netdev->type == LXC_NET_PHYS)
 		ret = create_matched_ifnames(value, lxc_conf, netdev);
 	else
-		ret = network_ifname(&netdev->link, value);
+		ret = network_ifname(netdev->link, value);
 
 	return ret;
 }
@@ -503,7 +503,7 @@ static int set_config_net_name(const char *key, const char *value,
 	if (!netdev)
 		return -1;
 
-	return network_ifname(&netdev->name, value);
+	return network_ifname(netdev->name, value);
 }
 
 static int set_config_net_veth_pair(const char *key, const char *value,
@@ -521,7 +521,7 @@ static int set_config_net_veth_pair(const char *key, const char *value,
 	if (!netdev)
 		return -1;
 
-	return network_ifname(&netdev->priv.veth_attr.pair, value);
+	return network_ifname(netdev->priv.veth_attr.pair, value);
 }
 
 static int set_config_net_macvlan_mode(const char *key, const char *value,
@@ -3690,8 +3690,7 @@ static int clr_config_net_name(const char *key, struct lxc_conf *lxc_conf,
 	if (!netdev)
 		return -1;
 
-	free(netdev->name);
-	netdev->name = NULL;
+	netdev->name[0] = '\0';
 
 	return 0;
 }
@@ -3725,8 +3724,7 @@ static int clr_config_net_link(const char *key, struct lxc_conf *lxc_conf,
 	if (!netdev)
 		return -1;
 
-	free(netdev->link);
-	netdev->link = NULL;
+	netdev->link[0] = '\0';
 
 	return 0;
 }
@@ -3763,8 +3761,7 @@ static int clr_config_net_veth_pair(const char *key, struct lxc_conf *lxc_conf,
 	if (!netdev)
 		return -1;
 
-	free(netdev->priv.veth_attr.pair);
-	netdev->priv.veth_attr.pair = NULL;
+	netdev->priv.veth_attr.pair[0] = '\0';
 
 	return 0;
 }
@@ -4032,7 +4029,7 @@ static int get_config_net_link(const char *key, char *retv, int inlen,
 	if (!netdev)
 		return -1;
 
-	if (netdev->link)
+	if (netdev->link[0] != '\0')
 		strprint(retv, inlen, "%s", netdev->link);
 
 	return fulllen;
@@ -4056,7 +4053,7 @@ static int get_config_net_name(const char *key, char *retv, int inlen,
 	if (!netdev)
 		return -1;
 
-	if (netdev->name)
+	if (netdev->name[0] != '\0')
 		strprint(retv, inlen, "%s", netdev->name);
 
 	return fulllen;
@@ -4129,8 +4126,9 @@ static int get_config_net_veth_pair(const char *key, char *retv, int inlen,
 		return 0;
 
 	strprint(retv, inlen, "%s",
-		 netdev->priv.veth_attr.pair ? netdev->priv.veth_attr.pair
-					     : netdev->priv.veth_attr.veth1);
+		 netdev->priv.veth_attr.pair[0] != '\0'
+		     ? netdev->priv.veth_attr.pair
+		     : netdev->priv.veth_attr.veth1);
 
 	return fulllen;
 }
diff --git a/src/lxc/confile_legacy.c b/src/lxc/confile_legacy.c
index f3bd8fbaf..80dd3851e 100644
--- a/src/lxc/confile_legacy.c
+++ b/src/lxc/confile_legacy.c
@@ -110,10 +110,6 @@ static void lxc_remove_nic(struct lxc_list *it)
 
 	lxc_list_del(it);
 
-	free(netdev->link);
-	free(netdev->name);
-	if (netdev->type == LXC_NET_VETH)
-		free(netdev->priv.veth_attr.pair);
 	free(netdev->upscript);
 	free(netdev->downscript);
 	free(netdev->hwaddr);
@@ -174,6 +170,16 @@ int set_config_network_legacy_type(const char *key, const char *value,
 	lxc_list_init(&netdev->ipv4);
 	lxc_list_init(&netdev->ipv6);
 
+	netdev->name[0] = '\0';
+	netdev->link[0] = '\0';
+	memset(&netdev->priv, 0, sizeof(netdev->priv));
+	/* I'm not completely sure if the memset takes care to zero the arrays
+	 * in the union as well. So let's make extra sure and set the first byte
+	 * to zero so that we don't have any surprises.
+	 */
+	netdev->priv.veth_attr.pair[0] = '\0';
+	netdev->priv.veth_attr.veth1[0] = '\0';
+
 	list = malloc(sizeof(*list));
 	if (!list) {
 		SYSERROR("failed to allocate memory");
@@ -423,7 +429,7 @@ int set_config_network_legacy_link(const char *key, const char *value,
 		free(it);
 		ret = create_matched_ifnames(value, lxc_conf, NULL);
 	} else {
-		ret = network_ifname(&netdev->link, value);
+		ret = network_ifname(netdev->link, value);
 	}
 
 	return ret;
@@ -438,7 +444,7 @@ int set_config_network_legacy_name(const char *key, const char *value,
 	if (!netdev)
 		return -1;
 
-	return network_ifname(&netdev->name, value);
+	return network_ifname(netdev->name, value);
 }
 
 int set_config_network_legacy_veth_pair(const char *key, const char *value,
@@ -455,7 +461,7 @@ int set_config_network_legacy_veth_pair(const char *key, const char *value,
 		return -1;
 	}
 
-	return network_ifname(&netdev->priv.veth_attr.pair, value);
+	return network_ifname(netdev->priv.veth_attr.pair, value);
 }
 
 int set_config_network_legacy_macvlan_mode(const char *key, const char *value,
@@ -848,12 +854,12 @@ int get_config_network_legacy_item(const char *key, char *retv, int inlen,
 	if (!netdev)
 		return -1;
 	if (strcmp(p1, "name") == 0) {
-		if (netdev->name)
+		if (netdev->name[0] != '\0')
 			strprint(retv, inlen, "%s", netdev->name);
 	} else if (strcmp(p1, "type") == 0) {
 		strprint(retv, inlen, "%s", lxc_net_type_to_str(netdev->type));
 	} else if (strcmp(p1, "link") == 0) {
-		if (netdev->link)
+		if (netdev->link[0] != '\0')
 			strprint(retv, inlen, "%s", netdev->link);
 	} else if (strcmp(p1, "flags") == 0) {
 		if (netdev->flags & IFF_UP)
@@ -895,7 +901,7 @@ int get_config_network_legacy_item(const char *key, char *retv, int inlen,
 	} else if (strcmp(p1, "veth.pair") == 0) {
 		if (netdev->type == LXC_NET_VETH) {
 			strprint(retv, inlen, "%s",
-				 netdev->priv.veth_attr.pair
+				 netdev->priv.veth_attr.pair[0] != '\0'
 				     ? netdev->priv.veth_attr.pair
 				     : netdev->priv.veth_attr.veth1);
 		}
diff --git a/src/lxc/confile_utils.c b/src/lxc/confile_utils.c
index ba4b9c0dd..b3ae08b68 100644
--- a/src/lxc/confile_utils.c
+++ b/src/lxc/confile_utils.c
@@ -183,6 +183,15 @@ struct lxc_netdev *lxc_network_add(struct lxc_list *networks, int idx, bool tail
 	memset(netdev, 0, sizeof(*netdev));
 	lxc_list_init(&netdev->ipv4);
 	lxc_list_init(&netdev->ipv6);
+	netdev->name[0] = '\0';
+	netdev->link[0] = '\0';
+	memset(&netdev->priv, 0, sizeof(netdev->priv));
+	/* I'm not completely sure if the memset takes care to zero the arrays
+	 * in the union as well. So let's make extra sure and set the first byte
+	 * to zero so that we don't have any surprises.
+	 */
+	netdev->priv.veth_attr.pair[0] = '\0';
+	netdev->priv.veth_attr.veth1[0] = '\0';
 
 	/* give network a unique index */
 	netdev->idx = idx;
@@ -258,7 +267,7 @@ void lxc_log_configured_netdevs(const struct lxc_conf *conf)
 		switch (netdev->type) {
 		case LXC_NET_VETH:
 			TRACE("type: veth");
-			if (netdev->priv.veth_attr.pair)
+			if (netdev->priv.veth_attr.pair[0] != '\0')
 				TRACE("veth pair: %s",
 				      netdev->priv.veth_attr.pair);
 			if (netdev->priv.veth_attr.veth1[0] != '\0')
@@ -300,9 +309,9 @@ void lxc_log_configured_netdevs(const struct lxc_conf *conf)
 		if (netdev->type != LXC_NET_EMPTY) {
 			TRACE("flags: %s",
 			      netdev->flags == IFF_UP ? "up" : "none");
-			if (netdev->link)
+			if (netdev->link[0] != '\0')
 				TRACE("link: %s", netdev->link);
-			if (netdev->name)
+			if (netdev->name[0] != '\0')
 				TRACE("name: %s", netdev->name);
 			if (netdev->hwaddr)
 				TRACE("hwaddr: %s", netdev->hwaddr);
@@ -350,10 +359,6 @@ static void lxc_free_netdev(struct lxc_netdev *netdev)
 {
 	struct lxc_list *cur, *next;
 
-	free(netdev->link);
-	free(netdev->name);
-	if (netdev->type == LXC_NET_VETH)
-		free(netdev->priv.veth_attr.pair);
 	free(netdev->upscript);
 	free(netdev->downscript);
 	free(netdev->hwaddr);
@@ -503,9 +508,15 @@ int config_ip_prefix(struct in_addr *addr)
 	return 0;
 }
 
-int network_ifname(char **valuep, const char *value)
+int network_ifname(char *valuep, const char *value)
 {
-	return set_config_string_item_max(valuep, value, IFNAMSIZ);
+	if (strlen(value) >= IFNAMSIZ) {
+		ERROR("Network devie name \"%s\" is too long (>= %zu)", value,
+		      (size_t)IFNAMSIZ);
+	}
+
+	strcpy(valuep, value);
+	return 0;
 }
 
 int rand_complete_hwaddr(char *hwaddr)
diff --git a/src/lxc/confile_utils.h b/src/lxc/confile_utils.h
index 222de3de9..2f1079a2c 100644
--- a/src/lxc/confile_utils.h
+++ b/src/lxc/confile_utils.h
@@ -77,7 +77,7 @@ extern int set_config_string_item_max(char **conf_item, const char *value,
 				      size_t max);
 extern int set_config_path_item(char **conf_item, const char *value);
 extern int config_ip_prefix(struct in_addr *addr);
-extern int network_ifname(char **valuep, const char *value);
+extern int network_ifname(char *valuep, const char *value);
 extern int rand_complete_hwaddr(char *hwaddr);
 extern bool lxc_config_net_hwaddr(const char *line);
 extern void update_hwaddr(const char *line);
diff --git a/src/lxc/criu.c b/src/lxc/criu.c
index 62a0eb7d6..2f444ec44 100644
--- a/src/lxc/criu.c
+++ b/src/lxc/criu.c
@@ -524,7 +524,7 @@ static void exec_criu(struct criu_opts *opts)
 			case LXC_NET_VETH:
 				veth = n->priv.veth_attr.pair;
 
-				if (n->link) {
+				if (n->link[0] != '\0') {
 					if (external_not_veth)
 						fmt = "veth[%s]:%s@%s";
 					else
@@ -543,7 +543,7 @@ static void exec_criu(struct criu_opts *opts)
 					goto err;
 				break;
 			case LXC_NET_MACVLAN:
-				if (!n->link) {
+				if (n->link[0] == '\0') {
 					ERROR("no host interface for macvlan %s", n->name);
 					goto err;
 				}
@@ -765,11 +765,17 @@ static bool restore_net_info(struct lxc_container *c)
 
 		snprintf(template, sizeof(template), "vethXXXXXX");
 
-		if (!netdev->priv.veth_attr.pair)
-			netdev->priv.veth_attr.pair = lxc_mkifname(template);
+		if (netdev->priv.veth_attr.pair[0] == '\0' &&
+		    netdev->priv.veth_attr.veth1[0] == '\0') {
+			char *tmp;
 
-		if (!netdev->priv.veth_attr.pair)
-			goto out_unlock;
+			tmp = lxc_mkifname(template);
+			if (!tmp)
+				goto out_unlock;
+
+			strcpy(netdev->priv.veth_attr.veth1, tmp);
+			free(tmp);
+		}
 	}
 
 	has_error = false;
diff --git a/src/lxc/network.c b/src/lxc/network.c
index f10dd3f3e..07a4360ac 100644
--- a/src/lxc/network.c
+++ b/src/lxc/network.c
@@ -101,7 +101,7 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 	char veth1buf[IFNAMSIZ], veth2buf[IFNAMSIZ];
 	unsigned int mtu = 0;
 
-	if (netdev->priv.veth_attr.pair) {
+	if (netdev->priv.veth_attr.pair[0] != '\0') {
 		veth1 = netdev->priv.veth_attr.pair;
 		if (handler->conf->reboot)
 			lxc_netdev_delete_by_name(veth1);
@@ -163,7 +163,7 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 			WARN("Failed to parse mtu");
 		else
 			INFO("Retrieved mtu %d", mtu);
-	} else if (netdev->link) {
+	} else if (netdev->link[0] != '\0') {
 		bridge_index = if_nametoindex(netdev->link);
 		if (bridge_index) {
 			mtu = netdev_get_mtu(bridge_index);
@@ -186,7 +186,7 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 		}
 	}
 
-	if (netdev->link) {
+	if (netdev->link[0] != '\0') {
 		err = lxc_bridge_attach(netdev->link, veth1);
 		if (err) {
 			ERROR("Failed to attach \"%s\" to bridge \"%s\": %s",
@@ -217,7 +217,7 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 out_delete:
 	if (netdev->ifindex != 0)
 		lxc_netdev_delete_by_name(veth1);
-	if (!netdev->priv.veth_attr.pair)
+	if (netdev->priv.veth_attr.pair != veth1)
 		free(veth1);
 	free(veth2);
 	return -1;
@@ -228,7 +228,7 @@ static int instantiate_macvlan(struct lxc_handler *handler, struct lxc_netdev *n
 	char peerbuf[IFNAMSIZ], *peer;
 	int err;
 
-	if (!netdev->link) {
+	if (netdev->link[0] == '\0') {
 		ERROR("No link for macvlan network device specified");
 		return -1;
 	}
@@ -279,7 +279,7 @@ static int instantiate_vlan(struct lxc_handler *handler, struct lxc_netdev *netd
 	static uint16_t vlan_cntr = 0;
 	unsigned int mtu = 0;
 
-	if (!netdev->link) {
+	if (netdev->link[0] == '\0') {
 		ERROR("No link for vlan network device specified");
 		return -1;
 	}
@@ -308,7 +308,7 @@ static int instantiate_vlan(struct lxc_handler *handler, struct lxc_netdev *netd
 		if (lxc_safe_uint(netdev->mtu, &mtu) < 0) {
 			ERROR("Failed to retrieve mtu from \"%d\"/\"%s\".",
 			      netdev->ifindex,
-			      netdev->name ? netdev->name : "(null)");
+			      netdev->name[0] != '\0' ? netdev->name : "(null)");
 			return -1;
 		}
 		err = lxc_netdev_set_mtu(peer, mtu);
@@ -325,7 +325,7 @@ static int instantiate_vlan(struct lxc_handler *handler, struct lxc_netdev *netd
 
 static int instantiate_phys(struct lxc_handler *handler, struct lxc_netdev *netdev)
 {
-	if (!netdev->link) {
+	if (netdev->link[0] == '\0') {
 		ERROR("No link for physical interface specified");
 		return -1;
 	}
@@ -380,7 +380,7 @@ static int shutdown_veth(struct lxc_handler *handler, struct lxc_netdev *netdev)
 	char *veth1;
 	int err;
 
-	if (netdev->priv.veth_attr.pair)
+	if (netdev->priv.veth_attr.pair[0] != '\0')
 		veth1 = netdev->priv.veth_attr.pair;
 	else
 		veth1 = netdev->priv.veth_attr.veth1;
@@ -2014,7 +2014,7 @@ int lxc_find_gateway_addresses(struct lxc_handler *handler)
 			return -1;
 		}
 
-		if (!netdev->link) {
+		if (netdev->link[0] == '\0') {
 			ERROR("Automatic gateway detection needs a link interface");
 			return -1;
 		}
@@ -2088,7 +2088,7 @@ static int lxc_create_network_unpriv_exec(const char *lxcpath, char *lxcname,
 			exit(EXIT_FAILURE);
 		}
 
-		if (netdev->link)
+		if (netdev->link[0] != '\0')
 			strncpy(netdev_link, netdev->link, IFNAMSIZ);
 		else
 			strncpy(netdev_link, "none", IFNAMSIZ);
@@ -2100,8 +2100,8 @@ static int lxc_create_network_unpriv_exec(const char *lxcpath, char *lxcname,
 
 		INFO("Execing lxc-user-nic create %s %s %s veth %s %s", lxcpath,
 		     lxcname, pidstr, netdev_link,
-		     netdev->name ? netdev->name : "(null)");
-		if (netdev->name)
+		     netdev->name[0] != '\0' ? netdev->name : "(null)");
+		if (netdev->name[0] != '\0')
 			execlp(LXC_USERNIC_PATH, LXC_USERNIC_PATH, "create",
 			       lxcpath, lxcname, pidstr, "veth", netdev_link,
 			       netdev->name, (char *)NULL);
@@ -2140,11 +2140,6 @@ static int lxc_create_network_unpriv_exec(const char *lxcpath, char *lxcname,
 		return -1;
 	}
 
-	netdev->name = malloc(IFNAMSIZ + 1);
-	if (!netdev->name) {
-		SYSERROR("Failed to allocate memory");
-		return -1;
-	}
 	memset(netdev->name, 0, IFNAMSIZ + 1);
 	strncpy(netdev->name, token, IFNAMSIZ);
 
@@ -2240,7 +2235,7 @@ static int lxc_delete_network_unpriv_exec(const char *lxcpath, char *lxcname,
 			exit(EXIT_FAILURE);
 		}
 
-		if (!netdev->link) {
+		if (netdev->link[0] == '\0') {
 			SYSERROR("Network link for network device \"%s\" is "
 				 "missing", netdev->priv.veth_attr.veth1);
 			exit(EXIT_FAILURE);
@@ -2423,7 +2418,7 @@ int lxc_network_move_created_netdev_priv(const char *lxcpath, char *lxcname,
 		}
 
 		DEBUG("Moved network device \"%s\"/\"%s\" to network namespace "
-		      "of %d:", ifname, netdev->name ? netdev->name : "(null)",
+		      "of %d:", ifname, netdev->name[0] != '\0' ? netdev->name : "(null)",
 		      pid);
 	}
 
@@ -2510,18 +2505,18 @@ bool lxc_delete_network_priv(struct lxc_handler *handler)
 			INFO("Interface \"%s\" with index %d already "
 					"deleted or existing in different network "
 					"namespace",
-					netdev->name ? netdev->name : "(null)",
+					netdev->name[0] != '\0' ? netdev->name : "(null)",
 					netdev->ifindex);
 		} else if (ret < 0) {
 			deleted_all = false;
 			WARN("Failed to remove interface \"%s\" with "
 					"index %d: %s",
-					netdev->name ? netdev->name : "(null)",
+					netdev->name[0] != '\0' ? netdev->name : "(null)",
 					netdev->ifindex, strerror(-ret));
 			continue;
 		}
 		INFO("Removed interface \"%s\" with index %d",
-				netdev->name ? netdev->name : "(null)",
+				netdev->name[0] != '\0' ? netdev->name : "(null)",
 				netdev->ifindex);
 
 		if (netdev->type != LXC_NET_VETH)
@@ -2530,11 +2525,11 @@ bool lxc_delete_network_priv(struct lxc_handler *handler)
 		/* Explicitly delete host veth device to prevent lingering
 		 * devices. We had issues in LXD around this.
 		 */
-		if (netdev->priv.veth_attr.pair)
+		if (netdev->priv.veth_attr.pair[0] != '\0')
 			hostveth = netdev->priv.veth_attr.pair;
 		else
 			hostveth = netdev->priv.veth_attr.veth1;
-		if (*hostveth == '\0')
+		if (hostveth[0] == '\0')
 			continue;
 
 		ret = lxc_netdev_delete_by_name(hostveth);
@@ -2767,9 +2762,12 @@ static int lxc_setup_netdev_in_child_namespaces(struct lxc_netdev *netdev)
 	 * When the IFLA_IFNAME attribute is passed something like "<prefix>%d"
 	 * netlink will replace the format specifier with an appropriate index.
 	 */
-	if (!netdev->name)
-		netdev->name = netdev->type == LXC_NET_PHYS ?
-			netdev->link : "eth%d";
+	if (netdev->name[0] == '\0') {
+		if (netdev->type == LXC_NET_PHYS)
+			strcpy(netdev->name, netdev->link);
+		else
+			strcpy(netdev->name, "eth%d");
+	}
 
 	/* rename the interface name */
 	if (strcmp(ifname, netdev->name) != 0) {
diff --git a/src/lxc/network.h b/src/lxc/network.h
index 17c095802..b85f9ad29 100644
--- a/src/lxc/network.h
+++ b/src/lxc/network.h
@@ -91,7 +91,7 @@ struct lxc_route6 {
  * @ifindex : Ifindex of the network device.
  */
 struct ifla_veth {
-	char *pair;
+	char pair[IFNAMSIZ];
 	char veth1[IFNAMSIZ];
 	int ifindex;
 };
@@ -151,8 +151,8 @@ struct lxc_netdev {
 	int ifindex;
 	int type;
 	int flags;
-	char *link;
-	char *name;
+	char link[IFNAMSIZ];
+	char name[IFNAMSIZ];
 	char *hwaddr;
 	char *mtu;
 	union netdev_p priv;
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 86d823552..c307c5336 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -816,13 +816,6 @@ static int read_unpriv_netifindex(struct lxc_list *network)
 		if (netdev->type != LXC_NET_VETH)
 			continue;
 
-		netdev->name = malloc(IFNAMSIZ);
-		if (!netdev->name) {
-			ERROR("Out of memory.");
-			close(netpipe);
-			return -1;
-		}
-
 		if (read(netpipe, netdev->name, IFNAMSIZ) != IFNAMSIZ) {
 			close(netpipe);
 			return -1;