conf: fix tty creation

We allocate pty {master,slave} file descriptors in the childs namespaces after
we have setup devpts. After we have sent the pty file descriptors to the parent
and set up the pty file descriptors under /dev/tty* and before we exec the init
binary we need to delete these file descriptors in the child. However, one of
my commits made the deletion occur before setting up the file descriptors under
/dev/tty*. This caused a failures when trying to attach to the container's ttys
since they werent actually configured although the file descriptors were
available in the in-memory configuration of the parent.
This commit reworks setting up tty such that deletion occurs after all setup
has been performed. The commit is actually minimal but needs to also move all
the functions into one place since they well now be called from
"lxc_create_ttys()".

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

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index fd74a66b311bae4de74b99f040c34d86e8a380c8..110d124369940897e7443011c814a5113ac0f035 100644 (file)
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -758,9 +758,7 @@ static int setup_dev_symlinks(const struct lxc_rootfs *rootfs)
        return 0;
 }
 
-/*
- * Build a space-separate list of ptys to pass to systemd.
- */
+/* Build a space-separate list of ptys to pass to systemd. */
 static bool append_ptyname(char **pp, char *name)
 {
        char *p;
@@ -781,7 +779,7 @@ static bool append_ptyname(char **pp, char *name)
        return true;
 }
 
-static int lxc_setup_tty(struct lxc_conf *conf)
+static int lxc_setup_ttys(struct lxc_conf *conf)
 {
        int i, ret;
        const struct lxc_tty_info *tty_info = &conf->tty_info;
@@ -874,6 +872,150 @@ static int lxc_setup_tty(struct lxc_conf *conf)
        return 0;
 }
 
