[lxc-devel] [lxc/master] conf: fix tty creation

brauner on Github lxc-bot at linuxcontainers.org
Tue Sep 5 10:47:05 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1224 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170905/aa66b4e6/attachment.bin>
-------------- next part --------------
From 73363c6134c61867ab304a35b233fd5b4d3bddc2 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 5 Sep 2017 12:19:28 +0200
Subject: [PATCH 1/2] conf: non-functional changes

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/conf.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index ea166c1de..613103827 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -854,23 +854,19 @@ static int lxc_setup_tty(struct lxc_conf *conf)
 		struct lxc_pty_info *pty_info = &tty_info->pty_info[i];
 
 		ret = snprintf(path, sizeof(path), "/dev/tty%d", i + 1);
-		if (ret < 0 || (size_t)ret >= sizeof(path)) {
-			ERROR("pathname too long for ttys");
+		if (ret < 0 || (size_t)ret >= sizeof(path))
 			return -1;
-		}
 
 		if (ttydir) {
 			/* create dev/lxc/tty%d" */
 			ret = snprintf(lxcpath, sizeof(lxcpath),
 				       "/dev/%s/tty%d", ttydir, i + 1);
-			if (ret < 0 || (size_t)ret >= sizeof(lxcpath)) {
-				ERROR("pathname too long for ttys");
+			if (ret < 0 || (size_t)ret >= sizeof(lxcpath))
 				return -1;
-			}
 
 			ret = creat(lxcpath, 0660);
 			if (ret < 0 && errno != EEXIST) {
-				SYSERROR("failed to create \"%s\"", lxcpath);
+				SYSERROR("Failed to create \"%s\"", lxcpath);
 				return -1;
 			}
 			if (ret >= 0)
@@ -878,13 +874,13 @@ static int lxc_setup_tty(struct lxc_conf *conf)
 
 			ret = unlink(path);
 			if (ret < 0 && errno != ENOENT) {
-				SYSERROR("failed to unlink \"%s\"", path);
+				SYSERROR("Failed to unlink \"%s\"", path);
 				return -1;
 			}
 
 			ret = mount(pty_info->name, lxcpath, "none", MS_BIND, 0);
 			if (ret < 0) {
-				WARN("failed to bind mount \"%s\" onto \"%s\"",
+				WARN("Failed to bind mount \"%s\" onto \"%s\"",
 				     pty_info->name, path);
 				continue;
 			}
@@ -893,14 +889,12 @@ static int lxc_setup_tty(struct lxc_conf *conf)
 
 			ret = snprintf(lxcpath, sizeof(lxcpath), "%s/tty%d",
 				       ttydir, i + 1);
-			if (ret < 0 || (size_t)ret >= sizeof(lxcpath)) {
-				ERROR("tty pathname too long");
+			if (ret < 0 || (size_t)ret >= sizeof(lxcpath))
 				return -1;
-			}
 
 			ret = symlink(lxcpath, path);
 			if (ret < 0) {
-				SYSERROR("failed to create symlink \"%s\" -> \"%s\"",
+				SYSERROR("Failed to create symlink \"%s\" -> \"%s\"",
 				         path, lxcpath);
 				return -1;
 			}
@@ -912,7 +906,7 @@ static int lxc_setup_tty(struct lxc_conf *conf)
 			if (ret < 0) {
 				ret = creat(path, 0660);
 				if (ret < 0) {
-					SYSERROR("failed to create \"%s\"", path);
+					SYSERROR("Failed to create \"%s\"", path);
 					/* this isn't fatal, continue */
 				} else {
 					close(ret);
@@ -921,11 +915,11 @@ static int lxc_setup_tty(struct lxc_conf *conf)
 
 			ret = mount(pty_info->name, path, "none", MS_BIND, 0);
 			if (ret < 0) {
-				SYSERROR("failed to mount '%s'->'%s'", pty_info->name, path);
+				SYSERROR("Failed to mount '%s'->'%s'", pty_info->name, path);
 				continue;
 			}
 
-			DEBUG("bind mounted \"%s\" onto \"%s\"", pty_info->name,
+			DEBUG("Bind mounted \"%s\" onto \"%s\"", pty_info->name,
 			      path);
 		}
 
@@ -935,7 +929,7 @@ static int lxc_setup_tty(struct lxc_conf *conf)
 		}
 	}
 
-	INFO("finished setting up %d /dev/tty<N> device(s)", tty_info->nbtty);
+	INFO("Finished setting up %d /dev/tty<N> device(s)", tty_info->nbtty);
 	return 0;
 }
 

From 2187efd31051513ef0758b6eaa336894e69039f9 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 5 Sep 2017 12:41:30 +0200
Subject: [PATCH 2/2] 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 at ubuntu.com>
---
 src/lxc/conf.c | 278 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 149 insertions(+), 129 deletions(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 613103827..bede03574 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -817,9 +817,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;
@@ -840,7 +838,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;
@@ -933,6 +931,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;
@@ -2664,77 +2806,6 @@ int find_unmapped_nsid(struct lxc_conf *conf, enum idtype idtype)
 	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);
@@ -3063,45 +3134,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;
@@ -3212,24 +3247,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");


More information about the lxc-devel mailing list