[lxc-devel] [PATCH 1/1] close-all-fds: fix behavior
Stéphane Graber
stgraber at ubuntu.com
Thu Jan 15 22:11:05 UTC 2015
On Tue, Jan 13, 2015 at 06:02:26AM +0000, Serge Hallyn wrote:
> 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 at ubuntu.com>
Acked-by: Stéphane Graber <stgraber at ubuntu.com>
> ---
> src/lxc/execute.c | 2 +-
> src/lxc/lxccontainer.c | 11 +++++++++--
> src/lxc/monitor.c | 2 +-
> src/lxc/start.c | 19 ++++++++++++++-----
> src/lxc/start.h | 3 ++-
> 5 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/src/lxc/execute.c b/src/lxc/execute.c
> index b78bcbf..a0f7ff1 100644
> --- 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 0d36687..7ed8717 100644
> --- 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 f6d36a9..1e1c094 100644
> --- 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 cd78665..f9bff51 100644
> --- 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 7c75b16..d39b3b4 100644
> --- 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 *);
>
> --
> 2.1.3
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20150115/53edcb75/attachment.sig>
More information about the lxc-devel
mailing list