[lxc-devel] [PATCH v2] fix console stdin,stdout,stderr fds

Dwight Engen dwight.engen at oracle.com
Fri Mar 7 21:51:47 UTC 2014


The fds for stdin,stdout,stderr that we were leaving open for /sbin/init
in the container were those from /dev/tty or lxc.console (if given), which
wasn't right. Inside the container it should only have access to the pty
that lxc creates representing the console.

This was noticed because busybox's init was resetting the termio on its
stdin which was effecting the actual users terminal instead of the pty.
This meant it was setting icanon so were were not passing keystrokes
immediately to the pty, and hence command line history/editing wasn't
working.

Fix by dup'ing the console pty to stdin,stdout,stderr just before
exec()ing /sbin/init. Fix fd leak in error handling that I noticed while
going through this code.

Also tested with lxc.console = none, lxc.console = /dev/tty7 and no
lxc.console specified.

V2: The first version was getting EBADF sometimes on dup2() because
lxc_console_set_stdfds() was being called after lxc_check_inherited()
had already closed the fds for the pty. Fix by calling
lxc_check_inherited() as late as possible which also extends coverage
of open fd checked code.

Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
---
 src/lxc/console.c     | 18 +++++++++++++++++-
 src/lxc/console.h     |  1 +
 src/lxc/execute.c     |  2 +-
 src/lxc/lxc_console.c |  1 -
 src/lxc/lxc_start.c   |  1 -
 src/lxc/start.c       | 20 ++++++++++++++------
 src/lxc/start.h       |  2 +-
 7 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/src/lxc/console.c b/src/lxc/console.c
index 6bfc8a3..67e5d0f 100644
--- a/src/lxc/console.c
+++ b/src/lxc/console.c
@@ -506,7 +506,7 @@ static void lxc_console_peer_default(struct lxc_console *console)
 	DEBUG("using '%s' as console", path);
 
 	if (!isatty(console->peer))
-		return;
+		goto err1;
 
 	ts = lxc_console_sigwinch_init(console->peer, console->master);
 	if (!ts)
@@ -611,7 +611,23 @@ err:
 	return -1;
 }
 
+int lxc_console_set_stdfds(struct lxc_handler *handler)
+{
+	struct lxc_conf *conf = handler->conf;
+	struct lxc_console *console = &conf->console;
+
+	if (console->slave < 0)
+		return 0;
 
+	if (dup2(console->slave, 0) < 0 ||
+	    dup2(console->slave, 1) < 0 ||
+	    dup2(console->slave, 2) < 0)
+	{
+		SYSERROR("failed to dup console");
+		return -1;
+	}
+	return 0;
+}
 
 static int lxc_console_cb_tty_stdin(int fd, uint32_t events, void *cbdata,
 				    struct lxc_epoll_descr *descr)
diff --git a/src/lxc/console.h b/src/lxc/console.h
index d45260c..eb3894b 100644
--- a/src/lxc/console.h
+++ b/src/lxc/console.h
@@ -36,3 +36,4 @@ extern int  lxc_console(struct lxc_container *c, int ttynum,
 		        int escape);
 extern int  lxc_console_getfd(struct lxc_container *c, int *ttynum,
 			      int *masterfd);
+extern int  lxc_console_set_stdfds(struct lxc_handler *);
diff --git a/src/lxc/execute.c b/src/lxc/execute.c
index b4f3ed9..e1b9ac7 100644
--- a/src/lxc/execute.c
+++ b/src/lxc/execute.c
@@ -162,7 +162,7 @@ int lxc_execute(const char *name, char *const argv[], int quiet,
 		.quiet = quiet
 	};
 
-	if (lxc_check_inherited(conf, -1))
+	if (lxc_check_inherited(conf))
 		return -1;
 
 	conf->is_execute = 1;
diff --git a/src/lxc/lxc_console.c b/src/lxc/lxc_console.c
index bfee6fb..0fd08e8 100644
--- a/src/lxc/lxc_console.c
+++ b/src/lxc/lxc_console.c
@@ -28,7 +28,6 @@
 #include <errno.h>
 #include <string.h>
 #include <fcntl.h>
-#include <termios.h>
 #include <unistd.h>
 #include <signal.h>
 #include <libgen.h>
diff --git a/src/lxc/lxc_start.c b/src/lxc/lxc_start.c
index 05fb161..d9a5694 100644
--- a/src/lxc/lxc_start.c
+++ b/src/lxc/lxc_start.c
@@ -27,7 +27,6 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
-#include <termios.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <signal.h>
diff --git a/src/lxc/start.c b/src/lxc/start.c
index eb1c659..8049870 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -31,7 +31,6 @@
 #include <unistd.h>
 #include <signal.h>
 #include <fcntl.h>
-#include <termios.h>
 #include <grp.h>
 #include <poll.h>
 #include <sys/param.h>
@@ -170,7 +169,7 @@ static int match_fd(int fd)
 	return (fd == 0 || fd == 1 || fd == 2);
 }
 
-int lxc_check_inherited(struct lxc_conf *conf, int fd_to_ignore)
+int lxc_check_inherited(struct lxc_conf *conf)
 {
 	struct dirent dirent, *direntp;
 	int fd, fddir;
@@ -197,7 +196,7 @@ restart:
 
 		fd = atoi(direntp->d_name);
 
-		if (fd == fddir || fd == lxc_log_fd || fd == fd_to_ignore)
+		if (fd == fddir || fd == lxc_log_fd)
 			continue;
 
 		if (match_fd(fd))
@@ -697,9 +696,6 @@ static int do_start(void *data)
 	if (lsm_process_label_set(lsm_label, 1, 1) < 0)
 		goto out_warn_father;
 
-	if (lxc_check_inherited(handler->conf, handler->sigfd))
-		return -1;
-
 	/* If we mounted a temporary proc, then unmount it now */
 	tmp_proc_unmount(handler->conf);
 
@@ -726,8 +722,20 @@ static int do_start(void *data)
 		goto out_warn_father;
 	}
 
+	/* Some init's such as busybox will set sane tty settings on stdin,
+	 * stdout, stderr which it thinks is the console. We already set them
+	 * the way we wanted on the real terminal, and we want init to do its
+	 * 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) < 0)
+		goto out_warn_father;
+
 	close(handler->sigfd);
 
+	if (lxc_check_inherited(handler->conf))
+		return -1;
+
 	/* after this call, we are in error because this
 	 * ops should not return as it execs */
 	handler->ops->start(handler, handler->data);
diff --git a/src/lxc/start.h b/src/lxc/start.h
index e2d6bc8..17791b5 100644
--- a/src/lxc/start.h
+++ b/src/lxc/start.h
@@ -76,7 +76,7 @@ struct lxc_handler {
 
 extern struct lxc_handler *lxc_init(const char *name, struct lxc_conf *, const char *);
 
-extern int lxc_check_inherited(struct lxc_conf *conf, int fd_to_ignore);
+extern int lxc_check_inherited(struct lxc_conf *conf);
 int __lxc_start(const char *, struct lxc_conf *, struct lxc_operations *,
 		void *, const char *);
 
-- 
1.8.5.3



More information about the lxc-devel mailing list