start: fix cgroup namespace preservation

Prior to this patch we raced with a very short-lived init process. Essentially,
the init process could exit before we had time to record the cgroup namespace
causing the container to abort and report ABORTING to the caller when it
actually started just fine. Let's not do this.

(This uses syscall(SYS_getpid) in the the child to retrieve the pid just in case
we're on an older glibc version and we end up in the namespace sharing branch
of the actual lxc_clone() call.)

Additionally this fixes the shortlived tests. They were faulty so far and
should have actually failed because of the cgroup namespace recording race but
the ret variable used to return from the function was not correctly
initialized. This fixes it.
Furthermore, the shortlived tests used the c->error_num variable to determine
success or failure but this is actually not correct when the container is
started daemonized.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/criu.c b/src/lxc/criu.c
index 3b542f599b0f067f6a29976a6d962ae88f57986a..9a389f0d761ac7e305d1b51cb9877bd8f2cd9105 100644 (file)
--- a/src/lxc/criu.c
+++ b/src/lxc/criu.c
@@ -827,7 +827,7 @@ out_unlock:
 // monitor process. do_restore calls exit() if it fails.
 static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_opts *opts, char *criu_version)
 {
-       int fd;
+       int fd, ret;
        pid_t pid;
        struct lxc_handler *handler;
        int status = 0;
@@ -868,7 +868,11 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
                goto out_fini_handler;
        }
 
