uniformly nullify std fds

In various places throughout the code, we want to "nullify" the std fds,
opening them to /dev/null or zero or so. Instead, let's unify this code and do
it in such a way that Coverity (probably) won't complain.

v2: use /dev/null for stdin as well
v3: add a comment about use of C's short circuiting
v4: axe comment, check errors on dup2, s/quiet/need_null_stdfds

Reported-by: Coverity
Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>

diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c
index 53465b1c6d5241c2b0623fe1711b4a7e95d5277d..520652c7237cbe036cd6c09d36c8a53fac889fca 100644 (file)
--- a/src/lxc/bdev.c
+++ b/src/lxc/bdev.c
@@ -224,12 +224,8 @@ static int do_mkfs(const char *path, const char *fstype)
 
        // If the file is not a block device, we don't want mkfs to ask
        // us about whether to proceed.
-       close(0);
-       close(1);
-       close(2);
-       open("/dev/zero", O_RDONLY);
-       open("/dev/null", O_RDWR);
-       open("/dev/null", O_RDWR);
+       if (null_stdfds() < 0)
+               exit(1);
        execlp("mkfs", "mkfs", "-t", fstype, path, NULL);
        exit(1);
 }

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 445cc226bace980f384c42d08a3945ad057a628e..7708a8c49371fb3a2932044063493e8870a5244f 100644 (file)
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -722,12 +722,10 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
                        return false;
                }
                lxc_check_inherited(conf, true, -1);
-               close(0);
-               close(1);
-               close(2);
-               open("/dev/zero", O_RDONLY);
-               open("/dev/null", O_RDWR);
-               open("/dev/null", O_RDWR);
+               if (null_stdfds() < 0) {
+                       ERROR("failed to close fds");
+                       return false;
+               }
                setsid();
        } else {
                if (!am_single_threaded()) {
@@ -956,7 +954,7 @@ static char *lxcbasename(char *path)
        return p;
 }
 
-static bool create_run_template(struct lxc_container *c, char *tpath, bool quiet,
+static bool create_run_template(struct lxc_container *c, char *tpath, bool need_null_stdfds,
                                char *const argv[])
 {
        pid_t pid;
@@ -978,13 +976,8 @@ static bool create_run_template(struct lxc_container *c, char *tpath, bool quiet
                char **newargv;
                struct lxc_conf *conf = c->lxc_conf;
 
-               if (quiet) {
-                       close(0);
-                       close(1);
-                       close(2);
-                       open("/dev/zero", O_RDONLY);
-                       open("/dev/null", O_RDWR);
-                       open("/dev/null", O_RDWR);
+               if (need_null_stdfds && null_stdfds() < 0) {
+                       exit(1);
                }
 
                src = c->lxc_conf->rootfs.path;

diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
index 3741586844df219db9feca9b565f9d987eab0e44..dd8ca9e02c445ac50b6c563dec00115182ce3dac 100644 (file)
--- a/src/lxc/monitor.c
+++ b/src/lxc/monitor.c
@@ -329,12 +329,8 @@ int lxc_monitord_spawn(const char *lxcpath)
                exit(EXIT_FAILURE);
        }
        lxc_check_inherited(NULL, true, pipefd[1]);
-       close(0);
-       close(1);
-       close(2);
-       open("/dev/null", O_RDONLY);
-       open("/dev/null", O_RDWR);
-       open("/dev/null", O_RDWR);
+       if (null_stdfds() < 0)
+               exit(EXIT_FAILURE);
        close(pipefd[0]);
        sprintf(pipefd_str, "%d", pipefd[1]);
        execvp(args[0], args);

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 71cd9ef87cabc4d410eb24371d5810cc72f7cf94..6eded6155941e2a5c41ea1de4c58e2d62149a503 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -762,14 +762,8 @@ static int do_start(void *data)
 
        close(handler->sigfd);
 
-       if (handler->backgrounded) {
-               close(0);
-               close(1);
-               close(2);
-               open("/dev/zero", O_RDONLY);
-               open("/dev/null", O_RDWR);
-               open("/dev/null", O_RDWR);
-       }
+       if (handler->backgrounded && null_stdfds() < 0)
+               goto out_warn_father;
 
        /* after this call, we are in error because this
         * ops should not return as it execs */

diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 467bc1b42af96a6df3ea862f321701b2c5bde2a2..7ced314872ac4bbca21d9e564f6188d95163db68 100644 (file)
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1445,3 +1445,24 @@ domount:
        INFO("Mounted /proc in container for security transition");
        return 1;
 }
+
+int null_stdfds(void)
+{
+       int fd, ret = -1;
+
+       fd = open("/dev/null", O_RDWR);
+       if (fd < 0)
+               return -1;
+
+       if (dup2(fd, 0) < 0)
+               goto err;
+       if (dup2(fd, 1) < 0)
+               goto err;
+       if (dup2(fd, 2) < 0)
+               goto err;
+
+       ret = 0;
+err:
+       close(fd);
+       return ret;
+}

diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index 6bd05e0110129bef815ece4f4767f15765100eca..ee12dde457fc9bad8d7004a94f3857d8addbb178 100644 (file)
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -280,4 +280,5 @@ int is_dir(const char *path);
 char *get_template_path(const char *t);
 int setproctitle(char *title);
 int mount_proc_if_needed(const char *rootfs);
+int null_stdfds(void);
 #endif /* __LXC_UTILS_H */