Fix race/corruption with multiple lxc-start, lxc-execute

If you start more than one lxc-start/lxc-execute with the same name at the
same time, or just do an lxc-start/lxc-execute with the name of a container
that is already running, lxc doesn't figure out that the container with this
name is already running until fairly late in the initialization process: ie
when __lxc_start() -> lxc_poll() -> lxc_command_mainloop_add() attempts to
create the same abstract socket name.

By this point a fair amount of initialization has been done that actually
messes up the running container. For example __lxc_start() -> lxc_spawn() ->
lxc_cgroup_create() -> lxc_one_cgroup_create() -> try_to_move_cgname() moves
the running container's cgroup to a name of deadXXXXXX.

The solution in this patch is to use the atomic existence of the abstract
socket name as the indicator that the container is already running.  To do
so, I just refactored lxc_command_mainloop_add() into an lxc_command_init()
routine that attempts to bind the socket, and ensure this is called earlier
before much initialization has been done.

In testing, I verified that maincmd_fd was still open at the time of lxc_fini,
so the entire lifetime of the container's run should be covered. The only
explicit close of this fd was in the reboot case of lxcapi_start(), which is
now moved to lxc_fini(), which I think is more appropriate.

Even though it is not checked any more, set maincmd_fd to -1 instead of 0 to
indicate its not open since 0 could be a valid fd.

Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>

diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 737540d3dac98025499d18efe08e5ed6c0b9f7bc..40c685e96e02e0930a9d8bb7069b4e0f6d5cc29a 100644 (file)
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -287,11 +287,9 @@ out_close:
        goto out;
 }
 
-extern int lxc_command_mainloop_add(const char *name,
-                                   struct lxc_epoll_descr *descr,
-                                   struct lxc_handler *handler)
+extern int lxc_command_init(const char *name, struct lxc_handler *handler)
 {
-       int ret, fd;
+       int fd;
        char path[sizeof(((struct sockaddr_un *)0)->sun_path)] = { 0 };
        char *offset = &path[1];
        int rc, len;
@@ -320,6 +318,16 @@ extern int lxc_command_mainloop_add(const char *name,
                return -1;
        }
 
+       handler->conf->maincmd_fd = fd;
+       return 0;
+}
+
+extern int lxc_command_mainloop_add(const char *name,
+                                   struct lxc_epoll_descr *descr,
+                                   struct lxc_handler *handler)
+{
+       int ret, fd = handler->conf->maincmd_fd;
+
        ret = lxc_mainloop_add_handler(descr, fd, incoming_command_handler,
                                       handler);
        if (ret) {
@@ -327,6 +335,5 @@ extern int lxc_command_mainloop_add(const char *name,
                close(fd);
        }
 
-       handler->conf->maincmd_fd = fd;
        return ret;
 }

diff --git a/src/lxc/commands.h b/src/lxc/commands.h
index 3b0ac9a657f230d7284101e36a0a18a114cee83b..0e1c8f96c40e31c82c30f1e120fc4a133b768343 100644 (file)
--- a/src/lxc/commands.h
+++ b/src/lxc/commands.h
@@ -60,6 +60,7 @@ extern int lxc_command_connected(const char *name, struct lxc_command *command,
 struct lxc_epoll_descr;
 struct lxc_handler;
 
+extern int lxc_command_init(const char *name, struct lxc_handler *handler);
 extern int lxc_command_mainloop_add(const char *name, struct lxc_epoll_descr *descr,
                                    struct lxc_handler *handler);
 

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 84e51db1b84a5b86690d84f81314398861329c24..65f18c82fda1bbfd8597a9b35f4470f734a18ad3 100644 (file)
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -1895,6 +1895,7 @@ struct lxc_conf *lxc_conf_init(void)
        new->console.master = -1;
        new->console.slave = -1;
        new->console.name[0] = '\0';
+       new->maincmd_fd = -1;
        new->rootfs.mount = default_rootfs_mount;
        new->loglevel = LXC_LOG_PRIORITY_NOTSET;
        lxc_list_init(&new->cgroup);

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 1345ab55b719a83bdfa18c140c31773469d2bd9d..fd77aac70dc4c2a70cf5dc0cdd25d37a67df0c2e 100644 (file)
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -381,9 +381,6 @@ reboot:
        if (conf->reboot) {
                INFO("container requested reboot");
                conf->reboot = 0;
-               if (conf->maincmd_fd)
-                       close(conf->maincmd_fd);
-               conf->maincmd_fd = 0;
                goto reboot;
        }
 

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 7320d74a8665f09638f308027f570c47ed617642..05271fa56efb48e93239dd83de4d59e41def6ac3 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -377,9 +377,12 @@ struct lxc_handler *lxc_init(const char *name, struct lxc_conf *conf)
                goto out_free;
        }
 
+       if (lxc_command_init(name, handler))
+               goto out_free_name;
+
        if (lxc_read_seccomp_config(conf) != 0) {
                ERROR("failed loading seccomp policy");
-               goto out_free_name;
+               goto out_close_maincmd_fd;
        }
 
        /* Begin the set the state to STARTING*/
@@ -421,6 +424,9 @@ out_delete_tty:
        lxc_delete_tty(&conf->tty_info);
 out_aborting:
        lxc_set_state(name, handler, ABORTING);
+out_close_maincmd_fd:
+       close(conf->maincmd_fd);
+       conf->maincmd_fd = -1;
 out_free_name:
        free(handler->name);
        handler->name = NULL;
@@ -446,6 +452,8 @@ void lxc_fini(const char *name, struct lxc_handler *handler)
 
        lxc_delete_console(&handler->conf->console);
        lxc_delete_tty(&handler->conf->tty_info);
+       close(handler->conf->maincmd_fd);
+       handler->conf->maincmd_fd = -1;
        free(handler->name);
        free(handler);
 }