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

[Stefano Brivio <sbrivio@redhat.com>]

On Wed,  4 Nov 2026 14:20:19 -0600
Richard Lawrence <rlawrence@tamu.edu> wrote:

> 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:
> 
> - 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 `c->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>

Applied, sorry it took so long.

Thanks for sticking to this, and welcome to the git log!

By the way, I took the liberty to fix a couple of minor coding style
issues on merge:

> ---
>  conf.c      | 55 ++++++++++++++++++++++++++----------------
>  conf.h      |  2 +-
>  isolation.c | 69 +++++++++++++++++++++++++++++++----------------------
>  isolation.h |  2 +-
>  passt.1     |  5 ++++
>  passt.c     |  2 +-
>  passt.h     |  2 ++
>  7 files changed, 84 insertions(+), 53 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 149a4c7..9f7eafb 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -750,7 +750,8 @@ 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\n");
>  
>  	passt_exit(status);
>  }
> @@ -1137,35 +1138,38 @@ static void conf_sock_listen(const struct ctx *c)
>  }
>  
>  /**
> - * 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
> + * @c:		Execution context
>   * @argc:	Argument count
>   * @argv:	Command line options
> - *
> - * Return: fd number from --fd option, or -1 if not supplied
>   */
> -int conf_tap_fd(int argc, char **argv)
> +void conf_fds(struct ctx *c, 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, 2 },
> +		{ 0 },
> +	};
>  	int name;
>  
> +	c->fd_tap = -1;
> +	c->pass_fds = false;
>  	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;
> +			c->fd_tap = (int)val;
> +		} else if (name == 2) {
> +			c->pass_fds = true;
> +		}
> +	} while (name != -1);
>  }
>  
>  /**
> @@ -1317,6 +1321,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",	no_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 +1572,11 @@ void conf(struct ctx *c, int argc, char **argv)
>  
>  			c->no_pidns = true;
>  			break;
> +		case 34:
> +			/* --pass-fds was parsed early */
> +			if (c->mode != MODE_PASTA)
> +				die("--pass-fds is for pasta mode only");
> +			break;
>  		case 'd':
>  			c->debug = 1;
>  			c->quiet = 0;
> @@ -1603,7 +1613,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  			c->fd_control_listen = c->fd_control = -1;
>  			break;
>  		case 'F':
> -			/* --fd was parsed early and c->fd_tap set in main() */
> +			/* --fd was parsed early */
>  			c->one_off = true;
>  			*c->sock_path = 0;
>  			break;
> @@ -1929,6 +1939,9 @@ void conf(struct ctx *c, int argc, char **argv)
>  	else if (optind != argc)
>  		die("Extra non-option argument: %s", argv[optind]);
>  
> +	if (c->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..654fd16 100644
> --- a/conf.h
> +++ b/conf.h
> @@ -7,7 +7,7 @@
>  #define CONF_H
>  
>  enum passt_modes conf_mode(int argc, char *argv[]);
> -int conf_tap_fd(int argc, char **argv);
> +void conf_fds(struct ctx *c, 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..1875e6b 100644
> --- a/isolation.c
> +++ b/isolation.c
> @@ -250,46 +250,57 @@ 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

a missing * here,

>   *  - 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;
> +	conf_fds(c, argc, argv);
>  
> -	fd = conf_tap_fd(argc, argv);
> +	if (c->pass_fds) {
> +		if (c->fd_tap >= 0 && c->fd_tap < STDERR_FILENO + 1) {
> +			/* Move the tap fd to a safer location */
> +			int new_fd = fcntl(c->fd_tap, F_DUPFD, STDERR_FILENO + 1);

line wrapping here,

>  
> -	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 (new_fd < 0 || close(c->fd_tap))
> +				die_perror("Could not relocate --fd descriptor");

and here,

> +			c->fd_tap = new_fd;
> +		}
> +	} else {
> +		int close_from = STDERR_FILENO + 1;
> +
> +		if (c->fd_tap >= 0) {
> +			/* Move the tap fd to a more convenient location */
> +			if (c->fd_tap != close_from			&&
> +				(dup2(c->fd_tap, close_from) != close_from	||
> +				 close(c->fd_tap)))

vertical alignment here,

> +				die_perror("Could not move --fd descriptor");
> +			c->fd_tap = 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 leaked file descriptors */
> +		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.

line wrapping here,

> +				*/
> +				warn(
> +	"Can't use close_range() to ensure no files leaked by parent");

and this, which was supposed to be at the beginning of the line.

> +			} else {
> +				die_perror("Failed to close files leaked by parent");
> +			}
>  		}
>  	}
> -
> -	return fd;
>  }
>  
>  /**

-- 
Stefano