[lxc-devel] [lxc/stable-2.0] stable 2.0: cherry picks

brauner on Github lxc-bot at linuxcontainers.org
Tue Aug 29 19:50:23 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170829/5b9915f8/attachment.bin>
-------------- next part --------------
From 06516dc1e0a3286a3adddcd4e9aaf0d39f44ac57 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 24 Aug 2017 15:31:38 +0200
Subject: [PATCH 1/4] openvswitch: delete ports intelligently

So far, when creating veth devices attached to openvswitch bridges we used to
fork() off a thread on container startup. This thread was kept around until the
container shut down. I have no good explanation why we did it that why but it's
certainly not necessary. Instead, let's fork() off the thread on container
shutdown to delete the veth.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/conf.c         | 22 ++++++++++--
 src/lxc/lxc_user_nic.c |  2 +-
 src/lxc/network.c      | 93 +++++++++++++++++++++++++-------------------------
 src/lxc/network.h      |  5 ++-
 4 files changed, 72 insertions(+), 50 deletions(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index c5609ec4e..8b8ae9109 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -2757,7 +2757,7 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 	}
 
 	if (netdev->link) {
-		err = lxc_bridge_attach(handler->lxcpath, handler->name, netdev->link, veth1);
+		err = lxc_bridge_attach(netdev->link, veth1);
 		if (err) {
 			ERROR("failed to attach \"%s\" to bridge \"%s\": %s",
 			      veth1, netdev->link, strerror(-err));
@@ -3135,11 +3135,20 @@ bool lxc_delete_network(struct lxc_handler *handler)
 			char *hostveth;
 			if (netdev->priv.veth_attr.pair) {
 				hostveth = netdev->priv.veth_attr.pair;
+
 				ret = lxc_netdev_delete_by_name(hostveth);
 				if (ret < 0)
 					WARN("Failed to remove interface \"%s\" from host: %s.", hostveth, strerror(-ret));
 				else
 					INFO("Removed interface \"%s\" from host.", hostveth);
+
+				if (is_ovs_bridge(netdev->link)) {
+					ret = lxc_ovs_delete_port(netdev->link, hostveth);
+					if (ret < 0)
+						WARN("Failed to remove port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
+					else
+						INFO("Removed port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
+				}
 			} else if (strlen(netdev->priv.veth_attr.veth1) > 0) {
 				hostveth = netdev->priv.veth_attr.veth1;
 				ret = lxc_netdev_delete_by_name(hostveth);
@@ -3147,7 +3156,16 @@ bool lxc_delete_network(struct lxc_handler *handler)
 					WARN("Failed to remove \"%s\" from host: %s.", hostveth, strerror(-ret));
 				} else {
 					INFO("Removed interface \"%s\" from host.", hostveth);
-					memset((void *)&netdev->priv.veth_attr.veth1, 0, sizeof(netdev->priv.veth_attr.veth1));
+
+					if (is_ovs_bridge(netdev->link)) {
+						ret = lxc_ovs_delete_port(netdev->link, hostveth);
+						if (ret < 0) {
+							WARN("Failed to remove port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
+						} else {
+							INFO("Removed port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
+							memset((void *)&netdev->priv.veth_attr.veth1, 0, sizeof(netdev->priv.veth_attr.veth1));
+						}
+					}
 				}
 			}
 		}
diff --git a/src/lxc/lxc_user_nic.c b/src/lxc/lxc_user_nic.c
index c93b4cc70..96af8363b 100644
--- a/src/lxc/lxc_user_nic.c
+++ b/src/lxc/lxc_user_nic.c
@@ -483,7 +483,7 @@ static bool create_nic(char *nic, char *br, int pid, char **cnic)
 		}
 
 		/* attach veth1 to bridge */
-		ret = lxc_bridge_attach(lxcpath, lxcname, br, veth1buf);
+		ret = lxc_bridge_attach(br, veth1buf);
 		if (ret < 0) {
 			usernic_error("Error attaching %s to %s.\n", veth1buf, br);
 			goto out_del;
diff --git a/src/lxc/network.c b/src/lxc/network.c
index 84fa9d5a5..6685792b6 100644
--- a/src/lxc/network.c
+++ b/src/lxc/network.c
@@ -1395,7 +1395,7 @@ int lxc_ipv6_dest_add(int ifindex, struct in6_addr *dest)
 	return ip_route_dest_add(AF_INET6, ifindex, dest);
 }
 
-static bool is_ovs_bridge(const char *bridge)
+bool is_ovs_bridge(const char *bridge)
 {
 	char brdirname[22 + IFNAMSIZ + 1] = {0};
 	struct stat sb;
@@ -1406,70 +1406,71 @@ static bool is_ovs_bridge(const char *bridge)
 	return false;
 }
 
+struct ovs_veth_args {
+	const char *bridge;
+	const char *nic;
+};
+
 /* Called from a background thread - when nic goes away, remove it from the
  * bridge.
  */
-static void ovs_cleanup_nic(const char *lxcpath, const char *name,
-			    const char *bridge, const char *nic)
+static int lxc_ovs_delete_port_exec(void *data)
 {
-	int ret;
+	struct ovs_veth_args *args = data;
 
-	ret = lxc_check_inherited(NULL, true, &(int){-1}, 1);
-	if (ret < 0)
-		return;
+	execlp("ovs-vsctl", "ovs-vsctl", "del-port", args->bridge, args->nic,
+	       (char *)NULL);
+	return -1;
+}
 
-	TRACE("Registering cleanup thread to remove nic \"%s\" from "
-	      "openvswitch bridge \"%s\"", nic, bridge);
+int lxc_ovs_delete_port(const char *bridge, const char *nic)
+{
+	int ret;
+	char cmd_output[MAXPATHLEN];
+	struct ovs_veth_args args;
 
-	ret = lxc_wait(name, "STOPPED", -1, lxcpath);
+	args.bridge = bridge;
+	args.nic = nic;
+	ret = run_command(cmd_output, sizeof(cmd_output),
+			  lxc_ovs_delete_port_exec, (void *)&args);
 	if (ret < 0) {
-		ERROR("Failed to register cleanup thread to remove nic \"%s\" "
-		      "from  openvswitch bridge \"%s\"", nic, bridge);
-		return;
+		ERROR("Failed to delete \"%s\" from openvswitch bridge \"%s\": "
+		      "%s", bridge, nic, cmd_output);
+		return -1;
 	}
 
-	execlp("ovs-vsctl", "ovs-vsctl", "del-port", bridge, nic, (char *)NULL);
-	exit(EXIT_FAILURE);
+	return 0;
 }
 
-static int attach_to_ovs_bridge(const char *lxcpath, const char *name, const char *bridge, const char *nic)
+static int lxc_ovs_attach_bridge_exec(void *data)
 {
-	pid_t pid;
-	char *cmd;
-	int ret;
+	struct ovs_veth_args *args = data;
 
-	cmd = on_path("ovs-vsctl", NULL);
-	if (!cmd)
-		return -1;
-	free(cmd);
+	execlp("ovs-vsctl", "ovs-vsctl", "add-port", args->bridge, args->nic,
+	       (char *)NULL);
+	return -1;
+}
 
-	pid = fork();
-	if (pid < 0)
+static int lxc_ovs_attach_bridge(const char *bridge, const char *nic)
+{
+	int ret;
+	char cmd_output[MAXPATHLEN];
+	struct ovs_veth_args args;
+
+	args.bridge = bridge;
+	args.nic = nic;
+	ret = run_command(cmd_output, sizeof(cmd_output),
+			  lxc_ovs_attach_bridge_exec, (void *)&args);
+	if (ret < 0) {
+		ERROR("Failed to attach \"%s\" to openvswitch bridge \"%s\": %s",
+		      bridge, nic, cmd_output);
 		return -1;
-	if (pid > 0) {
-		ret = wait_for_pid(pid);
-		if (ret < 0)
-			return ret;
-		pid = fork();
-		if (pid < 0)
-			return -1;  // how to properly recover?
-		if (pid > 0)
-			return 0;
-		ovs_cleanup_nic(lxcpath, name, bridge, nic);
-		exit(0);
 	}
 
-	if (execlp("ovs-vsctl", "ovs-vsctl", "add-port", bridge, nic, (char *)NULL))
-		exit(1);
-	// not reached
-	exit(1);
+	return 0;
 }
 
-/*
- * There is a lxc_bridge_attach, but no need of a bridge detach
- * as automatically done by kernel when a netdev is deleted.
- */
-int lxc_bridge_attach(const char *lxcpath, const char *name, const char *bridge, const char *ifname)
+int lxc_bridge_attach(const char *bridge, const char *ifname)
 {
 	int fd, index, err;
 	struct ifreq ifr;
@@ -1482,7 +1483,7 @@ int lxc_bridge_attach(const char *lxcpath, const char *name, const char *bridge,
 		return -EINVAL;
 
 	if (is_ovs_bridge(bridge))
-		return attach_to_ovs_bridge(lxcpath, name, bridge, ifname);
+		return lxc_ovs_attach_bridge(bridge, ifname);
 
 	fd = socket(AF_INET, SOCK_STREAM, 0);
 	if (fd < 0)
diff --git a/src/lxc/network.h b/src/lxc/network.h
index aa19dae33..b9dec641b 100644
--- a/src/lxc/network.h
+++ b/src/lxc/network.h
@@ -109,7 +109,10 @@ extern int lxc_ipv6_gateway_add(int ifindex, struct in6_addr *gw);
 /*
  * Attach an interface to the bridge
  */
-extern int lxc_bridge_attach(const char *lxcpath, const char *name, const char *bridge, const char *ifname);
+extern int lxc_bridge_attach(const char *bridge, const char *ifname);
+extern int lxc_ovs_delete_port(const char *bridge, const char *nic);
+
+extern bool is_ovs_bridge(const char *bridge);
 
 /*
  * Create default gateway

From 26c91f8b196fa1e27fdafbf77484025ec80fadfc Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 24 Aug 2017 16:10:30 +0200
Subject: [PATCH 2/4] conf: refactor network deletion

I'm ashamed at how aweful my previous code was.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/conf.c | 134 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 69 insertions(+), 65 deletions(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 8b8ae9109..a77b3df9e 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -3083,92 +3083,96 @@ int lxc_create_network(struct lxc_handler *handler)
 bool lxc_delete_network(struct lxc_handler *handler)
 {
 	int ret;
-	struct lxc_list *network = &handler->conf->network;
 	struct lxc_list *iterator;
-	struct lxc_netdev *netdev;
+	struct lxc_list *network = &handler->conf->network;
 	bool deleted_all = true;
 
 	lxc_list_for_each(iterator, network) {
-		netdev = iterator->elem;
+		char *hostveth = NULL;
+		struct lxc_netdev *netdev = iterator->elem;
+
+		/* We can only delete devices whose ifindex we have. If we don't
+		 * have the index it means that we didn't create it.
+		 */
+		if (!netdev->ifindex)
+			continue;
 
-		if (netdev->ifindex != 0 && netdev->type == LXC_NET_PHYS) {
-			if (lxc_netdev_rename_by_index(netdev->ifindex, netdev->link))
+		if (netdev->type == LXC_NET_PHYS) {
+			ret = lxc_netdev_rename_by_index(netdev->ifindex, netdev->link);
+			if (ret < 0)
 				WARN("Failed to rename interface with index %d "
-				     "to its initial name \"%s\".",
+				     "to its initial name \"%s\"",
 				     netdev->ifindex, netdev->link);
+			else
+				TRACE("Renamed interface with index %d to its "
+				      "initial name \"%s\"",
+				      netdev->ifindex, netdev->link);
 			continue;
 		}
 
-		if (netdev_deconf[netdev->type](handler, netdev)) {
-			WARN("Failed to destroy netdev");
-		}
+		ret = netdev_deconf[netdev->type](handler, netdev);
+		if (ret < 0)
+			WARN("Failed to deconfigure network device");
 
 		/* Recent kernel remove the virtual interfaces when the network
 		 * namespace is destroyed but in case we did not move the
 		 * interface to the network namespace, we have to destroy it
 		 */
-		if (netdev->ifindex != 0) {
-			ret = lxc_netdev_delete_by_index(netdev->ifindex);
-			if (-ret == ENODEV) {
-				INFO("Interface \"%s\" with index %d already "
-				     "deleted or existing in different network "
-				     "namespace.",
-				     netdev->name ? 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->ifindex, strerror(-ret));
-			} else {
-				INFO("Removed interface \"%s\" with index %d.",
-				     netdev->name ? netdev->name : "(null)",
-				     netdev->ifindex);
-			}
+		ret = lxc_netdev_delete_by_index(netdev->ifindex);
+		if (-ret == ENODEV) {
+			INFO("Interface \"%s\" with index %d already deleted "
+			     "or existing in different network namespace",
+			     netdev->name ? 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->ifindex, strerror(-ret));
+			continue;
 		}
+		INFO("Removed interface \"%s\" with index %d",
+		     netdev->name ? netdev->name : "(null)", netdev->ifindex);
+
+		if (netdev->type != LXC_NET_VETH)
+			continue;
+
+		if (am_unpriv())
+			continue;
 
 		/* Explicitly delete host veth device to prevent lingering
 		 * devices. We had issues in LXD around this.
 		 */
-		if (netdev->ifindex != 0 && netdev->type == LXC_NET_VETH && !am_unpriv()) {
-			char *hostveth;
-			if (netdev->priv.veth_attr.pair) {
-				hostveth = netdev->priv.veth_attr.pair;
-
-				ret = lxc_netdev_delete_by_name(hostveth);
-				if (ret < 0)
-					WARN("Failed to remove interface \"%s\" from host: %s.", hostveth, strerror(-ret));
-				else
-					INFO("Removed interface \"%s\" from host.", hostveth);
-
-				if (is_ovs_bridge(netdev->link)) {
-					ret = lxc_ovs_delete_port(netdev->link, hostveth);
-					if (ret < 0)
-						WARN("Failed to remove port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
-					else
-						INFO("Removed port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
-				}
-			} else if (strlen(netdev->priv.veth_attr.veth1) > 0) {
-				hostveth = netdev->priv.veth_attr.veth1;
-				ret = lxc_netdev_delete_by_name(hostveth);
-				if (ret < 0) {
-					WARN("Failed to remove \"%s\" from host: %s.", hostveth, strerror(-ret));
-				} else {
-					INFO("Removed interface \"%s\" from host.", hostveth);
-
-					if (is_ovs_bridge(netdev->link)) {
-						ret = lxc_ovs_delete_port(netdev->link, hostveth);
-						if (ret < 0) {
-							WARN("Failed to remove port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
-						} else {
-							INFO("Removed port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
-							memset((void *)&netdev->priv.veth_attr.veth1, 0, sizeof(netdev->priv.veth_attr.veth1));
-						}
-					}
-				}
-			}
+		if (netdev->priv.veth_attr.pair)
+			hostveth = netdev->priv.veth_attr.pair;
+		else
+			hostveth = netdev->priv.veth_attr.veth1;
+		if (*hostveth == '\0')
+			continue;
+
+		ret = lxc_netdev_delete_by_name(hostveth);
+		if (ret < 0) {
+			deleted_all = false;
+			WARN("Failed to remove interface \"%s\" from \"%s\": %s",
+			     hostveth, netdev->link, strerror(-ret));
+			continue;
+		}
+		INFO("Removed interface \"%s\" from \"%s\"", hostveth, netdev->link);
+
+		if (!is_ovs_bridge(netdev->link)) {
+			netdev->priv.veth_attr.veth1[0] = '\0';
+			continue;
 		}
+
+		/* Delete the openvswitch port. */
+		ret = lxc_ovs_delete_port(netdev->link, hostveth);
+		if (ret < 0)
+			WARN("Failed to remove port \"%s\" from openvswitch "
+			     "bridge \"%s\"", hostveth, netdev->link);
+		else
+			INFO("Removed port \"%s\" from openvswitch bridge \"%s\"",
+			     hostveth, netdev->link);
+
+		netdev->priv.veth_attr.veth1[0] = '\0';
 	}
 
 	return deleted_all;

From 9790c1e2edc444089c4813776505cb0c8a9373c8 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 25 Aug 2017 00:02:47 +0200
Subject: [PATCH 3/4] conf: do not check union on wrong net type

This will obviously not work.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/conf.c | 25 ++-----------------------
 1 file changed, 2 insertions(+), 23 deletions(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index a77b3df9e..8a11ff345 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -3049,27 +3049,6 @@ int lxc_create_network(struct lxc_handler *handler)
 
 		netdev = iterator->elem;
 
-		if (netdev->type != LXC_NET_MACVLAN && netdev->priv.macvlan_attr.mode) {
-			ERROR("Invalid macvlan.mode for a non-macvlan netdev");
-			return -1;
-		}
-
-		if (netdev->type != LXC_NET_VETH && netdev->priv.veth_attr.pair) {
-			ERROR("Invalid veth pair for a non-veth netdev");
-			return -1;
-		}
-
-		if (netdev->type != LXC_NET_VLAN && netdev->priv.vlan_attr.vid > 0) {
-			ERROR("Invalid vlan.id for a non-macvlan netdev");
-			return -1;
-		}
-
-		if (netdev->type < 0 || netdev->type > LXC_NET_MAXCONFTYPE) {
-			ERROR("invalid network configuration type '%d'",
-			      netdev->type);
-			return -1;
-		}
-
 		if (netdev_conf[netdev->type](handler, netdev)) {
 			ERROR("failed to create netdev");
 			return -1;
@@ -3114,9 +3093,9 @@ bool lxc_delete_network(struct lxc_handler *handler)
 		if (ret < 0)
 			WARN("Failed to deconfigure network device");
 
-		/* Recent kernel remove the virtual interfaces when the network
+		/* Recent kernels remove the virtual interfaces when the network
 		 * namespace is destroyed but in case we did not move the
-		 * interface to the network namespace, we have to destroy it
+		 * interface to the network namespace, we have to destroy it.
 		 */
 		ret = lxc_netdev_delete_by_index(netdev->ifindex);
 		if (-ret == ENODEV) {

From 67c407682fac3c68ccca3c52f5648162826c8a6a Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 25 Aug 2017 07:35:02 +0200
Subject: [PATCH 4/4] attach: non-functional changes

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/attach.c | 172 ++++++++++++++++++++++++++++++-------------------------
 src/lxc/attach.h |   8 ++-
 2 files changed, 100 insertions(+), 80 deletions(-)

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 360dd6d8e..b66824f23 100644
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -39,6 +39,8 @@
 #include <sys/syscall.h>
 #include <sys/wait.h>
 
+#include <lxc/lxccontainer.h>
+
 #if !HAVE_DECL_PR_CAPBSET_DROP
 #define PR_CAPBSET_DROP 24
 #endif
@@ -58,14 +60,12 @@
 #include "namespace.h"
 #include "utils.h"
 
-#include <lxc/lxccontainer.h>
-
 #if HAVE_SYS_PERSONALITY_H
 #include <sys/personality.h>
 #endif
 
 #ifndef SOCK_CLOEXEC
-#  define SOCK_CLOEXEC                02000000
+#define SOCK_CLOEXEC 02000000
 #endif
 
 #ifndef MS_REC
@@ -73,7 +73,7 @@
 #endif
 
 #ifndef MS_SLAVE
-#define MS_SLAVE (1<<19)
+#define MS_SLAVE (1 << 19)
 #endif
 
 lxc_log_define(lxc_attach, lxc);
@@ -118,7 +118,7 @@ static int lsm_openat(int procfd, pid_t pid, int on_exec)
 static int lsm_set_label_at(int lsm_labelfd, int on_exec, char *lsm_label)
 {
 	int fret = -1;
-	const char* name;
+	const char *name;
 	char *command = NULL;
 
 	name = lsm_name();
@@ -136,7 +136,8 @@ static int lsm_set_label_at(int lsm_labelfd, int on_exec, char *lsm_label)
 	if (strcmp(name, "AppArmor") == 0) {
 		int size;
 
-		command = malloc(strlen(lsm_label) + strlen("changeprofile ") + 1);
+		command =
+		    malloc(strlen(lsm_label) + strlen("changeprofile ") + 1);
 		if (!command) {
 			SYSERROR("Failed to write apparmor profile.");
 			goto out;
@@ -178,12 +179,12 @@ static int lsm_set_label_at(int lsm_labelfd, int on_exec, char *lsm_label)
 #define __PROC_STATUS_LEN (5 + 21 + 7 + 1)
 static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
 {
+	int ret;
+	bool found;
 	FILE *proc_file;
 	char proc_fn[__PROC_STATUS_LEN];
-	bool found;
-	int ret;
-	char *line = NULL;
 	size_t line_bufsz = 0;
+	char *line = NULL;
 	struct lxc_proc_context_info *info = NULL;
 
 	/* Read capabilities. */
@@ -217,7 +218,8 @@ static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
 	fclose(proc_file);
 
 	if (!found) {
-		SYSERROR("Could not read capability bounding set from %s.", proc_fn);
+		SYSERROR("Could not read capability bounding set from %s.",
+			 proc_fn);
 		errno = ENOENT;
 		goto on_error;
 	}
@@ -244,7 +246,6 @@ static int lxc_attach_to_ns(pid_t pid, int which)
 	int fd[LXC_NS_MAX];
 	int i, j, saved_errno;
 
-
 	if (access("/proc/self/ns", X_OK)) {
 		ERROR("Does this kernel version support namespaces?");
 		return -1;
@@ -268,7 +269,8 @@ static int lxc_attach_to_ns(pid_t pid, int which)
 				close(fd[j]);
 
 			errno = saved_errno;
-			SYSERROR("Failed to open namespace: \"%s\".", ns_info[i].proc_name);
+			SYSERROR("Failed to open namespace: \"%s\".",
+				 ns_info[i].proc_name);
 			return -1;
 		}
 	}
@@ -284,7 +286,8 @@ static int lxc_attach_to_ns(pid_t pid, int which)
 				close(fd[j]);
 
 			errno = saved_errno;
-			SYSERROR("Failed to attach to namespace \"%s\".", ns_info[i].proc_name);
+			SYSERROR("Failed to attach to namespace \"%s\".",
+				 ns_info[i].proc_name);
 			return -1;
 		}
 
@@ -307,7 +310,7 @@ static int lxc_attach_remount_sys_proc(void)
 	}
 
 	if (detect_shared_rootfs()) {
-		if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL)) {
+		if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL)) {
 			SYSERROR("Failed to make / rslave.");
 			ERROR("Continuing...");
 		}
@@ -347,9 +350,9 @@ static int lxc_attach_remount_sys_proc(void)
 
 static int lxc_attach_drop_privs(struct lxc_proc_context_info *ctx)
 {
-	int last_cap = lxc_caps_last_cap();
-	int cap;
+	int cap, last_cap;
 
+	last_cap = lxc_caps_last_cap();
 	for (cap = 0; cap <= last_cap; cap++) {
 		if (ctx->capability_mask & (1LL << cap))
 			continue;
@@ -363,11 +366,12 @@ static int lxc_attach_drop_privs(struct lxc_proc_context_info *ctx)
 	return 0;
 }
 
-static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char** extra_env, char** extra_keep)
+static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy,
+				      char **extra_env, char **extra_keep)
 {
 	if (policy == LXC_ATTACH_CLEAR_ENV) {
-		char **extra_keep_store = NULL;
 		int path_kept = 0;
+		char **extra_keep_store = NULL;
 
 		if (extra_keep) {
 			size_t count, i;
@@ -403,6 +407,7 @@ static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char*
 
 		if (clearenv()) {
 			char **p;
+
 			SYSERROR("Failed to clear environment.");
 			if (extra_keep_store) {
 				for (p = extra_keep_store; *p; p++)
@@ -414,6 +419,7 @@ static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char*
 
 		if (extra_keep_store) {
 			size_t i;
+
 			for (i = 0; extra_keep[i]; i++) {
 				if (extra_keep_store[i]) {
 					if (setenv(extra_keep[i], extra_keep_store[i], 1) < 0)
@@ -462,10 +468,9 @@ static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char*
 
 static char *lxc_attach_getpwshell(uid_t uid)
 {
+	int fd, ret;
 	pid_t pid;
 	int pipes[2];
-	int ret;
-	int fd;
 	char *result = NULL;
 
 	/* We need to fork off a process that runs the getent program, and we
@@ -483,21 +488,20 @@ static char *lxc_attach_getpwshell(uid_t uid)
 	}
 
 	if (pid) {
+		int status;
 		FILE *pipe_f;
-		char *line = NULL;
-		size_t line_bufsz = 0;
 		int found = 0;
-		int status;
+		size_t line_bufsz = 0;
+		char *line = NULL;
 
 		close(pipes[1]);
 
 		pipe_f = fdopen(pipes[0], "r");
 		while (getline(&line, &line_bufsz, pipe_f) != -1) {
-			char *token;
-			char *saveptr = NULL;
-			long value;
-			char *endptr = NULL;
 			int i;
+			long value;
+			char *token;
+			char *endptr = NULL, *saveptr = NULL;
 
 			/* If we already found something, just continue to read
 			 * until the pipe doesn't deliver any more data, but
@@ -608,7 +612,7 @@ static char *lxc_attach_getpwshell(uid_t uid)
 	}
 }
 
-static void lxc_attach_get_init_uidgid(uid_t* init_uid, gid_t* init_gid)
+static void lxc_attach_get_init_uidgid(uid_t *init_uid, gid_t *init_gid)
 {
 	FILE *proc_file;
 	char proc_fn[__PROC_STATUS_LEN];
@@ -632,11 +636,11 @@ static void lxc_attach_get_init_uidgid(uid_t* init_uid, gid_t* init_gid)
 		 */
 		ret = sscanf(line, "Uid: %ld", &value);
 		if (ret != EOF && ret == 1) {
-			uid = (uid_t) value;
+			uid = (uid_t)value;
 		} else {
 			ret = sscanf(line, "Gid: %ld", &value);
 			if (ret != EOF && ret == 1)
-				gid = (gid_t) value;
+				gid = (gid_t)value;
 		}
 		if (uid != (uid_t)-1 && gid != (gid_t)-1)
 			break;
@@ -658,42 +662,45 @@ static void lxc_attach_get_init_uidgid(uid_t* init_uid, gid_t* init_gid)
 
 struct attach_clone_payload {
 	int ipc_socket;
-	lxc_attach_options_t* options;
-	struct lxc_proc_context_info* init_ctx;
+	lxc_attach_options_t *options;
+	struct lxc_proc_context_info *init_ctx;
 	lxc_attach_exec_t exec_function;
-	void* exec_payload;
+	void *exec_payload;
 };
 
 static int attach_child_main(void* data);
 
 /* Help the optimizer along if it doesn't know that exit always exits. */
-#define rexit(c)  do { int __c = (c); _exit(__c); return __c; } while(0)
+#define rexit(c)                                                               \
+	do {                                                                   \
+		int __c = (c);                                                 \
+		_exit(__c);                                                    \
+		return __c;                                                    \
+	} while (0)
 
 /* Define default options if no options are supplied by the user. */
 static lxc_attach_options_t attach_static_default_options = LXC_ATTACH_OPTIONS_DEFAULT;
 
-static bool fetch_seccomp(const char *name, const char *lxcpath,
-		struct lxc_proc_context_info *i, lxc_attach_options_t *options)
+static bool fetch_seccomp(struct lxc_container *c,
+			  lxc_attach_options_t *options)
 {
-	struct lxc_container *c;
 	char *path;
 
-	if (!(options->namespaces & CLONE_NEWNS) || !(options->attach_flags & LXC_ATTACH_LSM))
+	if (!(options->namespaces & CLONE_NEWNS) ||
+	    !(options->attach_flags & LXC_ATTACH_LSM)) {
+		free(c->lxc_conf->seccomp);
+		c->lxc_conf->seccomp = NULL;
 		return true;
+	}
 
-	c = lxc_container_new(name, lxcpath);
-	if (!c)
-		return false;
-	i->container = c;
-
-	/* Initialize an empty lxc_conf */
-	if (!c->set_config_item(c, "lxc.seccomp", "")) {
+	/* Remove current setting. */
+	if (!c->set_config_item(c, "lxc.seccomp", ""))
 		return false;
-	}
 
 	/* Fetch the current profile path over the cmd interface. */
 	path = c->get_running_config_item(c, "lxc.seccomp");
 	if (!path) {
+		INFO("Failed to get running config item for lxc.seccomp");
 		return true;
 	}
 
@@ -710,30 +717,35 @@ static bool fetch_seccomp(const char *name, const char *lxcpath,
 		return false;
 	}
 
+	INFO("Retrieved seccomp policy.");
 	return true;
 }
 
 static signed long get_personality(const char *name, const char *lxcpath)
 {
-	char *p = lxc_cmd_get_config_item(name, "lxc.arch", lxcpath);
+	char *p;
 	signed long ret;
 
+	p = lxc_cmd_get_config_item(name, "lxc.arch", lxcpath);
 	if (!p)
 		return -1;
+
 	ret = lxc_config_parse_arch(p);
 	free(p);
+
 	return ret;
 }
 
-int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_function, void* exec_payload, lxc_attach_options_t* options, pid_t* attached_process)
+int lxc_attach(const char *name, const char *lxcpath,
+	       lxc_attach_exec_t exec_function, void *exec_payload,
+	       lxc_attach_options_t *options, pid_t *attached_process)
 {
 	int ret, status;
-	pid_t init_pid, pid, attached_pid, expected;
-	struct lxc_proc_context_info *init_ctx;
-	char* cwd;
-	char* new_cwd;
 	int ipc_sockets[2];
+	char *cwd, *new_cwd;
 	signed long personality;
+	pid_t attached_pid, expected, init_pid, pid;
+	struct lxc_proc_context_info *init_ctx;
 
 	if (!options)
 		options = &attach_static_default_options;
@@ -746,20 +758,19 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 
 	init_ctx = lxc_proc_get_context_info(init_pid);
 	if (!init_ctx) {
-		ERROR("Failed to get context of init process: %ld.",
-		      (long)init_pid);
+		ERROR("Failed to get context of init process: %ld", (long)init_pid);
 		return -1;
 	}
 
 	personality = get_personality(name, lxcpath);
 	if (init_ctx->personality < 0) {
-		ERROR("Failed to get personality of the container.");
+		ERROR("Failed to get personality of the container");
 		lxc_proc_put_context_info(init_ctx);
 		return -1;
 	}
 	init_ctx->personality = personality;
 
-	if (!fetch_seccomp(name, lxcpath, init_ctx, options))
+	if (!fetch_seccomp(init_ctx->container, options))
 		WARN("Failed to get seccomp policy.");
 
 	cwd = getcwd(NULL, 0);
@@ -831,7 +842,6 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 	 *          setns() (otherwise, user namespaces will hate us).
 	 */
 	pid = fork();
-
 	if (pid < 0) {
 		SYSERROR("Failed to create first subprocess.");
 		free(cwd);
@@ -874,7 +884,8 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		}
 
 		/* Get pid of attached process from intermediate process. */
-		ret = lxc_read_nointr_expect(ipc_sockets[0], &attached_pid, sizeof(attached_pid), NULL);
+		ret = lxc_read_nointr_expect(ipc_sockets[0], &attached_pid,
+					     sizeof(attached_pid), NULL);
 		if (ret <= 0) {
 			if (ret != 0)
 				ERROR("Expected to receive pid: %s.", strerror(errno));
@@ -905,7 +916,8 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 
 		/* Wait for the attached process to finish initializing. */
 		expected = 1;
-		ret = lxc_read_nointr_expect(ipc_sockets[0], &status, sizeof(status), &expected);
+		ret = lxc_read_nointr_expect(ipc_sockets[0], &status,
+					     sizeof(status), &expected);
 		if (ret <= 0) {
 			if (ret != 0)
 				ERROR("Expected to receive sequence number 1: %s.", strerror(errno));
@@ -924,7 +936,8 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		 * up its LSM labels.
 		 */
 		expected = 3;
-		ret = lxc_read_nointr_expect(ipc_sockets[0], &status, sizeof(status), &expected);
+		ret = lxc_read_nointr_expect(ipc_sockets[0], &status,
+					     sizeof(status), &expected);
 		if (ret <= 0) {
 			ERROR("Expected to receive sequence number 3: %s.",
 			      strerror(errno));
@@ -932,9 +945,12 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		}
 
 		/* Open LSM fd and send it to child. */
-		if ((options->namespaces & CLONE_NEWNS) && (options->attach_flags & LXC_ATTACH_LSM) && init_ctx->lsm_label) {
+		if ((options->namespaces & CLONE_NEWNS) &&
+		    (options->attach_flags & LXC_ATTACH_LSM) &&
+		    init_ctx->lsm_label) {
 			int on_exec, saved_errno;
 			int labelfd = -1;
+
 			on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ? 1 : 0;
 			/* Open fd for the LSM security module. */
 			labelfd = lsm_openat(procfd, attached_pid, on_exec);
@@ -975,7 +991,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		shutdown(ipc_sockets[0], SHUT_RDWR);
 		close(ipc_sockets[0]);
 		if (to_cleanup_pid)
-			(void) wait_for_pid(to_cleanup_pid);
+			(void)wait_for_pid(to_cleanup_pid);
 		lxc_proc_put_context_info(init_ctx);
 		return -1;
 	}
@@ -988,7 +1004,8 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 	/* Wait for the parent to have setup cgroups. */
 	expected = 0;
 	status = -1;
-	ret = lxc_read_nointr_expect(ipc_sockets[1], &status, sizeof(status), &expected);
+	ret = lxc_read_nointr_expect(ipc_sockets[1], &status, sizeof(status),
+				     &expected);
 	if (ret <= 0) {
 		ERROR("Expected to receive sequence number 0: %s.", strerror(errno));
 		shutdown(ipc_sockets[1], SHUT_RDWR);
@@ -1062,21 +1079,17 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 
 static int attach_child_main(void* data)
 {
-	struct attach_clone_payload* payload = (struct attach_clone_payload*)data;
-	int ipc_socket = payload->ipc_socket;
-	lxc_attach_options_t* options = payload->options;
-	struct lxc_proc_context_info* init_ctx = payload->init_ctx;
+	int expected, fd, lsm_labelfd, ret, status;
+	long flags;
 #if HAVE_SYS_PERSONALITY_H
 	long new_personality;
 #endif
-	int ret;
-	int status;
-	int expected;
-	long flags;
-	int fd;
-	int lsm_labelfd;
 	uid_t new_uid;
 	gid_t new_gid;
+	struct attach_clone_payload* payload = (struct attach_clone_payload*)data;
+	int ipc_socket = payload->ipc_socket;
+	lxc_attach_options_t* options = payload->options;
+	struct lxc_proc_context_info* init_ctx = payload->init_ctx;
 
 	/* Wait for the initial thread to signal us that it's ready for us to
 	 * start initializing.
@@ -1095,7 +1108,8 @@ static int attach_child_main(void* data)
 	 * parent process, otherwise /proc may not properly reflect the new pid
 	 * namespace.
 	 */
-	if (!(options->namespaces & CLONE_NEWNS) && (options->attach_flags & LXC_ATTACH_REMOUNT_PROC_SYS)) {
+	if (!(options->namespaces & CLONE_NEWNS) &&
+	    (options->attach_flags & LXC_ATTACH_REMOUNT_PROC_SYS)) {
 		ret = lxc_attach_remount_sys_proc();
 		if (ret < 0) {
 			shutdown(ipc_socket, SHUT_RDWR);
@@ -1132,7 +1146,9 @@ static int attach_child_main(void* data)
 	/* Always set the environment (specify (LXC_ATTACH_KEEP_ENV, NULL, NULL)
 	 * if you want this to be a no-op).
 	 */
-	ret = lxc_attach_set_environment(options->env_policy, options->extra_env_vars, options->extra_keep_env);
+	ret = lxc_attach_set_environment(options->env_policy,
+					 options->extra_env_vars,
+					 options->extra_keep_env);
 	if (ret < 0) {
 		ERROR("Could not set initial environment for attached process.");
 		shutdown(ipc_socket, SHUT_RDWR);
@@ -1176,7 +1192,8 @@ static int attach_child_main(void* data)
 			rexit(-1);
 		}
 	}
-	if ((new_uid != 0 || options->namespaces & CLONE_NEWUSER) && setuid(new_uid)) {
+	if ((new_uid != 0 || options->namespaces & CLONE_NEWUSER) &&
+	    setuid(new_uid)) {
 		SYSERROR("Switching to container uid.");
 		shutdown(ipc_socket, SHUT_RDWR);
 		rexit(-1);
@@ -1213,7 +1230,8 @@ static int attach_child_main(void* data)
 		rexit(-1);
 	}
 
-	if ((options->namespaces & CLONE_NEWNS) && (options->attach_flags & LXC_ATTACH_LSM) && init_ctx->lsm_label) {
+	if ((options->namespaces & CLONE_NEWNS) &&
+	    (options->attach_flags & LXC_ATTACH_LSM) && init_ctx->lsm_label) {
 		int on_exec;
 		/* Receive fd for LSM security module. */
 		ret = lxc_abstract_unix_recv_fds(ipc_socket, &lsm_labelfd, 1, NULL, 0);
diff --git a/src/lxc/attach.h b/src/lxc/attach.h
index 39fcab783..fb6bc5a07 100644
--- a/src/lxc/attach.h
+++ b/src/lxc/attach.h
@@ -24,8 +24,8 @@
 #ifndef __LXC_ATTACH_H
 #define __LXC_ATTACH_H
 
-#include <sys/types.h>
 #include <lxc/attach_options.h>
+#include <sys/types.h>
 
 struct lxc_conf;
 
@@ -36,6 +36,8 @@ struct lxc_proc_context_info {
 	unsigned long long capability_mask;
 };
 
-extern int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_function, void* exec_payload, lxc_attach_options_t* options, pid_t* attached_process);
+extern int lxc_attach(const char *name, const char *lxcpath,
+		      lxc_attach_exec_t exec_function, void *exec_payload,
+		      lxc_attach_options_t *options, pid_t *attached_process);
 
-#endif
+#endif /* __LXC_ATTACH_H */


More information about the lxc-devel mailing list