attach: mount a sane prox for LSM setup

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@ubuntu.com>
Acked-by: Stéphane Graber <stgraber@ubuntu.com>

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 69dafd4bf3744d9dee0ff928233a1b83ef12460a..731d7a63294074a36766e778a92ec8a8c0ccd617 100644 (file)
--- 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 7a4adce0d4ed3f5699b2accb7a36482f6cf16d52..9870455b3caeccb7470b822d995a74f46893ff79 100644 (file)
--- 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 e597725f55cd1d4107189a0d56fd871b0af2300b..ac00af3b5996a361d4436b8964e93cc7420bd337 100644 (file)
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1401,3 +1401,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 5dbc803734aa50c0a8c54f9ca71af14ccf6a095e..e9e07d98b18150a395f2f76d170a249786fc2767 100644 (file)
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -280,3 +280,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);