+int lxc_allocate_ttys(const char *name, struct lxc_conf *conf)
+{
+       struct lxc_tty_info *tty_info = &conf->tty_info;
+       int i, ret;
+
+       /* no tty in the configuration */
+       if (!conf->tty)
+               return 0;
+
+       tty_info->pty_info = malloc(sizeof(*tty_info->pty_info) * conf->tty);
+       if (!tty_info->pty_info) {
+               SYSERROR("failed to allocate struct *pty_info");
+               return -ENOMEM;
+       }
+
+       for (i = 0; i < conf->tty; i++) {
+               struct lxc_pty_info *pty_info = &tty_info->pty_info[i];
+
+               process_lock();
+               ret = openpty(&pty_info->master, &pty_info->slave,
+                             pty_info->name, NULL, NULL);
+               process_unlock();
+               if (ret) {
+                       SYSERROR("failed to create pty device number %d", i);
+                       tty_info->nbtty = i;
+                       lxc_delete_tty(tty_info);
+                       return -ENOTTY;
+               }
+
+               DEBUG("allocated pty \"%s\" with master fd %d and slave fd %d",
+                     pty_info->name, pty_info->master, pty_info->slave);
+
+               /* Prevent leaking the file descriptors to the container */
+               ret = fcntl(pty_info->master, F_SETFD, FD_CLOEXEC);
+               if (ret < 0)
+                       WARN("failed to set FD_CLOEXEC flag on master fd %d of "
+                            "pty device \"%s\": %s",
+                            pty_info->master, pty_info->name, strerror(errno));
+
+               ret = fcntl(pty_info->slave, F_SETFD, FD_CLOEXEC);
+               if (ret < 0)
+                       WARN("failed to set FD_CLOEXEC flag on slave fd %d of "
+                            "pty device \"%s\": %s",
+                            pty_info->slave, pty_info->name, strerror(errno));
+
+               pty_info->busy = 0;
+       }
+
+       tty_info->nbtty = conf->tty;
+
+       INFO("finished allocating %d pts devices", conf->tty);
+       return 0;
+}
+
+void lxc_delete_tty(struct lxc_tty_info *tty_info)
+{
+       int i;
+
+       for (i = 0; i < tty_info->nbtty; i++) {
+               struct lxc_pty_info *pty_info = &tty_info->pty_info[i];
+
+               close(pty_info->master);
+               close(pty_info->slave);
+       }
+
+       free(tty_info->pty_info);
+       tty_info->pty_info = NULL;
+       tty_info->nbtty = 0;
+}
+
+static int lxc_send_ttys_to_parent(struct lxc_handler *handler)
+{
+       int i;
+       struct lxc_conf *conf = handler->conf;
+       struct lxc_tty_info *tty_info = &conf->tty_info;
+       int sock = handler->data_sock[0];
+       int ret = -1;
+
+       if (!conf->tty)
+               return 0;
+
+       for (i = 0; i < conf->tty; i++) {
+               int ttyfds[2];
+               struct lxc_pty_info *pty_info = &tty_info->pty_info[i];
+
+               ttyfds[0] = pty_info->master;
+               ttyfds[1] = pty_info->slave;
+
+               ret = lxc_abstract_unix_send_fds(sock, ttyfds, 2, NULL, 0);
+               if (ret < 0)
+                       break;
+
+               TRACE("Send pty \"%s\" with master fd %d and slave fd %d to "
+                     "parent", pty_info->name, pty_info->master, pty_info->slave);
+       }
+
+       if (ret < 0)
+               ERROR("Failed to send %d ttys to parent: %s", conf->tty,
+                     strerror(errno));
+       else
+               TRACE("Sent %d ttys to parent", conf->tty);
+
+       return ret;
+}
+
+static int lxc_create_ttys(struct lxc_handler *handler)
+{
+       int ret = -1;
+       struct lxc_conf *conf = handler->conf;
+
+       ret = lxc_allocate_ttys(handler->name, conf);
+       if (ret < 0) {
+               ERROR("Failed to allocate ttys");
+               goto on_error;
+       }
+
+       ret = lxc_send_ttys_to_parent(handler);
+       if (ret < 0) {
+               ERROR("Failed to send ttys to parent");
+               goto on_error;
+       }
+
+       if (!conf->is_execute) {
+               ret = lxc_setup_ttys(conf);
+               if (ret < 0) {
+                       ERROR("Failed to setup ttys");
+                       goto on_error;
+               }
+       }
+
+       if (conf->pty_names) {
+               ret = setenv("container_ttys", conf->pty_names, 1);
+               if (ret < 0)
+                       SYSERROR("Failed to set \"container_ttys=%s\"", conf->pty_names);
+       }
+
+       ret = 0;
+
+on_error:
+       lxc_delete_tty(&conf->tty_info);
+
+       return ret;
+}
+
 static int setup_rootfs_pivot_root(const char *rootfs)
 {
        int oldroot = -1, newroot = -1;
@@ -2584,77 +2726,6 @@ again:
        return freeid;
 }
 
