[pull] master from upstream

conf.c

@@ -1153,17 +1153,32 @@ static void conf_sock_listen(const struct ctx *c)
 
 /**
  * conf_tap_fd() - Read tap fd as supplied by -F command line option
- * @arg:	Argument to -F command line option
+ * @argc:	Argument count
+ * @argv:	Command line options
+ *
+ * Return: fd number from --fd option, or -1 if not supplied
  */
-int conf_tap_fd(const char *arg)
+int conf_tap_fd(int argc, char **argv)
 {
-	const char *p = arg;
+	const struct option optfd[] = {	{ "fd", required_argument, NULL, 'F' },
+					{ 0 }, };
+	const char *fdarg = NULL, *p;
 	unsigned long val;
+	int name;
+
+	optind = 0;
+	do {
+		name = getopt_long(argc, argv, "-:F:", optfd, NULL);
+		if (name == 'F')
+			fdarg = optarg;
+	} while (name != -1);
+
+	if (!fdarg)
+		return -1;
 
-	if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p)	||
-	    val > INT_MAX					||
-	    (val != STDIN_FILENO && val <= STDERR_FILENO))
-		die("Invalid --fd: %s", arg);
+	p = fdarg;
+	if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) || val > INT_MAX)
+		die("Invalid --fd: %s", fdarg);
 
 	return val;
 }
@@ -1593,7 +1608,7 @@ void conf(struct ctx *c, int argc, char **argv)
 			c->fd_control_listen = c->fd_control = -1;
 			break;
 		case 'F':
-			c->fd_tap = conf_tap_fd(optarg);
+			/* --fd was parsed early and c->fd_tap set in main() */
 			c->one_off = true;
 			*c->sock_path = 0;
 			break;

conf.h

@@ -7,7 +7,7 @@
 #define CONF_H
 
 enum passt_modes conf_mode(int argc, char *argv[]);
-int conf_tap_fd(const char *arg);
+int conf_tap_fd(int argc, char **argv);
 void conf(struct ctx *c, int argc, char **argv);
 void conf_listen_handler(struct ctx *c, uint32_t events);
 void conf_handler(struct ctx *c, uint32_t events);

isolation.c

@@ -24,13 +24,18 @@
  * done anything we need to do with those resources, so we have
  * multiple stages of self-isolation.  In order these are:
  *
- * 1. isolate_initial()
+ * 1a. isolate_initial()
  * ====================
  *
  * Executed immediately after startup, drops capabilities we don't
  * need at any point during execution (or which we gain back when we
- * need by joining other namespaces), and closes any leaked file we
- * might have inherited from the parent process.
+ * need by joining other namespaces).
+ *
+ * 1b. isolate_fds()
+ * ================
+ *
+ * Executed immediately after isolate_initial().  Closes any leaked
+ * files we might have inherited from the parent process.
  *
  * 2. isolate_user()
  * =================
@@ -88,6 +93,7 @@
 #include "passt.h"
 #include "log.h"
 #include "isolation.h"
+#include "conf.h"
 
 #define CAP_VERSION	_LINUX_CAPABILITY_VERSION_3
 #define CAP_WORDS	_LINUX_CAPABILITY_U32S_3
@@ -193,16 +199,13 @@ static int move_root(void)
 
 /**
  * isolate_initial() - Early, mostly config independent self isolation
- * @argc:	Argument count
- * @argv:	Command line options: only --fd (if present) is relevant here
  *
  * Should:
  *  - drop unneeded capabilities
- *  - close all open files except for standard streams and the one from --fd
  * Mustn't:
  *  - remove filesystem access (we need to access files during setup)
  */
