[PATCH v2] pasta: Add --pass-fds option to skip closing file descriptors

[PATCH v2] pasta: Add --pass-fds option to skip closing file descriptors

[Richard Lawrence]

When pasta spawns a command (`pasta [options] -- [command]`), all open file
descriptors inherited from the parent process except standard streams (0-2)
and the tap descriptor (`--fd`) are normally closed by `isolate_fds()`.

However, when pasta is used for testing, development, or launching tools like
Bubblewrap (`bwrap`), the spawned command may require file descriptors
passed directly from the parent process. In this case, closing inherited file
descriptors breaks expected behavior.

Add a boolean `--pass-fds` flag for pasta mode that skips the `close_range()`
cleanup step in `isolate_fds()`, leaving all open file descriptors intact
for the child command.

Summary of changes:

- Introduce `struct fd_opts` and update `conf_tap_fd()` to `conf_fds()`,
  allowing early parsing of both `-F`/`--fd` and `--pass-fds` before main
  option processing.
- Update `isolate_fds()` to check `opts.pass_fds`: if set, skip calling
  `close_range()` while still relocating the tap fd out of standard stream
  range (0-2) if needed.
- Enforce validation in `conf()` ensuring `--pass-fds` is restricted to pasta
  mode when spawning a command (incompatible with attaching to an
  existing netns).
- Update man page and usage help output.

Link: https://bugs.passt.top/show_bug.cgi?id=204
Signed-off-by: Richard Lawrence <rlawrence@tamu.edu>
---
 conf.c      | 58 ++++++++++++++++++++++++++---------------
 conf.h      |  7 ++++-
 isolation.c | 74 +++++++++++++++++++++++++++++++++--------------------
 isolation.h |  2 +-
 passt.1     |  5 ++++
 passt.c     |  2 +-
 passt.h     |  2 ++
 7 files changed, 99 insertions(+), 51 deletions(-)

diff --git a/conf.c b/conf.c
index 149a4c7..4479467 100644
--- a/conf.c
+++ b/conf.c
@@ -750,7 +750,9 @@ pasta_opts:
 		"			Don't copy all addresses to namespace\n"
 		"  --ns-mac-addr ADDR	Set MAC address on tap interface\n"
 		"  --no-splice		Disable inbound socket splicing\n"
-		"  --splice-only	Only enable loopback forwarding\n");
+		"  --splice-only	Only enable loopback forwarding\n"
+		"  --pass-fds		Preserve open file descriptors and\n"
+		"			pass them to the spawned command\n");
 
 	passt_exit(status);
 }
@@ -1136,36 +1138,43 @@ static void conf_sock_listen(const struct ctx *c)
 		die_perror("Couldn't add configuration socket to epoll");
 }
 
+
+
+
 /**
- * conf_tap_fd() - Read tap fd as supplied by -F command line option
+ * conf_fds() - Read fds as supplied by -F/--fd and --pass-fds options
  * @argc:	Argument count
  * @argv:	Command line options
- *
- * Return: fd number from --fd option, or -1 if not supplied
+ 
+ * Return: fd_opts { fd_tap, pass_fds }
  */
-int conf_tap_fd(int argc, char **argv)
+struct fd_opts conf_fds(int argc, char **argv)
 {
-	const struct option optfd[] = {	{ "fd", required_argument, NULL, 'F' },
-					{ 0 }, };
-	const char *fdarg = NULL, *p;
-	unsigned long val;
+	const struct option opt[] = {
+		{ "fd",      required_argument, NULL, 'F' },
+		{ "pass-fds", no_argument, NULL, 34 },
+		{ 0 },
+	};
+	struct fd_opts opts = { .fd_tap = -1, .pass_fds = false };
 	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;
+		name = getopt_long(argc, argv, "-:F:", opt, NULL);
+		if (name == 'F') {
+			const char *p = optarg;
+			unsigned long val;
 
-	p = fdarg;
-	if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) || val > INT_MAX)
-		die("Invalid --fd: %s", fdarg);
+			if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) ||
+			    val > INT_MAX)
+				die("Invalid --fd: %s", optarg);
 
-	return val;
+			opts.fd_tap = (int)val;
+		} else if (name == 34) {
+			opts.pass_fds = true;
+		}
+	} while (name != -1);
+	return opts;
 }
 
 /**
@@ -1317,6 +1326,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		{"conf-path",	required_argument,	NULL,		'c' },
 		{"chroot-fallback", no_argument,	NULL, 		32 },
 		{"no-pidns",	no_argument,		NULL,		33 },
+		{"pass-fds",	required_argument,	NULL,		34 },
 		{ 0 },
 	};
 	const char *optstring = "+dqfel:hs:c:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:";
@@ -1567,6 +1577,11 @@ void conf(struct ctx *c, int argc, char **argv)
 
 			c->no_pidns = true;
 			break;
+		case 34:
+			/* --pass-fds was parsed early and c->pasta_pass_fds set in main() */
+			if (c->mode != MODE_PASTA)
+				die("--pass-fds is for pasta mode only");
+			break;
 		case 'd':
 			c->debug = 1;
 			c->quiet = 0;
@@ -1929,6 +1944,9 @@ void conf(struct ctx *c, int argc, char **argv)
 	else if (optind != argc)
 		die("Extra non-option argument: %s", argv[optind]);
 
+	if (c->pasta_pass_fds && *netns)
+		die("--pass-fds can only be used when spawning a command");
+
 	if (c->no_pidns && *netns)
 		die("--no-pidns is incompatible with PID or --netns");
 
