[lxc-devel] [PATCH 1/1] attach: mount a sane prox for LSM setup

[Serge Hallyn]

To set lsm labels, a namespace-local proc mount is needed.

If a container does not have a lxc.mount.auto = proc set, then
tasks in the container do not have a correct /proc mount until
init feels like doing the mount.  At startup we handlie this
by mounting a temporary /proc if needed.  We weren't doing this
at attach, though, so that

lxc-start -n $container
lxc-wait -t 5 -s RUNNING -n $container
lxc-attach -n $container -- uname -a

could in a racy way fail with something like

lxc-attach: lsm/apparmor.c: apparmor_process_label_set: 183 No such file or directory - failed to change apparmor profile to lxc-container-default

Thanks to Chris Townsend for finding this bug at
https://bugs.launchpad.net/ubuntu/+source/lxc/+bug/1452451

Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
 src/lxc/attach.c |  8 ++++++++
 src/lxc/conf.c   | 44 +-------------------------------------------
 src/lxc/utils.c  | 43 +++++++++++++++++++++++++++++++++++++++++++
 src/lxc/utils.h  |  1 +
 4 files changed, 53 insertions(+), 43 deletions(-)

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 69dafd4..731d7a6 100644
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -1040,10 +1040,18 @@ static int attach_child_main(void* data)
 	/* set new apparmor profile/selinux context */
 	if ((options->namespaces & CLONE_NEWNS) && (options->attach_flags & LXC_ATTACH_LSM)) {
 		int on_exec;
+		int proc_mounted;
 
 		on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ? 1 : 0;
+		proc_mounted = mount_proc_if_needed("/");
+		if (proc_mounted == -1) {
+			ERROR("Error mounting a sane /proc");
+			rexit(-1);
+		}
 		ret = lsm_process_label_set(init_ctx->lsm_label,
 				init_ctx->container->lxc_conf, 0, on_exec);
+		if (proc_mounted)
+			umount("/proc");
 		if (ret < 0) {
 			rexit(-1);
 		}
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index f0d2008..1b2ff05 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -3551,48 +3551,6 @@ int ttys_shift_ids(struct lxc_conf *c)
 	return 0;
 }
 
-/*
- * _do_tmp_proc_mount: Mount /proc inside container if not already
- * mounted
- *
- * @rootfs : the rootfs where proc should be mounted
- *
- * Returns < 0 on failure, 0 if the correct proc was already mounted
- * and 1 if a new proc was mounted.
- */
-static int do_tmp_proc_mount(const char *rootfs)
-{
-	char path[MAXPATHLEN];
-	char link[20];
-	int linklen, ret;
-
-	ret = snprintf(path, MAXPATHLEN, "%s/proc/self", rootfs);
-	if (ret < 0 || ret >= MAXPATHLEN) {
-		SYSERROR("proc path name too long");
-		return -1;
-	}
-	memset(link, 0, 20);
-	linklen = readlink(path, link, 20);
-	INFO("I am %d, /proc/self points to '%s'", getpid(), link);
-	ret = snprintf(path, MAXPATHLEN, "%s/proc", rootfs);
-	if (linklen < 0) /* /proc not mounted */
-		goto domount;
-	/* can't be longer than rootfs/proc/1 */
-	if (strncmp(link, "1", linklen) != 0) {
-		/* wrong /procs mounted */
-		umount2(path, MNT_DETACH); /* ignore failure */
-		goto domount;
-	}
-	/* the right proc is already mounted */
-	return 0;
-
-domount:
-	if (mount("proc", path, "proc", 0, NULL))
-		return -1;
-	INFO("Mounted /proc in container for security transition");
-	return 1;
-}
-
 int tmp_proc_mount(struct lxc_conf *lxc_conf)
 {
 	int mounted;
@@ -3604,7 +3562,7 @@ int tmp_proc_mount(struct lxc_conf *lxc_conf)
 		} else
 			mounted = 1;
 	} else
-		mounted = do_tmp_proc_mount(lxc_conf->rootfs.mount);
+		mounted = mount_proc_if_needed(lxc_conf->rootfs.mount);
 	if (mounted == -1) {
 		SYSERROR("failed to mount /proc in the container.");
 		return -1;
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index fe71e9a..0588fdd 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1663,3 +1663,46 @@ int setproctitle(char *title)
 
 	return ret;
 }
+
+/*
+ * Mount a proc under @rootfs if proc self points to a pid other than
+ * my own.  This is needed to have a known-good proc mount for setting
+ * up LSMs both at container startup and attach.
+ *
+ * @rootfs : the rootfs where proc should be mounted
+ *
+ * Returns < 0 on failure, 0 if the correct proc was already mounted
+ * and 1 if a new proc was mounted.
+ */
+int mount_proc_if_needed(const char *rootfs)
+{
+	char path[MAXPATHLEN];
+	char link[20];
+	int linklen, ret;
+
+	ret = snprintf(path, MAXPATHLEN, "%s/proc/self", rootfs);
+	if (ret < 0 || ret >= MAXPATHLEN) {
+		SYSERROR("proc path name too long");
+		return -1;
+	}
+	memset(link, 0, 20);
+	linklen = readlink(path, link, 20);
+	INFO("I am %d, /proc/self points to '%s'", getpid(), link);
+	ret = snprintf(path, MAXPATHLEN, "%s/proc", rootfs);
+	if (linklen < 0) /* /proc not mounted */
+		goto domount;
+	/* can't be longer than rootfs/proc/1 */
+	if (strncmp(link, "1", linklen) != 0) {
+		/* wrong /procs mounted */
+		umount2(path, MNT_DETACH); /* ignore failure */
+		goto domount;
+	}
+	/* the right proc is already mounted */
+	return 0;
+
+domount:
+	if (mount("proc", path, "proc", 0, NULL))
+		return -1;
+	INFO("Mounted /proc in container for security transition");
+	return 1;
+}
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index cc18906..95ea2af 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -287,3 +287,4 @@ bool switch_to_ns(pid_t pid, const char *ns);
 int is_dir(const char *path);
 char *get_template_path(const char *t);
 int setproctitle(char *title);
+int mount_proc_if_needed(const char *rootfs);
-- 
2.1.4