-void isolate_initial(int argc, char **argv)
+void isolate_initial(void)
 {
 	uint64_t keep;
 
@@ -243,8 +246,49 @@ void isolate_initial(int argc, char **argv)
 		keep |= BIT(CAP_SETFCAP) | BIT(CAP_SYS_PTRACE);
 
 	drop_caps_ep_except(keep);
+}
+
+/*
+ * isolate_fds() - Close leaked files, but not --fd, stdin, stdout, stderr
+ * @argc:	Argument count
+ * @argv:	Command line options, as we need to skip any file given via --fd
+ *
+ * Should:
+ *  - close all open files except for standard streams and the one from --fd
+ *  - move the --fd descriptor out of the range 0-2
+ *
+ * Return: new fd number for descriptor from --fd, or -1 if not specified
+ */
+int isolate_fds(int argc, char **argv)
+{
+	int fd, close_from = STDERR_FILENO + 1;
+
+	fd = conf_tap_fd(argc, argv);
+
+	if (fd >= 0) {
+		/* Move the passed fd to a more convenient location */
+		if (fd != close_from			&&
+		    (dup2(fd, close_from) != close_from	||
+		     close(fd)))
+			die_perror("Could not move --fd descriptor");
+		fd = close_from++;
+	}
+
+	if (close_range(close_from, ~0U, CLOSE_RANGE_UNSHARE)) {
+		if (errno == ENOSYS || errno == EINVAL) {
+			/* This probably means close_range() or the
+			 * CLOSE_RANGE_UNSHARE flag is not supported by the
+			 * kernel.  Not much we can do here except carry on and
+			 * hope for the best.
+			 */
+			warn(
+"Can't use close_range() to ensure no files leaked by parent");
+		} else {
+			die_perror("Failed to close files leaked by parent");
+		}
+	}
 
-	close_open_files(argc, argv);
+	return fd;
 }
 
 /**

isolation.h

@@ -10,7 +10,8 @@
 #include <stdbool.h>
 #include <unistd.h>
 
-void isolate_initial(int argc, char **argv);
+void isolate_initial(void);
+int isolate_fds(int argc, char **argv);
 void isolate_user(const struct ctx *c, uid_t uid, gid_t gid, bool use_userns,
 		  const char *userns);
 int isolate_prefork(const struct ctx *c);

passt.c

@@ -333,8 +333,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;
@@ -344,7 +344,17 @@ int main(int argc, char **argv)
 
 	arch_avx2_exec(argv);
 
-	isolate_initial(argc, argv);
+	isolate_initial();
+	c->fd_tap = isolate_fds(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;
@@ -413,24 +423,24 @@ 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");
 
 	if (!c->foreground) {
 		__daemon(c->pidfile_fd, devnull_fd);
-		close(c->pidfile_fd);
-		c->pidfile_fd = -1;
 		log_stderr = false;
 	} else {
 		pidfile_write(c->pidfile_fd, getpid());
+	}
+
+	if (c->pidfile_fd >= 0) {
+		close(c->pidfile_fd);
 		c->pidfile_fd = -1;
 	}
 
+	if (devnull_fd > STDERR_FILENO)
+		close(devnull_fd);
+
 	if (pasta_child_pid) {
 		kill(pasta_child_pid, SIGUSR1);
 		log_stderr = false;

util.c

@@ -26,7 +26,6 @@
 #include <stdbool.h>
 #include <linux/errqueue.h>
 #include <linux/in6.h>
-#include <getopt.h>
 
 #include "linux_dep.h"
 #include "util.h"
@@ -38,7 +37,6 @@
 #include "epoll_ctl.h"
 #include "pasta.h"
 #include "serialise.h"
-#include "conf.h"
 #ifdef HAS_GETRANDOM
 #include <sys/random.h>
 #endif
@@ -524,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;
@@ -924,53 +921,6 @@ const char *str_ee_origin(const struct sock_extended_err *ee)
 	return "<invalid>";
 }
 
-/**
- * close_open_files() - Close leaked files, but not --fd, stdin, stdout, stderr
- * @argc:	Argument count
- * @argv:	Command line options, as we need to skip any file given via --fd
- */
-void close_open_files(int argc, char **argv)
-{
-	const struct option optfd[] = { { "fd", required_argument, NULL, 'F' },
-					{ 0 },
-				      };
-	long fd = -1;
-	int name, rc;
-
-	do {
-		name = getopt_long(argc, argv, "-:F:", optfd, NULL);
-
-		if (name == 'F')
-			fd = conf_tap_fd(optarg);
-	} while (name != -1);
-
-	if (fd == -1) {
-		rc = close_range(STDERR_FILENO + 1, ~0U, CLOSE_RANGE_UNSHARE);
-	} else if (fd == STDERR_FILENO + 1) { /* Still a single range */
-		rc = close_range(STDERR_FILENO + 2, ~0U, CLOSE_RANGE_UNSHARE);
-	} else {
-		rc = close_range(STDERR_FILENO + 1, fd - 1,
-				 CLOSE_RANGE_UNSHARE);
-		if (!rc)
-			rc = close_range(fd + 1, ~0U, CLOSE_RANGE_UNSHARE);
-	}
-
-	if (rc) {
-		if (errno == ENOSYS || errno == EINVAL) {
-			/* This probably means close_range() or the
-			 * CLOSE_RANGE_UNSHARE flag is not supported by the
-			 * kernel.  Not much we can do here except carry on and
-			 * hope for the best.
-			 */
-			warn(
-"Can't use close_range() to ensure no files leaked by parent");
-		} else {
-			die_perror("Failed to close files leaked by parent");
-		}
-	}
-
-}
-
 /**
  * snprintf_check() - snprintf() wrapper, checking for truncation and errors
  * @str:	Output buffer

util.h

@@ -168,7 +168,6 @@ intmax_t read_file_integer(const char *path, intmax_t fallback);
 int write_remainder(int fd, const struct iovec *iov, size_t iovcnt,
 		    size_t skip, size_t length);
 int read_remainder(int fd, const struct iovec *iov, size_t cnt, size_t skip);
-void close_open_files(int argc, char **argv);
 bool snprintf_check(char *str, size_t size, const char *format, ...);
 long clamped_scale(long x, long y, long lo, long hi, long f);