[lxc-devel] [lxc/stable-1.0] stable-1.0: console backend bugfixes

brauner on Github lxc-bot at linuxcontainers.org
Wed Nov 29 02:50:12 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 404 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20171129/f39d0d91/attachment.bin>
-------------- next part --------------
From b5d5506d6029c23208d8746d7b566271a0d0ade3 Mon Sep 17 00:00:00 2001
From: Li Feng <lifeng68 at huawei.com>
Date: Fri, 19 May 2017 22:40:07 +0800
Subject: [PATCH 1/6] Fix the bug of 'ts->stdoutfd' did not fill with
 parameters 'stdoutfd'

Signed-off-by: Li Feng <lifeng68 at huawei.com>
---
 src/lxc/console.c | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 src/lxc/console.c

diff --git a/src/lxc/console.c b/src/lxc/console.c
old mode 100644
new mode 100755

From d1f34018c1a5ac03e3be0631cfc62cbd7745e13d Mon Sep 17 00:00:00 2001
From: Li Feng <lifeng68 at huawei.com>
Date: Sat, 20 May 2017 17:40:36 +0800
Subject: [PATCH 2/6] DO NOT add the handles of adjust winsize when the 'stdin'
 is not a tty

Signed-off-by: Li Feng <lifeng68 at huawei.com>
---
 src/lxc/console.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/src/lxc/console.c b/src/lxc/console.c
index dcbe02edd..68c7d1a3b 100755
--- a/src/lxc/console.c
+++ b/src/lxc/console.c
@@ -121,6 +121,11 @@ struct lxc_tty_state *lxc_console_sigwinch_init(int srcfd, int dstfd)
 	ts->masterfd = dstfd;
 	ts->sigfd = -1;
 
+	if (!isatty(srcfd)) {
+		INFO("fd %d does not refer to a tty device", srcfd);
+		return ts;
+	}
+
 	/* add tty to list to be scanned at SIGWINCH time */
 	lxc_list_add_elem(&ts->node, ts);
 	lxc_list_add_tail(&lxc_ttys, &ts->node);
@@ -128,20 +133,20 @@ struct lxc_tty_state *lxc_console_sigwinch_init(int srcfd, int dstfd)
 	sigemptyset(&mask);
 	sigaddset(&mask, SIGWINCH);
 	if (sigprocmask(SIG_BLOCK, &mask, &ts->oldmask)) {
-		SYSERROR("failed to block SIGWINCH.");
+		SYSERROR("failed to block SIGWINCH");
 		ts->sigfd = -1;
 		return ts;
 	}
 
 	ts->sigfd = signalfd(-1, &mask, 0);
 	if (ts->sigfd < 0) {
-		SYSERROR("failed to get signalfd.");
+		SYSERROR("failed to create signal fd");
 		sigprocmask(SIG_SETMASK, &ts->oldmask, NULL);
 		ts->sigfd = -1;
 		return ts;
 	}
 
-	DEBUG("%d got SIGWINCH fd %d", getpid(), ts->sigfd);
+	DEBUG("process %d created signal fd %d to handle SIGWINCH events", getpid(), ts->sigfd);
 	return ts;
 }
 
