[PATCH 5/6] main: Ensure fds 0-2 are populated

[David Gibson <david@gibson.dropbear.id.au>]

Usually fds 0-2 are stdin, stdout and stderr.  However, certain things
which might invoke passt can close some of those standard fds.  If so,
anything we open might be placed in one of the standard slots.

For the handful of things we open early enough, this can be a problem
because we close fds 0-2 in __daemon(), replacing them with dupes of
/dev/null.

We could avoid closing those fds in __daemon() if they're not standard
streams.  However, leaving things other than the standard streams in fds
0-2 is a footgun: a stray printf() that occurs in a circumstance it
shouldn't could send harmful garbage to a device or socket.  It's also
likely to be confusing if debugging with strace or similar.

To avoid this, fill any missing standard streams with a dupe of /dev/null,
right after isolate_initial().  Since open()ing /dev/null itself could
land in one of those fd 0-2 slots, we need to be careful when we close it
not to leave a new gap.

Link: https://bugs.passt.top/show_bug.cgi?id=215
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 passt.c | 19 +++++++++++++------
 util.c  |  3 +--
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/passt.c b/passt.c
index b99e5998..663a1005 100644
--- a/passt.c
+++ b/passt.c
@@ -330,8 +330,8 @@ static void passt_worker(void *opaque, int nfds, struct epoll_event *events)
 int main(int argc, char **argv)
 {
 	struct epoll_event events[NUM_EPOLL_EVENTS];
+	int nfds, devnull_fd = -1, fd;
 	struct ctx *c = &passt_ctx;
-	int nfds, devnull_fd = -1;
 	struct rlimit limit;
 	struct timespec now;
 	struct sigaction sa;
@@ -343,6 +343,15 @@ int main(int argc, char **argv)
 
 	c->fd_tap = isolate_initial(argc, argv);
 
+	if ((devnull_fd = open("/dev/null", O_RDWR | O_CLOEXEC)) < 0)
+		die_perror("Failed to open /dev/null");
+	/* Ensure fds 0-2 are populated */
+	for (fd = 0; fd <= STDERR_FILENO; fd++) {
+		if (fcntl(fd, F_GETFD) < 0 &&
+		    dup2(devnull_fd, fd) < 0)
+			die_perror("Failed to populate fd %d", fd);
+	}
+
 	sigemptyset(&sa.sa_mask);
 	sa.sa_flags = 0;
 	sa.sa_handler = exit_handler;
@@ -410,11 +419,6 @@ int main(int argc, char **argv)
 	fwd_neigh_table_init(c);
 	nl_neigh_notify_init(c);
 
-	if (!c->foreground) {
-		if ((devnull_fd = open("/dev/null", O_RDWR | O_CLOEXEC)) < 0)
-			die_perror("Failed to open /dev/null");
-	}
-
 	if (isolate_prefork(c))
 		die("Failed to sandbox process, exiting");
 
@@ -428,6 +432,9 @@ int main(int argc, char **argv)
 		c->pidfile_fd = -1;
 	}
 
+	if (devnull_fd > STDERR_FILENO)
+		close(devnull_fd);
+
 	if (pasta_child_pid) {
 		kill(pasta_child_pid, SIGUSR1);
 		log_stderr = false;
diff --git a/util.c b/util.c
index ce5021a9..bd4c6caa 100644
--- a/util.c
+++ b/util.c
@@ -522,8 +522,7 @@ int __daemon(int pidfile_fd, int devnull_fd)
 	if (setsid()				< 0 ||
 	    dup2(devnull_fd, STDIN_FILENO)	< 0 ||
 	    dup2(devnull_fd, STDOUT_FILENO)	< 0 ||
-	    dup2(devnull_fd, STDERR_FILENO)	< 0 ||
-	    close(devnull_fd))
+	    dup2(devnull_fd, STDERR_FILENO)	< 0)
 		passt_exit(EXIT_FAILURE);
 
 	return 0;
-- 
2.55.0