[lxc-devel] [lxc/master] start: simplify cgroup namespace preservation

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 485 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20171222/78fc1710/attachment.bin>
-------------- next part --------------
From 4b826b1fdc516c71c7222ef68a45c4f6ad964df1 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 22 Dec 2017 17:11:45 +0100
Subject: [PATCH 1/2] start: make us dumpable

When set set{u,g}id() the kernel will make us undumpable. This is unnecessary
since we can guarantee that whatever is running inside the child process at
this point this is fully trusted by the parent. Making us dumpable let's users
use debuggers on the child process before the exec as well and also allows us
to open /proc/<child-pid> files in lieu of the child.
Note, that we only need to perform the prctl(PR_SET_DUMPABLE, ...) if our
effective uid on the host is not 0. If our effective uid on the host is 0 then
we will keep all capabilities in the child user namespace across set{g,u}id().

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

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 83991cf53..6ac7784e6 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -930,14 +930,22 @@ static int do_start(void *data)
 	 * privilege over our namespace.
 	 */
 	if (!lxc_list_empty(&handler->conf->id_map)) {
-		if (lxc_switch_uid_gid(0, 0) < 0)
+		ret = lxc_switch_uid_gid(0, 0);
+		if (ret < 0)
 			goto out_warn_father;
 
 		/* Drop groups only after we switched to a valid gid in the new
 		 * user namespace.
 		 */
-		if (lxc_setgroups(0, NULL) < 0)
+		ret = lxc_setgroups(0, NULL);
+		if (ret < 0)
 			goto out_warn_father;
+
+		if (!handler->am_root) {
+			ret = prctl(PR_SET_DUMPABLE, 1, 0, 0, 0);
+			if (ret < 0)
+				goto out_warn_father;
+		}
 	}
 
 	if (access(handler->lxcpath, X_OK)) {

From 8bf3abfbd04670101dad7c2adc197a77054a3d70 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 22 Dec 2017 17:18:50 +0100
Subject: [PATCH 2/2] start: simplify cgroup namespace preservation

Since we are now dumpable we can open /proc/<child-pid>/ns/cgroup so let's
avoid the overhead of sending around fds.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/start.c | 46 ++++++++++++++--------------------------------
 1 file changed, 14 insertions(+), 32 deletions(-)

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 6ac7784e6..c7d87fb3c 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -868,7 +868,7 @@ void lxc_abort(const char *name, struct lxc_handler *handler)
 
 static int do_start(void *data)
 {
-	int fd, ret;
+	int ret;
 	struct lxc_list *iterator;
 	char path[PATH_MAX];
 	struct lxc_handler *handler = data;
@@ -1014,30 +1014,12 @@ static int do_start(void *data)
 	/* Setup the container, ip, names, utsname, ... */
 	ret = lxc_setup(handler);
 	close(handler->data_sock[1]);
+	close(handler->data_sock[0]);
 	if (ret < 0) {
 		ERROR("Failed to setup container \"%s\".", handler->name);
-		close(handler->data_sock[0]);
 		goto out_warn_father;
 	}
 
-	if (handler->clone_flags & CLONE_NEWCGROUP) {
-		fd = lxc_preserve_ns(lxc_raw_getpid(), "cgroup");
-		if (fd < 0) {
-			ERROR("%s - Failed to preserve cgroup namespace", strerror(errno));
-			close(handler->data_sock[0]);
-			goto out_warn_father;
-		}
-
-		ret = lxc_abstract_unix_send_fds(handler->data_sock[0], &fd, 1, NULL, 0);
-		close(fd);
-		if (ret < 0) {
-			ERROR("%s - Failed to preserve cgroup namespace", strerror(errno));
-			close(handler->data_sock[0]);
-			goto out_warn_father;
-		}
-	}
-	close(handler->data_sock[0]);
-
 	/* Set the label to change to when we exec(2) the container's init. */
 	if (lsm_process_label_set(NULL, handler->conf, 1, 1) < 0)
 		goto out_warn_father;
@@ -1442,7 +1424,7 @@ static int lxc_spawn(struct lxc_handler *handler)
 		if (handler->on_clone_flags & ns_info[i].clone_flag)
 			INFO("Cloned %s", ns_info[i].flag_name);
 
-	if (!preserve_ns(handler->nsfd, handler->clone_flags & ~CLONE_NEWNET, handler->pid)) {
+	if (!preserve_ns(handler->nsfd, handler->on_clone_flags, handler->pid)) {
 		ERROR("Failed to preserve cloned namespaces for lxc.hook.stop");
 		goto out_delete_net;
 	}
@@ -1547,6 +1529,17 @@ static int lxc_spawn(struct lxc_handler *handler)
 	cgroup_disconnect();
 	cgroups_connected = false;
 
+	if (handler->clone_flags & CLONE_NEWCGROUP) {
+		/* Now we're ready to preserve the cgroup namespace */
+		ret = lxc_preserve_ns(handler->pid, "cgroup");
+		if (ret < 0) {
+			ERROR("%s - Failed to preserve cgroup namespace", strerror(errno));
+			goto out_delete_net;
+		}
+		handler->nsfd[LXC_NS_CGROUP] = ret;
+		DEBUG("Preserved cgroup namespace via fd %d", ret);
+	}
+
 	snprintf(pidstr, 20, "%d", handler->pid);
 	if (setenv("LXC_PID", pidstr, 1))
 		SYSERROR("Failed to set environment variable: LXC_PID=%s.", pidstr);
@@ -1585,17 +1578,6 @@ static int lxc_spawn(struct lxc_handler *handler)
 		goto out_delete_net;
 	}
 
-	if (handler->clone_flags & CLONE_NEWCGROUP) {
-		ret = lxc_abstract_unix_recv_fds(handler->data_sock[1],
-						 &handler->nsfd[LXC_NS_CGROUP],
-						 1, NULL, 0);
-		if (ret < 0) {
-			ERROR("%s - Failed to preserve cgroup namespace", strerror(errno));
-			goto out_delete_net;
-		}
-		DEBUG("Preserved cgroup namespace via fd %d", handler->nsfd[LXC_NS_CGROUP]);
-	}
-
 	if (handler->ops->post_start(handler, handler->data))
 		goto out_abort;