-       resolve_clone_flags(handler);
+       ret = resolve_clone_flags(handler);
+       if (ret < 0) {
+               ERROR("%s - Unsupported clone flag specified", strerror(errno));
+               goto out_fini_handler;
+       }
 
        if (pipe(pipes) < 0) {
                SYSERROR("pipe() failed");

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 5875298114fe609fc3e6f2de06b773c1c177b06f..7117120598dd7576491e2cd1978427b47ad58456 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -883,13 +883,14 @@ static int must_drop_cap_sys_boot(struct lxc_conf *conf)
 
 static int do_start(void *data)
 {
+       int fd, ret;
        struct lxc_list *iterator;
-       struct lxc_handler *handler = data;
-       int devnull_fd = -1, ret;
        char path[PATH_MAX];
        bool have_cap_setgid;
        uid_t new_uid;
        gid_t new_gid;
+       int devnull_fd = -1;
+       struct lxc_handler *handler = data;
 
        if (sigprocmask(SIG_SETMASK, &handler->oldmask, NULL)) {
                SYSERROR("Failed to set signal mask.");
@@ -1009,7 +1010,7 @@ static int do_start(void *data)
         *
         *      8:cpuset:/
         */
-       if (cgns_supported()) {
+       if (handler->clone_flags & CLONE_NEWCGROUP) {
                if (unshare(CLONE_NEWCGROUP) < 0) {
                        INFO("Failed to unshare CLONE_NEWCGROUP.");
                        goto out_warn_father;
@@ -1030,13 +1031,31 @@ static int do_start(void *data)
 
        /* Setup the container, ip, names, utsname, ... */
        ret = lxc_setup(handler);
-       close(handler->data_sock[0]);
        close(handler->data_sock[1]);
        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(syscall(SYS_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;
@@ -1207,7 +1226,7 @@ static int lxc_recv_ttys_from_child(struct lxc_handler *handler)
        return ret;
 }
 
-void resolve_clone_flags(struct lxc_handler *handler)
+int resolve_clone_flags(struct lxc_handler *handler)
 {
        handler->clone_flags = CLONE_NEWPID | CLONE_NEWNS;
 
@@ -1218,18 +1237,34 @@ void resolve_clone_flags(struct lxc_handler *handler)
                if (!lxc_requests_empty_network(handler))
                        handler->clone_flags |= CLONE_NEWNET;
        } else {
-               INFO("Inheriting a NET namespace.");
+               INFO("Inheriting a net namespace.");
        }
 
        if (handler->conf->inherit_ns_fd[LXC_NS_IPC] == -1)
                handler->clone_flags |= CLONE_NEWIPC;
        else
-               INFO("Inheriting an IPC namespace.");
+               INFO("Inheriting an ipc namespace.");
 
        if (handler->conf->inherit_ns_fd[LXC_NS_UTS] == -1)
                handler->clone_flags |= CLONE_NEWUTS;
        else
-               INFO("Inheriting a UTS namespace.");
+               INFO("Inheriting a uts namespace.");
+
+       if (handler->conf->inherit_ns_fd[LXC_NS_PID] == -1)
+               handler->clone_flags |= CLONE_NEWPID;
+       else
+               INFO("Inheriting pid namespace");
+
+       if (cgns_supported()) {
+               if (handler->conf->inherit_ns_fd[LXC_NS_CGROUP] == -1)
+                       handler->clone_flags |= CLONE_NEWCGROUP;
+               else
+                       INFO("Inheriting cgroup namespace");
+       } else if (handler->conf->inherit_ns_fd[LXC_NS_CGROUP] >= 0) {
+                       return -EINVAL;
+       }
+
+       return 0;
 }
 
 /* lxc_spawn() performs crucial setup tasks and clone()s the new process which
@@ -1267,7 +1302,11 @@ static int lxc_spawn(struct lxc_handler *handler)
                return -1;
        }
 
-       resolve_clone_flags(handler);
+       ret = resolve_clone_flags(handler);
+       if (ret < 0) {
+               lxc_sync_fini(handler);
+               return -1;
+       }
 
        if (handler->clone_flags & CLONE_NEWNET) {
                if (!lxc_list_empty(&handler->conf->network)) {
@@ -1332,6 +1371,7 @@ static int lxc_spawn(struct lxc_handler *handler)
                 */
                flags &= ~CLONE_NEWNET;
        }
+
        handler->pid = lxc_clone(do_start, handler, flags);
        if (handler->pid < 0) {
                SYSERROR("Failed to clone a new set of namespaces.");
@@ -1457,6 +1497,17 @@ 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;
 

diff --git a/src/lxc/start.h b/src/lxc/start.h
index 28f9817bf2f15ab6d1e46f6c4d338009d3f65a51..e35e61f7e66f6e36457b79df811e3d743aeaf178 100644 (file)
--- a/src/lxc/start.h
+++ b/src/lxc/start.h
@@ -142,6 +142,6 @@ extern int lxc_check_inherited(struct lxc_conf *conf, bool closeall,
 extern int __lxc_start(const char *, struct lxc_handler *,
                       struct lxc_operations *, void *, const char *, bool);
 
-extern void resolve_clone_flags(struct lxc_handler *handler);
+extern int resolve_clone_flags(struct lxc_handler *handler);
 #endif
 

diff --git a/src/tests/shortlived.c b/src/tests/shortlived.c
index a7a6018277c785e050b07990935df1dccb907286..74a96f1661c937cd7dbd3d3d882bac28f274b516 100644 (file)
--- a/src/tests/shortlived.c
+++ b/src/tests/shortlived.c
@@ -99,35 +99,12 @@ int main(int argc, char *argv[])
        const char *s;
        bool b;
        struct lxc_container *c;
-       struct lxc_log log;
-       char template[sizeof(P_tmpdir"/shortlived_XXXXXX")];
-       int ret = 0;
+       int ret = EXIT_FAILURE;
 
-       strcpy(template, P_tmpdir"/shortlived_XXXXXX");
-       i = lxc_make_tmpfile(template, false);
-       if (i < 0) {
-               lxc_error("Failed to create temporary log file for container %s\n", MYNAME);
-               exit(EXIT_FAILURE);
-       } else {
-               lxc_debug("Using \"%s\" as temporary log file for container %s\n", template, MYNAME);
-               close(i);
-       }
-
-       log.name = MYNAME;
-       log.file = template;
-       log.level = "TRACE";
-       log.prefix = "shortlived";
-       log.quiet = false;
-       log.lxcpath = NULL;
-       if (lxc_log_init(&log))
-               exit(EXIT_FAILURE);
-
-       ret = 1;
        /* test a real container */
        c = lxc_container_new(MYNAME, NULL);
        if (!c) {
                fprintf(stderr, "%d: error creating lxc_container %s\n", __LINE__, MYNAME);
-               ret = 1;
                goto out;
        }
 
@@ -136,8 +113,7 @@ int main(int argc, char *argv[])
                goto out;
        }
 
-       ret = create_container();
-       if (ret) {
+       if (create_container() < 0) {
                fprintf(stderr, "%d: failed to create a container\n", __LINE__);
                goto out;
        }
@@ -174,13 +150,6 @@ int main(int argc, char *argv[])
                        goto out;
                }
 
-               /* The container needs to exit with a successful error code. */
-               if (c->error_num != 0) {
-                       fprintf(stderr, "%d: %s exited successfully on %dth iteration\n", __LINE__, c->name, i);
-                       goto out;
-               }
-               fprintf(stderr, "%d: %s exited correctly with error code %d on %dth iteration\n", __LINE__, c->name, c->error_num, i);
-
                if (!c->wait(c, "STOPPED", 30)) {
                        fprintf(stderr, "%d: %s failed to wait on %dth iteration\n", __LINE__, c->name, i);
                        goto out;
@@ -194,13 +163,6 @@ int main(int argc, char *argv[])
                        goto out;
                }
 
-               /* The container needs to exit with a successful error code. */
-               if (c->error_num != 0) {
-                       fprintf(stderr, "%d: %s exited successfully on %dth iteration\n", __LINE__, c->name, i);
-                       goto out;
-               }
-               fprintf(stderr, "%d: %s exited correctly with error code %d on %dth iteration\n", __LINE__, c->name, c->error_num, i);
-
                if (!c->wait(c, "STOPPED", 30)) {
                        fprintf(stderr, "%d: %s failed to wait on %dth iteration\n", __LINE__, c->name, i);
                        goto out;
@@ -219,13 +181,6 @@ int main(int argc, char *argv[])
                        goto out;
                }
 
-               /* The container needs to exit with an error code.*/
-               if (c->error_num == 0) {
-                       fprintf(stderr, "%d: %s exited successfully on %dth iteration\n", __LINE__, c->name, i);
-                       goto out;
-               }
-               fprintf(stderr, "%d: %s exited correctly with error code %d on %dth iteration\n", __LINE__, c->name, c->error_num, i);
-
                if (!c->wait(c, "STOPPED", 30)) {
                        fprintf(stderr, "%d: %s failed to wait on %dth iteration\n", __LINE__, c->name, i);
                        goto out;
@@ -242,13 +197,6 @@ int main(int argc, char *argv[])
                        goto out;
                }
 
-               /* The container needs to exit with an error code.*/
-               if (c->error_num == 0) {
-                       fprintf(stderr, "%d: %s exited successfully on %dth iteration\n", __LINE__, c->name, i);
-                       goto out;
-               }
-               fprintf(stderr, "%d: %s exited correctly with error code %d on %dth iteration\n", __LINE__, c->name, c->error_num, i);
-
                if (!c->wait(c, "STOPPED", 30)) {
                        fprintf(stderr, "%d: %s failed to wait on %dth iteration\n", __LINE__, c->name, i);
                        goto out;
@@ -258,7 +206,7 @@ int main(int argc, char *argv[])
        c->stop(c);
 
        fprintf(stderr, "all lxc_container tests passed for %s\n", c->name);
-       ret = 0;
+       ret = EXIT_SUCCESS;
 
 out:
        if (c) {
@@ -266,6 +214,5 @@ out:
                destroy_container();
        }
        lxc_container_put(c);
-       unlink(template);
        exit(ret);
 }