@@ -647,16 +652,17 @@ int lxc_console(struct lxc_container *c, int ttynum,
 	struct lxc_epoll_descr descr;
 	struct termios oldtios;
 	struct lxc_tty_state *ts;
+	int istty = 0;
 
-	if (!isatty(stdinfd)) {
-		ERROR("stdin is not a tty");
-		return -1;
-	}
-
-	ret = lxc_setup_tios(stdinfd, &oldtios);
-	if (ret) {
-		ERROR("failed to setup tios");
-		return -1;
+	istty = isatty(stdinfd);
+	if (istty) {
+		ret = lxc_setup_tios(stdinfd, &oldtios);
+		if (ret) {
+			ERROR("failed to setup terminal properties");
+			return -1;
+		}
+	} else {
+		INFO("fd %d does not refer to a tty device", stdinfd);
 	}
 
 	ttyfd = lxc_cmd_console(c->name, &ttynum, &masterfd, c->config_path);
@@ -678,8 +684,10 @@ int lxc_console(struct lxc_container *c, int ttynum,
 	ts->winch_proxy_lxcpath = c->config_path;
 	ts->stdoutfd = stdoutfd;
 
-	lxc_console_winsz(stdinfd, masterfd);
-	lxc_cmd_console_winch(ts->winch_proxy, ts->winch_proxy_lxcpath);
+	if (istty) {
+		lxc_console_winsz(stdinfd, masterfd);
+		lxc_cmd_console_winch(ts->winch_proxy, ts->winch_proxy_lxcpath);
+	}
 
 	ret = lxc_mainloop_open(&descr);
 	if (ret) {
@@ -736,8 +744,10 @@ int lxc_console(struct lxc_container *c, int ttynum,
 	close(masterfd);
 	close(ttyfd);
 err1:
-	tcsetattr(stdinfd, TCSAFLUSH, &oldtios);
+	if (istty) {
+		if (tcsetattr(stdinfd, TCSAFLUSH, &oldtios) < 0)
+			WARN("failed to reset terminal properties: %s.", strerror(errno));
+	}
 
 	return ret;
 }
-

From 3b6d5e21aa0b880179f51cc830120dbb735c1d8f Mon Sep 17 00:00:00 2001
From: Li Feng <lifeng68 at huawei.com>
Date: Wed, 21 Jun 2017 13:38:06 +0800
Subject: [PATCH 3/6] Fix memory leak of 'lxc_tty_state'

Signed-off-by: Li Feng <lifeng68 at huawei.com>
---
 src/lxc/console.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/lxc/console.c b/src/lxc/console.c
index 68c7d1a3b..1b86be1aa 100755
--- a/src/lxc/console.c
+++ b/src/lxc/console.c
@@ -135,6 +135,7 @@ struct lxc_tty_state *lxc_console_sigwinch_init(int srcfd, int dstfd)
 	if (sigprocmask(SIG_BLOCK, &mask, &ts->oldmask)) {
 		SYSERROR("failed to block SIGWINCH");
 		ts->sigfd = -1;
+		lxc_list_del(&ts->node);
 		return ts;
 	}
 
@@ -143,6 +144,7 @@ struct lxc_tty_state *lxc_console_sigwinch_init(int srcfd, int dstfd)
 		SYSERROR("failed to create signal fd");
 		sigprocmask(SIG_SETMASK, &ts->oldmask, NULL);
 		ts->sigfd = -1;
+		lxc_list_del(&ts->node);
 		return ts;
 	}
 
@@ -152,11 +154,12 @@ struct lxc_tty_state *lxc_console_sigwinch_init(int srcfd, int dstfd)
 
 void lxc_console_sigwinch_fini(struct lxc_tty_state *ts)
 {
-	if (ts->sigfd >= 0)
+	if (ts->sigfd >= 0) {
 		close(ts->sigfd);
+		lxc_list_del(&ts->node);
+		sigprocmask(SIG_SETMASK, &ts->oldmask, NULL);
+	}
 
-	lxc_list_del(&ts->node);
-	sigprocmask(SIG_SETMASK, &ts->oldmask, NULL);
 	free(ts);
 }
 
@@ -297,7 +300,7 @@ int lxc_setup_tios(int fd, struct termios *oldtios)
 
 static void lxc_console_peer_proxy_free(struct lxc_console *console)
 {
-	if (console->tty_state && console->tty_state->sigfd != -1) {
+	if (console->tty_state) {
 		lxc_console_sigwinch_fini(console->tty_state);
 		console->tty_state = NULL;
 	}
@@ -738,8 +741,7 @@ int lxc_console(struct lxc_container *c, int ttynum,
 err4:
 	lxc_mainloop_close(&descr);
 err3:
-	if (ts->sigfd != -1)
-		lxc_console_sigwinch_fini(ts);
+	lxc_console_sigwinch_fini(ts);
 err2:
 	close(masterfd);
 	close(ttyfd);

From e66fbeb8929246317f4caae0154a9ad965ee0b34 Mon Sep 17 00:00:00 2001
From: Li Feng <lifeng68 at huawei.com>
Date: Mon, 10 Jul 2017 17:19:52 +0800
Subject: [PATCH 4/6] start: dup std{in,out,err} to pty slave

In the case the container has a console with a valid slave pty file descriptor
we duplicate std{in,out,err} to the slave file descriptor so console logging
works correctly. When the container does not have a valid slave pty file
descriptor for its console and is started daemonized we should dup to
/dev/null.

Closes #1646.
Closes #1951.

Signed-off-by: Li Feng <lifeng68 at huawei.com>
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/start.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 664dbc3ba..736bd28d5 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -751,8 +751,13 @@ static int do_start(void *data)
 	 * setup on its console ie. the pty allocated in lxc_console_create()
 	 * so make sure that that pty is stdin,stdout,stderr.
 	 */
-	if (lxc_console_set_stdfds(handler->conf->console.slave) < 0)
-		goto out_warn_father;
+	 if (handler->conf->console.slave >= 0)
+		 if (set_stdfds(handler->conf->console.slave) < 0) {
+			ERROR("Failed to redirect std{in,out,err} to pty file "
+			      "descriptor %d",
+			      handler->conf->console.slave);
+			goto out_warn_father;
+		 }
 
 	/* If we mounted a temporary proc, then unmount it now */
 	tmp_proc_unmount(handler->conf);
@@ -782,8 +787,30 @@ static int do_start(void *data)
 
 	close(handler->sigfd);
 
-	/* after this call, we are in error because this
-	 * ops should not return as it execs */
+	if (devnull_fd < 0) {
+		devnull_fd = open_devnull();
+
+		if (devnull_fd < 0)
+			goto out_warn_father;
+	}
+
+	if (handler->conf->console.slave < 0 && handler->backgrounded)
+		if (set_stdfds(devnull_fd) < 0) {
+			ERROR("Failed to redirect std{in,out,err} to "
+			      "\"/dev/null\"");
+			goto out_warn_father;
+		}
+
+	if (devnull_fd >= 0) {
+		close(devnull_fd);
+		devnull_fd = -1;
+	}
+
+	setsid();
+
+	/* After this call, we are in error because this ops should not return
+	 * as it execs.
+	 */
 	handler->ops->start(handler, handler->data);
 
 out_warn_father:

From 4ed3c95f6854c1a1940a74ca10656ab8164764a2 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 29 Nov 2017 03:46:12 +0100
Subject: [PATCH 5/6] utils: backport open_devnull() and set_stdfds()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/utils.c | 32 ++++++++++++++++++++++++++++++++
 src/lxc/utils.h |  2 ++
 2 files changed, 34 insertions(+)

diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index f8239874d..b79193acd 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1365,3 +1365,35 @@ int safe_mount(const char *src, const char *dest, const char *fstype,
 
 	return 0;
 }
+
+int open_devnull(void)
+{
+	int fd = open("/dev/null", O_RDWR);
+
+	if (fd < 0)
+		SYSERROR("Can't open /dev/null");
+
+	return fd;
+}
+
+int set_stdfds(int fd)
+{
+	int ret;
+
+	if (fd < 0)
+		return -1;
+
+	ret = dup2(fd, STDIN_FILENO);
+	if (ret < 0)
+		return -1;
+
+	ret = dup2(fd, STDOUT_FILENO);
+	if (ret < 0)
+		return -1;
+
+	ret = dup2(fd, STDERR_FILENO);
+	if (ret < 0)
+		return -1;
+
+	return 0;
+}
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index bf62bd060..d1113e76a 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -294,4 +294,6 @@ int null_stdfds(void);
 
 int safe_mount(const char *src, const char *dest, const char *fstype,
 		unsigned long flags, const void *data, const char *rootfs);
+int open_devnull(void);
+int set_stdfds(int fd);
 #endif /* __LXC_UTILS_H */

From d3fdea6247ad91bc0f8486a8d876b52112be81c1 Mon Sep 17 00:00:00 2001
From: LiFeng <lifeng68 at huawei.com>
Date: Tue, 5 Sep 2017 23:16:50 +0800
Subject: [PATCH 6/6] console: clean tty state + return 0 on peer exit

In the past, if the console client exited, lxc_console_cb_con return 1. And
the lxc_poll will exit, the process will wait at waitpid. At this moment, the
process could not handle any command (For example get the container state
LXC_CMD_GET_STATE or stop the container LXC_CMD_STOP.).

I think we should clean the tty_state and return 0 in this case. So, we can use
the lxc-console to connect the console of the container. And we will not exit
the function lxc_polland we can handle the commands by lxc_cmd_process

Reproducer prior to this commit:
- open a new terminal, get the tty device name by command tty /dev/pts/6
- set lxc.console.path = /dev/pts/6
- start the container and the ouptut will print to /dev/pts/6
- close /dev/pts/6
- try an operation e.g. getting state with lxc-ls and lxc-ls will hang

Closes #1787.

Signed-off-by: LiFeng <lifeng68 at huawei.com>
Acked-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/console.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/lxc/console.c b/src/lxc/console.c
index 1b86be1aa..f77586ca3 100755
--- a/src/lxc/console.c
+++ b/src/lxc/console.c
@@ -174,6 +174,15 @@ static int lxc_console_cb_con(int fd, uint32_t events, void *data,
 	if (r <= 0) {
 		INFO("console client on fd %d has exited", fd);
 		lxc_mainloop_del_handler(descr, fd);
+		if (fd == console->peer) {
+			if (console->tty_state) {
+				lxc_console_sigwinch_fini(console->tty_state);
+				console->tty_state = NULL;
+			}
+			console->peer = -1;
+			close(fd);
+			return 0;
+		}
 		close(fd);
 		return 1;
 	}


More information about the lxc-devel mailing list