-int lxc_create_tty(const char *name, struct lxc_conf *conf)
-{
-       struct lxc_tty_info *tty_info = &conf->tty_info;
-       int i, ret;
-
-       /* no tty in the configuration */
-       if (!conf->tty)
-               return 0;
-
-       tty_info->pty_info = malloc(sizeof(*tty_info->pty_info) * conf->tty);
-       if (!tty_info->pty_info) {
-               SYSERROR("failed to allocate struct *pty_info");
-               return -ENOMEM;
-       }
-
-       for (i = 0; i < conf->tty; i++) {
-               struct lxc_pty_info *pty_info = &tty_info->pty_info[i];
-
-               process_lock();
-               ret = openpty(&pty_info->master, &pty_info->slave,
-                             pty_info->name, NULL, NULL);
-               process_unlock();
-               if (ret) {
-                       SYSERROR("failed to create pty device number %d", i);
-                       tty_info->nbtty = i;
-                       lxc_delete_tty(tty_info);
-                       return -ENOTTY;
-               }
-
-               DEBUG("allocated pty \"%s\" with master fd %d and slave fd %d",
-                     pty_info->name, pty_info->master, pty_info->slave);
-
-               /* Prevent leaking the file descriptors to the container */
-               ret = fcntl(pty_info->master, F_SETFD, FD_CLOEXEC);
-               if (ret < 0)
-                       WARN("failed to set FD_CLOEXEC flag on master fd %d of "
-                            "pty device \"%s\": %s",
-                            pty_info->master, pty_info->name, strerror(errno));
-
-               ret = fcntl(pty_info->slave, F_SETFD, FD_CLOEXEC);
-               if (ret < 0)
-                       WARN("failed to set FD_CLOEXEC flag on slave fd %d of "
-                            "pty device \"%s\": %s",
-                            pty_info->slave, pty_info->name, strerror(errno));
-
-               pty_info->busy = 0;
-       }
-
-       tty_info->nbtty = conf->tty;
-
-       INFO("finished allocating %d pts devices", conf->tty);
-       return 0;
-}
-
-void lxc_delete_tty(struct lxc_tty_info *tty_info)
-{
-       int i;
-
-       for (i = 0; i < tty_info->nbtty; i++) {
-               struct lxc_pty_info *pty_info = &tty_info->pty_info[i];
-
-               close(pty_info->master);
-               close(pty_info->slave);
-       }
-
-       free(tty_info->pty_info);
-       tty_info->pty_info = NULL;
-       tty_info->nbtty = 0;
-}
-
-
 int chown_mapped_root_exec_wrapper(void *args)
 {
        execvp("lxc-usernsexec", args);
@@ -3001,45 +3072,9 @@ static bool verify_start_hooks(struct lxc_conf *conf)
        return true;
 }
 
-static int lxc_send_ttys_to_parent(struct lxc_handler *handler)
-{
-       int i;
-       struct lxc_conf *conf = handler->conf;
-       struct lxc_tty_info *tty_info = &conf->tty_info;
-       int sock = handler->data_sock[0];
-       int ret = -1;
-
-       if (!conf->tty)
-               return 0;
-
-       for (i = 0; i < conf->tty; i++) {
-               int ttyfds[2];
-               struct lxc_pty_info *pty_info = &tty_info->pty_info[i];
-
-               ttyfds[0] = pty_info->master;
-               ttyfds[1] = pty_info->slave;
-
-               ret = lxc_abstract_unix_send_fds(sock, ttyfds, 2, NULL, 0);
-               if (ret < 0)
-                       break;
-
-               TRACE("Send pty \"%s\" with master fd %d and slave fd %d to "
-                     "parent", pty_info->name, pty_info->master, pty_info->slave);
-       }
-
-       if (ret < 0)
-               ERROR("Failed to send %d ttys to parent: %s", conf->tty,
-                     strerror(errno));
-       else
-               TRACE("Sent %d ttys to parent", conf->tty);
-
-       lxc_delete_tty(tty_info);
-
-       return ret;
-}
-
 int lxc_setup(struct lxc_handler *handler)
 {
+       int ret;
        const char *name = handler->name;
        struct lxc_conf *lxc_conf = handler->conf;
        const char *lxcpath = handler->lxcpath;
@@ -3155,24 +3190,9 @@ int lxc_setup(struct lxc_handler *handler)
                return -1;
        }
 
-       if (lxc_create_tty(name, lxc_conf)) {
-               ERROR("failed to create the ttys");
-               return -1;
-       }
-
-       if (lxc_send_ttys_to_parent(handler) < 0) {
-               ERROR("failure sending console info to parent");
-               return -1;
-       }
-
-       if (!lxc_conf->is_execute && lxc_setup_tty(lxc_conf)) {
-               ERROR("failed to setup the ttys for '%s'", name);
+       ret = lxc_create_ttys(handler);
+       if (ret < 0)
                return -1;
-       }
-
-       if (lxc_conf->pty_names && setenv("container_ttys", lxc_conf->pty_names, 1))
-               SYSERROR("failed to set environment variable for container ptys");
-
 
        if (setup_personality(lxc_conf->personality)) {
                ERROR("failed to setup personality");