[lxc-devel] [PATCH v3] fix console stdin,stdout,stderr fds
Dwight Engen
dwight.engen at oracle.com
Fri Mar 21 20:39:34 UTC 2014
On Fri, 21 Mar 2014 15:00:11 -0400
Stéphane Graber <stgraber at ubuntu.com> wrote:
> On Fri, Mar 21, 2014 at 11:19:21AM -0400, Dwight Engen wrote:
> > On Tue, 11 Mar 2014 23:06:22 -0500
> > Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> >
> > > 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.
> > > >
> > > > V3: Don't move lxc_check_inherited() since it needs to be called
> > > > while the tmp proc mount is still mounted. Move call to
> > > > lxc_console_set_stdfds() just before it.
> > > >
> > > > Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> > >
> > > looks good, thanks!
> > >
> > > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> >
> > Hi, I think maybe this one missed being pushed, unless I missed
> > something else that needed to be fixed up with it.
>
> Ah, I assumed Serge pushed it already, let me grab it from the archive
> and send it then.
Thanks Stéphane.
> >
> > > > ---
> > > > src/lxc/console.c | 18 +++++++++++++++++-
> > > > src/lxc/console.h | 1 +
> > > > src/lxc/lxc_console.c | 1 -
> > > > src/lxc/lxc_start.c | 1 -
> > > > src/lxc/start.c | 10 +++++++++-
> > > > 5 files changed, 27 insertions(+), 4 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/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 6b57ee1..ec10496 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>
> > > > @@ -697,6 +696,15 @@ static int do_start(void *data)
> > > > if (lsm_process_label_set(lsm_label, 1, 1) < 0)
> > > > 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;
> > > > +
> > > > if (lxc_check_inherited(handler->conf, handler->sigfd))
> > > > return -1;
> > > >
> > > > --
> > > > 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