close-all-fds: fix behavior

We want to close all inherited fds in three cases - one, if a container
is daemonized.  Two, if the user specifies -C on the lxc-start command
line.  Three, in src/lxc/monitor.c.  The presence of -C is passed in the
lxc_conf may not always exist.

One call to lxc_check_inherited was being done from lxc_start(), which
doesn't know whether we are daemonized.  Move that call to its caller,
lxcapi_start(), which does know.

Pass an explicit closeall boolean as second argument to lxc_check_inherited.
If it is true, then all fds are closed.  If it is false, then we check
the lxc_conf->close_all_fds.

With this, all tests pass, and the logic appears correct.

Note that when -C is not true, then we only warn about inherited fds,
but we do not abort the container start.  This appears to have ben the case
since commit 92c7f6295518 in 2011.  Unfortunately the referenced URL with
the justification is no longer valid.  We may want to consider becoming
stricter about this again.  (Note that the commit did say "for now")

Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Acked-by: Stéphane Graber <stgraber@ubuntu.com>

diff --git a/src/lxc/execute.c b/src/lxc/execute.c
index b78bcbfc0462c4c6ba33432e0e49f380c36e9663..a0f7ff11ceeab3aeb8e36762e45f9eb268d5e7b2 100644 (file)
--- a/src/lxc/execute.c
+++ b/src/lxc/execute.c
@@ -118,7 +118,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, false, -1))
                return -1;
 
        conf->is_execute = 1;

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 0d366873bf69a87beae11079b1349568208d7a5a..7ed87170868766728930f51eeb1e68f7a6319849 100644 (file)
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -606,7 +606,6 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
        * while container is running...
        */
        if (daemonize) {
-               conf->close_all_fds = 1;
                lxc_monitord_spawn(c->config_path);
 
                pid_t pid = fork();
@@ -634,7 +633,7 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
                        SYSERROR("Error chdir()ing to /.");
                        return false;
                }
-               lxc_check_inherited(conf, -1);
+               lxc_check_inherited(conf, true, -1);
                close(0);
                close(1);
                close(2);
@@ -673,6 +672,13 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
 
 reboot:
        conf->reboot = 0;
+
+       if (lxc_check_inherited(conf, daemonize, -1)) {
+               ERROR("Inherited fds found");
+               ret = 1;
+               goto out;
+       }
+
        ret = lxc_start(c->name, argv, conf, c->config_path);
        c->error_num = ret;
 
@@ -682,6 +688,7 @@ reboot:
                goto reboot;
        }
 
+out:
        if (c->pidfile) {
                unlink(c->pidfile);
                free(c->pidfile);

diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
index f6d36a96bb840a82dbdad39a982859728542cb77..1e1c094a9fbec30a55d839d3928750f808787bb4 100644 (file)
--- a/src/lxc/monitor.c
+++ b/src/lxc/monitor.c
@@ -331,7 +331,7 @@ int lxc_monitord_spawn(const char *lxcpath)
                SYSERROR("failed to setsid");
                exit(EXIT_FAILURE);
        }
-       lxc_check_inherited(NULL, pipefd[1]);
+       lxc_check_inherited(NULL, true, pipefd[1]);
        close(0);
        close(1);
        close(2);

diff --git a/src/lxc/start.c b/src/lxc/start.c
index cd78665ecf221c923ef44ee2e642508cf6ea92ea..f9bff519211aea6bebf7701a5780f2e8259fa4ab 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -170,12 +170,24 @@ 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)
+/*
+ * Check for any fds we need to close
+ * * if fd_to_ignore != -1, then if we find that fd open we will ignore it.
+ * * By default we warn about open fds we find.
+ * * If closeall is true, we will close open fds.
+ * * If lxc-start was passed "-C", then conf->close_all_fds will be true,
+ *     in which case we also close all open fds.
+ * * A daemonized container will always pass closeall=true.
+ */
+int lxc_check_inherited(struct lxc_conf *conf, bool closeall, int fd_to_ignore)
 {
        struct dirent dirent, *direntp;
        int fd, fddir;
        DIR *dir;
 
+       if (conf && conf->close_all_fds)
+               closeall = true;
+
 restart:
        dir = opendir("/proc/self/fd");
        if (!dir) {
@@ -203,7 +215,7 @@ restart:
                if (match_fd(fd))
                        continue;
 
-               if (conf == NULL || conf->close_all_fds) {
+               if (closeall) {
                        close(fd);
                        closedir(dir);
                        INFO("closed inherited fd %d", fd);
@@ -1187,9 +1199,6 @@ int lxc_start(const char *name, char *const argv[], struct lxc_conf *conf,
                .argv = argv,
        };
 
-       if (lxc_check_inherited(conf, -1))
-               return -1;
-
        conf->need_utmp_watch = 1;
        return __lxc_start(name, conf, &start_ops, &start_arg, lxcpath);
 }

diff --git a/src/lxc/start.h b/src/lxc/start.h
index 7c75b16f3c1494b3d6688c74d6923304a2b498f4..d39b3b46a38cb6841ac1a7573fcf5c5d5156743d 100644 (file)
--- a/src/lxc/start.h
+++ b/src/lxc/start.h
@@ -25,6 +25,7 @@
 
 #include <signal.h>
 #include <sys/param.h>
+#include <stdbool.h>
 
 #include "config.h"
 #include "state.h"
@@ -81,7 +82,7 @@ extern void lxc_abort(const char *name, struct lxc_handler *handler);
 extern struct lxc_handler *lxc_init(const char *name, struct lxc_conf *, const char *);
 extern void lxc_fini(const char *name, struct lxc_handler *handler);
 
-extern int lxc_check_inherited(struct lxc_conf *conf, int fd_to_ignore);
+extern int lxc_check_inherited(struct lxc_conf *conf, bool closeall, int fd_to_ignore);
 int __lxc_start(const char *, struct lxc_conf *, struct lxc_operations *,
                void *, const char *);