[lxc-devel] [PATCH v2] fix console stdin,stdout,stderr fds
Serge Hallyn
serge.hallyn at ubuntu.com
Fri Mar 7 23:01:17 UTC 2014
Quoting Dwight Engen (dwight.engen at oracle.com):
> 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;
> +
Hi,
so the reason why the lxc_check_inherited() was where it
was, was because at this point we've called tmp_proc_umount().
So in some cases /proc won't be mounted, and lxc_check_inherited()
finds the fds to close under /proc
> /* 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
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
More information about the lxc-devel
mailing list