[lxc-devel] [PATCH v3 4/4] uniformly nullify std fds

[Tycho Andersen]

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

Reported-by: Coverity
Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
---
 src/lxc/bdev.c         |  8 ++------
 src/lxc/lxccontainer.c | 24 +++++++++++-------------
 src/lxc/monitor.c      |  8 ++------
 src/lxc/start.c        | 10 ++--------
 src/lxc/utils.c        | 16 ++++++++++++++++
 src/lxc/utils.h        |  1 +
 6 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c
index 53465b1..520652c 100644
--- 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 445cc22..a0dd2a2 100644
--- 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()) {
@@ -978,13 +976,13 @@ 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);
+		/*
+		 * Here, we're taking advantage of C's short circuiting of
+		 * conditions: we should only fail if quiet is set and
+		 * null_stdfds fails.
+		 */
+		if (quiet && null_stdfds() < 0) {
+			exit(1);
 		}
 
 		src = c->lxc_conf->rootfs.path;
diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
index 0e56f75..144b16e 100644
--- 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 71cd9ef..6eded61 100644
--- 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 467bc1b..42dd38f 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1445,3 +1445,19 @@ domount:
 	INFO("Mounted /proc in container for security transition");
 	return 1;
 }
+
+int null_stdfds(void)
+{
+	int fd;
+
+	fd = open("/dev/null", O_RDWR);
+	if (fd < 0)
+		return -1;
+
+	dup2(fd, 0);
+	dup2(fd, 1);
+	dup2(fd, 2);
+	close(fd);
+
+	return 0;
+}
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index 6bd05e0..ee12dde 100644
--- 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 */
-- 
2.1.4