start: lxc_setup() after unshare(CLONE_NEWCGROUP)

When the running kernel supports cgroup namespaces and users want to manually
set up cgroups via lxc.hook.mount before the init binary starts the cgroup
namespace needs to be already unshared. Otherwise the view on the cgroup mounts
is wrong. This commit places the call to lxc_setup() after the
LXC_SYNC_POST_CGROUP barrier.

Before this commit, the tty fds we allocate from a fresh devpts instance in the
container's namespaces before the init binary starts were referring to the
host's cgroup namespace since lxc_setup() was called before
unshare(CLONE_NEWCGROUP). Although not a security risk at this point since
setns() restricts its calls to /proc/<self>/ns files it's still better to do it
*after* the cgroup namespace has been unshared.

Adding a Suggested-by line for the lxc.mount.hook fix for Quentin.

Closes #1597.

Suggested-by: Quentin Dufour <quentin@dufour.tk>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index c8dba54578379f05f83656b26183608560426118..ab038d36db5fb6b0ee6d403c039b56d2720fbc16 100644 (file)
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -4237,7 +4237,7 @@ int lxc_setup(struct lxc_handler *handler)
                return -1;
        }
 
-       NOTICE("'%s' is setup.", name);
+       NOTICE("Container \"%s\" is set up", name);
 
        return 0;
 }

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 481776186a8d614f8c6c74e65012b27f9b090b97..9fa208f2bc2042e46bdbb117d15948ae2da26660 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -1012,12 +1012,6 @@ static int do_start(void *data)
                     "standard file descriptors. Migration will not work.");
        }
 
-       /* Setup the container, ip, names, utsname, ... */
-       if (lxc_setup(handler)) {
-               ERROR("Failed to setup container \"%s\".", handler->name);
-               goto out_warn_father;
-       }
-
        /* Ask father to setup cgroups and wait for him to finish. */
        if (lxc_sync_barrier_parent(handler, LXC_SYNC_CGROUP))
                goto out_error;
@@ -1042,6 +1036,12 @@ static int do_start(void *data)
                INFO("Unshared CLONE_NEWCGROUP.");
        }
 
+       /* Setup the container, ip, names, utsname, ... */
+       if (lxc_setup(handler)) {
+               ERROR("Failed to setup container \"%s\".", handler->name);
+               goto out_warn_father;
+       }
+
        /* 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;
@@ -1161,6 +1161,9 @@ static int do_start(void *data)
 
        setsid();
 
+       if (lxc_sync_barrier_parent(handler, LXC_SYNC_CGROUP_LIMITS))
+               goto out_warn_father;
+
        /* After this call, we are in error because this ops should not return
         * as it execs.
         */
@@ -1485,20 +1488,18 @@ static int lxc_spawn(struct lxc_handler *handler)
                goto out_delete_net;
        }
 
+       if (lxc_sync_barrier_child(handler, LXC_SYNC_CGROUP_UNSHARE))
+               goto out_delete_net;
+
        if (!cgroup_setup_limits(handler, true)) {
                ERROR("Failed to setup the devices cgroup for container \"%s\".", name);
                goto out_delete_net;
        }
+       TRACE("Set up cgroup device limits");
 
        cgroup_disconnect();
        cgroups_connected = false;
 
-       /* Read tty fds allocated by child. */
-       if (lxc_recv_ttys_from_child(handler) < 0) {
-               ERROR("Failed to receive tty info from child process.");
-               goto out_delete_net;
-       }
-
        /* Tell the child to complete its initialization and wait for it to exec
         * or return an error. (The child will never return
         * LXC_SYNC_POST_CGROUP+1. It will either close the sync pipe, causing
@@ -1508,6 +1509,12 @@ static int lxc_spawn(struct lxc_handler *handler)
        if (lxc_sync_barrier_child(handler, LXC_SYNC_POST_CGROUP))
                return -1;
 
+       /* Read tty fds allocated by child. */
+       if (lxc_recv_ttys_from_child(handler) < 0) {
+               ERROR("Failed to receive tty info from child process.");
+               goto out_delete_net;
+       }
+
        if (handler->ops->post_start(handler, handler->data))
                goto out_abort;
 

diff --git a/src/lxc/sync.h b/src/lxc/sync.h
index 12a8b95926e8cb0ea85983c7ed586d4037b3fc12..744db613e4d813ba2a766cd23dbd196d35c43da8 100644 (file)
--- a/src/lxc/sync.h
+++ b/src/lxc/sync.h
@@ -30,6 +30,8 @@ enum {
        LXC_SYNC_CONFIGURE,
        LXC_SYNC_POST_CONFIGURE,
        LXC_SYNC_CGROUP,
+       LXC_SYNC_CGROUP_UNSHARE,
+       LXC_SYNC_CGROUP_LIMITS,
        LXC_SYNC_POST_CGROUP,
        LXC_SYNC_RESTART,
        LXC_SYNC_POST_RESTART,