diff --git a/conf.h b/conf.h
index 19bf9bc..e11661f 100644
--- a/conf.h
+++ b/conf.h
@@ -6,8 +6,13 @@
 #ifndef CONF_H
 #define CONF_H
 
+struct fd_opts {
+	int fd_tap;
+	bool pass_fds;
+};
+
 enum passt_modes conf_mode(int argc, char *argv[]);
-int conf_tap_fd(int argc, char **argv);
+struct fd_opts conf_fds(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);
diff --git a/isolation.c b/isolation.c
index a00475f..4105dc3 100644
--- a/isolation.c
+++ b/isolation.c
@@ -250,46 +250,64 @@ void isolate_initial(void)
 }
 
 /*
- * isolate_fds() - Close leaked files, but not --fd, stdin, stdout, stderr
+ * isolate_fds() - Close leaked files if applicable
+ * @c:		Execution context
  * @argc:	Argument count
- * @argv:	Command line options, as we need to skip any file given via --fd
+ * @argv:	Command line options
  *
  * Should:
- *  - close all open files except for standard streams and the one from --fd
+ *  - close all open files except for standard streams and the one from --fd,
+      unless --pass-fds is used, in which case all fds are preserved
  *  - move the --fd descriptor out of the range 0-2
- *
- * Return: new fd number for descriptor from --fd, or -1 if not specified
+ *  - update the execution context
  */
-int isolate_fds(int argc, char **argv)
+void isolate_fds(struct ctx *c, int argc, char **argv)
 {
-	int fd, close_from = STDERR_FILENO + 1;
+	struct fd_opts opts = conf_fds(argc, argv);
+	if (opts.fd_tap >= 0 && opts.fd_tap < 3) {
+		/* Move the tap fd to a safer location */
+		int new_fd = fcntl(opts.fd_tap, F_DUPFD, STDERR_FILENO + 1);
 
-	fd = conf_tap_fd(argc, argv);
+		if (new_fd < 0)
+			die_perror("Could not relocate --fd descriptor");
 
-	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++;
+		close(opts.fd_tap);
+		opts.fd_tap = new_fd;
 	}
+	c->fd_tap = opts.fd_tap;
+	c->pasta_pass_fds = opts.pass_fds;
+
+	/* Close leaked file descriptors unless --pass-fds is used */
+	if (!opts.pass_fds) {
+		int rc = 0;
+		/* Keep standard streams and the tap fd (if given) */
+		int keep = opts.fd_tap >= STDERR_FILENO ? opts.fd_tap : STDERR_FILENO;
+
+		if (keep > STDERR_FILENO + 1) {
+			/* Tap fd was relocated to above some other leaked fd(s)*/
+			/* Close fds between stderr and the tap fd */
+			if (close_range(STDERR_FILENO + 1, keep - 1, CLOSE_RANGE_UNSHARE))
+				rc = -1;
+		}
 
-	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 all other fds */
+		if (close_range(keep + 1, ~0U, CLOSE_RANGE_UNSHARE))
+			rc = -1;
+
+		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");
+			}
 		}
 	}
-
-	return fd;
 }
 
 /**
diff --git a/isolation.h b/isolation.h
index ec47038..fe8d7d5 100644
--- a/isolation.h
+++ b/isolation.h
@@ -11,7 +11,7 @@
 #include <unistd.h>
 
 void isolate_initial(void);
-int isolate_fds(int argc, char **argv);
+void isolate_fds(struct ctx *c, 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);
diff --git a/passt.1 b/passt.1
index 0b780fe..56e241f 100644
--- a/passt.1
+++ b/passt.1
@@ -766,6 +766,11 @@ of local traffic in pasta\fR in the \fBNOTES\fR for more details.
 Do not create a tap device in the namespace. In this mode, \fIpasta\fR only
 forwards loopback traffic between namespaces.
 
+.TP
+.BR \-\-pass\-fds
+Preserve all open file descriptors and pass them to the spawned command.
+This option may only be used when \fBpasta\fR spawns a command.
+
 .SH EXAMPLES
 
 .SS \fBpasta
diff --git a/passt.c b/passt.c
index 5054551..fa7afc5 100644
--- a/passt.c
+++ b/passt.c
@@ -345,7 +345,7 @@ int main(int argc, char **argv)
 	arch_avx2_exec(argv);
 
 	isolate_initial();
-	c->fd_tap = isolate_fds(argc, argv);
+	isolate_fds(c, argc, argv);
 
 	if ((devnull_fd = open("/dev/null", O_RDWR | O_CLOEXEC)) < 0)
 		die_perror("Failed to open /dev/null");
diff --git a/passt.h b/passt.h
index afd8e9f..b065d41 100644
--- a/passt.h
+++ b/passt.h
@@ -196,6 +196,7 @@ struct ip6_ctx {
  * @pasta_ifi:		Index of namespace interface for pasta
  * @pasta_conf_ns:	Configure namespace after creating it
  * @no_pidns:		Don't create a new PID namespace for spawned command
+ * @pasta_pass_fds:	Do not close leaked file descriptors
  * @fwd:		Forwarding tables
  * @fwd_pending:	Pending forward tables
  * @no_tcp:		Disable TCP operation
@@ -280,6 +281,7 @@ struct ctx {
 	unsigned int pasta_ifi;
 	int pasta_conf_ns;
 	bool no_pidns;
+	bool pasta_pass_fds;
 
 	struct fwd_table *fwd[PIF_NUM_TYPES];
 	struct fwd_table *fwd_pending[PIF_NUM_TYPES];
-- 
2.52.0