Merge pull request #1773 from brauner/2017-08-31/ensure_lxc_user_nic_tests_privilege_over_netns

src/lxc/conf.c

@@ -3067,41 +3067,35 @@ static bool verify_start_hooks(struct lxc_conf *conf)
 static int lxc_send_ttys_to_parent(struct lxc_handler *handler)
 {
 	int i;
-	int *ttyfds;
-	struct lxc_pty_info *pty_info;
 	struct lxc_conf *conf = handler->conf;
-	const struct lxc_tty_info *tty_info = &conf->tty_info;
-	int sock = handler->ttysock[0];
+	struct lxc_tty_info *tty_info = &conf->tty_info;
+	int sock = handler->data_sock[0];
 	int ret = -1;
-	size_t num_ttyfds = (2 * conf->tty);
 
-	ttyfds = malloc(num_ttyfds * sizeof(int));
-	if (!ttyfds)
-		return -1;
+	for (i = 0; i < conf->tty; i++) {
+		int ttyfds[2];
+		struct lxc_pty_info *pty_info = &tty_info->pty_info[i];
 
-	for (i = 0; i < num_ttyfds; i++) {
-		pty_info = &tty_info->pty_info[i / 2];
-		ttyfds[i++] = pty_info->slave;
-		ttyfds[i] = pty_info->master;
-		TRACE("send pty \"%s\" with master fd %d and slave fd %d to "
-		      "parent",
-		      pty_info->name, pty_info->master, pty_info->slave);
+		ttyfds[0] = pty_info->master;
+		ttyfds[1] = pty_info->slave;
+
+		ret = lxc_abstract_unix_send_fds(sock, ttyfds, 2, NULL, 0);
+		if (ret < 0)
+			break;
+
+		TRACE("Send pty \"%s\" with master fd %d and slave fd %d to "
+		      "parent", pty_info->name, pty_info->master, pty_info->slave);
 	}
 
-	ret = lxc_abstract_unix_send_fds(sock, ttyfds, num_ttyfds, NULL, 0);
 	if (ret < 0)
-		ERROR("failed to send %d ttys to parent: %s", conf->tty,
+		ERROR("Failed to send %d ttys to parent: %s", conf->tty,
 		      strerror(errno));
 	else
-		TRACE("sent %d ttys to parent", conf->tty);
-
-	close(handler->ttysock[0]);
-	close(handler->ttysock[1]);
+		TRACE("Sent %d ttys to parent", conf->tty);
 
-	for (i = 0; i < num_ttyfds; i++)
-		close(ttyfds[i]);
-
-	free(ttyfds);
+	close(handler->data_sock[0]);
+	close(handler->data_sock[1]);
+	lxc_delete_tty(tty_info);
 
 	return ret;
 }
@@ -3129,6 +3123,11 @@ int lxc_setup(struct lxc_handler *handler)
 		return -1;
 	}
 
+	if (lxc_network_send_name_and_ifindex_to_parent(handler) < 0) {
+		ERROR("Failed to network device names and ifindices to parent");
+		return -1;
+	}
+
 	if (lxc_conf->autodev > 0) {
 		if (mount_autodev(name, &lxc_conf->rootfs, lxcpath)) {
 			ERROR("failed to mount /dev in the container");
@@ -3460,17 +3459,6 @@ int lxc_clear_hooks(struct lxc_conf *c, const char *key)
 	return 0;
 }
 
-static void lxc_clear_saved_nics(struct lxc_conf *conf)
-{
-	int i;
-
-	if (!conf->saved_nics)
-		return;
-	for (i=0; i < conf->num_savednics; i++)
-		free(conf->saved_nics[i].orig_name);
-	free(conf->saved_nics);
-}
-
 static inline void lxc_clear_aliens(struct lxc_conf *conf)
 {
 	struct lxc_list *it,*next;
@@ -3525,7 +3513,6 @@ void lxc_conf_free(struct lxc_conf *conf)
 	lxc_clear_cgroups(conf, "lxc.cgroup");
 	lxc_clear_hooks(conf, "lxc.hook");
 	lxc_clear_mount_entries(conf);
-	lxc_clear_saved_nics(conf);
 	lxc_clear_idmaps(conf);
 	lxc_clear_groups(conf);
 	lxc_clear_includes(conf);

src/lxc/conf.h

@@ -244,8 +244,6 @@ struct lxc_conf {
 	struct lxc_list cgroup;
 	struct lxc_list id_map;
 	struct lxc_list network;
-	struct saved_nic *saved_nics;
-	int num_savednics;
 	int auto_mounts;
 	struct lxc_list mount_list;
 	struct lxc_list caps;

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;
 }

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);
 		}

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')
@@ -285,6 +294,10 @@ void lxc_log_configured_netdevs(const struct lxc_conf *conf)
 			break;
 		case LXC_NET_PHYS:
 			TRACE("type: phys");
+			if (netdev->priv.phys_attr.ifindex > 0) {
+				TRACE("host side ifindex for phys device: %d",
+				      netdev->priv.phys_attr.ifindex);
+			}
 			break;
 		case LXC_NET_EMPTY:
 			TRACE("type: empty");
@@ -300,9 +313,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 +363,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 +512,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)

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);

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,13 @@ 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') {
+			if (!lxc_mkifname(template))
+				goto out_unlock;
 
-		if (!netdev->priv.veth_attr.pair)
-			goto out_unlock;
+			strcpy(netdev->priv.veth_attr.veth1, template);
+		}
 	}
 
 	has_error = false;

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;
 };
@@ -107,10 +107,19 @@ struct ifla_macvlan {
 	int mode; /* private, vepa, bridge, passthru */
 };
 
+/* Contains information about the physical network device as seen from the host.
+ * @ifindex : The ifindex of the physical network device in the host's network
+ *            namespace.
+ */
+struct ifla_phys {
+	int ifindex;
+};
+
 union netdev_p {
+	struct ifla_macvlan macvlan_attr;
+	struct ifla_phys phys_attr;
 	struct ifla_veth veth_attr;
 	struct ifla_vlan vlan_attr;
-	struct ifla_macvlan macvlan_attr;
 };
 
 /*
@@ -151,8 +160,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;
@@ -166,16 +175,11 @@ struct lxc_netdev {
 	char *downscript;
 };
 
-struct saved_nic {
-	int ifindex;
-	char *orig_name;
-};
-
 /* Convert a string mac address to a socket structure. */
 extern int lxc_convert_mac(char *macaddr, struct sockaddr *sockaddr);
 
 /* Move a device between namespaces. */
-extern int lxc_netdev_move_by_index(int ifindex, pid_t pid, const char* ifname);
+extern int lxc_netdev_move_by_index(int ifindex, pid_t pid, const char *ifname);
 extern int lxc_netdev_move_by_name(const char *ifname, pid_t pid,
 				   const char *newname);
 
@@ -252,7 +256,7 @@ extern int lxc_neigh_proxy_off(const char *name, int family);
 /* Generate a new unique network interface name.
  * Allocated memory must be freed by caller.
  */
-extern char *lxc_mkifname(const char *template);
+extern char *lxc_mkifname(char *template);
 
 extern const char *lxc_net_type_to_str(int type);
 extern int setup_private_host_hw_addr(char *veth1);
@@ -268,8 +272,12 @@ extern int lxc_find_gateway_addresses(struct lxc_handler *handler);
 extern int lxc_create_network_unpriv(const char *lxcpath, char *lxcname,
 				     struct lxc_list *network, pid_t pid);
 extern int lxc_requests_empty_network(struct lxc_handler *handler);
-extern void lxc_restore_phys_nics_to_netns(int netnsfd, struct lxc_conf *conf);
+extern int lxc_restore_phys_nics_to_netns(struct lxc_handler *handler);
 extern int lxc_setup_network_in_child_namespaces(const struct lxc_conf *conf,
 						 struct lxc_list *network);
+extern int lxc_network_send_veth_names_to_child(struct lxc_handler *handler);
+extern int lxc_network_recv_veth_names_from_parent(struct lxc_handler *handler);
+extern int lxc_network_send_name_and_ifindex_to_parent(struct lxc_handler *handler);
+extern int lxc_network_recv_name_and_ifindex_from_child(struct lxc_handler *handler);
 
 #endif /* __LXC_NETWORK_H */

src/lxc/start.h

@@ -35,6 +35,7 @@
 #include "namespace.h"
 
 struct lxc_handler {
+	bool am_root;
 	pid_t pid;
 	char *name;
 	lxc_state_t state;
@@ -49,8 +50,10 @@ struct lxc_handler {
 	const char *lxcpath;
 	void *cgroup_data;
 
-	/* socketpair for child->parent tty fd passing */
-	int ttysock[2];
+	/* Abstract unix domain SOCK_DGRAM socketpair to pass arbitrary data
+	 * between child and parent.
+	 */
+	int data_sock[2];
 
 	/* indicates whether should we close std{in,out,err} on start */
 	bool backgrounded;

src/lxc/utils.c

@@ -2409,3 +2409,24 @@ bool has_fs_type(const char *path, fs_type_magic magic_val)
 
 	return has_type;
 }
+
+bool lxc_nic_exists(char *nic)
+{
+#define __LXC_SYS_CLASS_NET_LEN 15 + IFNAMSIZ + 1
+	char path[__LXC_SYS_CLASS_NET_LEN];
+	int ret;
+	struct stat sb;
+
+	if (!strcmp(nic, "none"))
+		return true;
+
+	ret = snprintf(path, __LXC_SYS_CLASS_NET_LEN, "/sys/class/net/%s", nic);
+	if (ret < 0 || (size_t)ret >= __LXC_SYS_CLASS_NET_LEN)
+		return false;
+
+	ret = stat(path, &sb);
+	if (ret < 0)
+		return false;
+
+	return true;
+}

src/lxc/utils.h

@@ -402,5 +402,6 @@ extern void *must_realloc(void *orig, size_t sz);
 typedef __typeof__(((struct statfs *)NULL)->f_type) fs_type_magic;
 extern bool has_fs_type(const char *path, fs_type_magic magic_val);
 extern bool is_fs_type(const struct statfs *fs, fs_type_magic magic_val);
+extern bool lxc_nic_exists(char *nic);
 
 #endif /* __LXC_UTILS_H */