Re: [PATCH] pasta: Add --pass-fds option to preserve specific file descriptors

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 17 Jul 2026 16:14:06 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Jul 16, 2026 at 02:03:00PM -0500, Richard Lawrence wrote:
> > Introduce a new long-only command line option '--pass-fds' for
> > pasta mode. This option accepts a comma-separated list of file
> > descriptor numbers. It parses, validates, sorts, and deduplicates
> > these file descriptors, and then skips closing them by calling
> > close_range() on the gaps between them inside close_open_files().
> > 
> > The option is documented in the usage help and the passt.1 man page.  
> 
> This commit message really needs to say *why* this is a desirable
> change.  It needs to make the case for this, not just say what the
> patch does.
> 
> I'm guessing this is for the case of trying to pass fds not to pasta
> per se, but to the command running within the namespace pasta creates.

Right, I'm also guessing this is about:

  https://bugs.passt.top/show_bug.cgi?id=204

but it definitely needs to be stated in the commit message.

> If that's the case, there's already a workaround that's good enough,
> I'm not (so far) convinced there's a case for an option like this.
> Specifically, you can manually create a namespace with unshare, and
> pass whatever fds you wish to it, then attach pasta to that existing
> namespace, rather than having pasta create its own.
> 
> Even if I accept the need for passing arbitrary fds to a command
> spawned by pasta, I think this is an overly complex way of
> accomplishing it: the alternate approach would be to delay the closing
> of extra fds until after the pasta command is spawned.

That comes with the issue that we would still pass those file
descriptors on to the command we spawn, which is what we want to avoid
with close_open_files(), or isolate_fds() after the series
mentioned below.

I think we should still close those as soon as possible, but I agree
that this is an overly complex way to implement the option.

For example, we don't actually need to sort the array, we could actually
scan the array while closing ranges directly, that is:

1. close the first open interval between standard error, or the file
   descriptor passed as --fd (which is now moved to index 3), and the
   file descriptor with the lowest index in the array. Remove this
   index, i, from the array

2. as long as there is any file descriptor left in the array, close the
   range between i and the lowest index (open interval), and set i to
   this index

that should yield a significantly more compact implementation. Also
have a look at CONTRIBUTING.md for coding style references.

In general this patch seems to hammer together several general-purpose
bits until they work for this specific option, instead of directly
implement what we need (and nothing more).

I would suggest giving a try at _writing_ that implementation instead
of somehow generating it. It's probably going to be shorter and easier
to audit.

> Less importantly, this patch will definitely conflict with some
> patches I've just posted, so will likely need reworking and rebasing
> for that.

I guess / hope this implementation should become simpler on top of those
patches (which fixes some actual issues so we would need that series
first anyway).

Richard, if you need a reference, this would be David's series at:

  https://archives.passt.top/passt-dev/20260717054634.1293553-1-david@gibson.dropbear.id.au/

which you can apply to your local tree with:

  b4 shazam https://archives.passt.top/passt-dev/20260717054634.1293553-1-david@gibson.dropbear.id.au/

> > Co-authored-by: rarensu <40872193+rarensu@users.noreply.github.com>
> > Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>  
> 
> I'm not clear to me if these are people, or LLMs.  We haven't devised
> a formal policy for AI assisted contributions, but for that reason
> amongst others I would certainly argue against using Co-authored-by as
> the way of indicating it.

By the way, the first one seems to be Richard himself, the second one is
a language model, but indeed, please don't use those tags, because it's
quite debatable whether google-labs-jules[bot] is actually an author.

Just add the specific information in plain English I would say.

> > Signed-off-by: Richard Lawrence <rlawrence@tamu.edu>
> > ---
> >  conf.c  |  9 +++++-
> >  passt.1 |  5 ++++
> >  util.c  | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
> >  3 files changed, 96 insertions(+), 11 deletions(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index 5b6cc2b..0032d02 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -732,7 +732,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 FDS	Comma-separated list of fds to pass to\n"
> > +		"			the spawned command\n");
> >  
> >  	passt_exit(status);
> >  }
> > @@ -1316,6 +1318,7 @@ void conf(struct ctx *c, int argc, char **argv)
> >  		{"stats", required_argument,		NULL,		31 },
> >  		{"conf-path",	required_argument,	NULL,		'c' },
> >  		{"chroot-fallback", no_argument,	NULL, 		32 },
> > +		{"pass-fds",	required_argument,	NULL,		33 },
> >  		{ 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:";
> > @@ -1557,6 +1560,10 @@ void conf(struct ctx *c, int argc, char **argv)
> >  		case 32:
> >  			c->chroot_fallback = true;
> >  			break;
> > +		case 33:
> > +			if (c->mode != MODE_PASTA)
> > +				die("--pass-fds is for pasta mode only");
> > +			break;
> >  		case 'd':
> >  			c->debug = 1;
> >  			c->quiet = 0;
> > diff --git a/passt.1 b/passt.1
> > index 1570f6a..93c9b24 100644
> > --- a/passt.1
> > +++ b/passt.1
> > @@ -755,6 +755,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 " " \fIfds\fR
> > +Pass a comma-separated list of file descriptors to the spawned command.
> > +These file descriptors will be kept open.
> > +
> >  .SH EXAMPLES
> >  
> >  .SS \fBpasta
> > diff --git a/util.c b/util.c
> > index 4bc5d6f..c73fd74 100644
> > --- a/util.c
> > +++ b/util.c
> > @@ -932,29 +932,102 @@ const char *str_ee_origin(const struct sock_extended_err *ee)
> >  void close_open_files(int argc, char **argv)
> >  {
> >  	const struct option optfd[] = { { "fd", required_argument, NULL, 'F' },
> > +					{ "pass-fds", required_argument, NULL, 33 },
> >  					{ 0 },
> >  				      };
> >  	long fd = -1;
> > -	int name, rc;
> > +	const char *pass_fds_arg = NULL;
> > +	int name, rc = 0;
> > +	int old_opterr = opterr;
> >  
> > +	opterr = 0;
> >  	do {
> >  		name = getopt_long(argc, argv, "-:F:", optfd, NULL);
> >  
> >  		if (name == 'F')
> >  			fd = conf_tap_fd(optarg);
> > +		else if (name == 33)
> > +			pass_fds_arg = optarg;
> >  	} while (name != -1);
> > +	opterr = old_opterr;
> >  
> > -	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);
> > +	#define MAX_PASS_FDS 1024
> > +	unsigned int keep_fds[MAX_PASS_FDS];
> > +	int keep_fds_cnt = 0;
> > +
> > +	if (fd != -1 && fd > STDERR_FILENO) {
> > +		keep_fds[keep_fds_cnt++] = (unsigned int)fd;
> > +	}
> > +
> > +	if (pass_fds_arg) {
> > +		const char *p = pass_fds_arg;
> > +		while (*p) {
> > +			char *endptr;
> > +			unsigned long val = strtoul(p, &endptr, 10);
> > +			if (p == endptr) {
> > +				die("Invalid --pass-fds option: %s", pass_fds_arg);
> > +			}
> > +			if (val > INT_MAX || val <= STDERR_FILENO) {
> > +				die("Invalid file descriptor in --pass-fds: %lu", val);
> > +			}
> > +			if (keep_fds_cnt >= MAX_PASS_FDS) {
> > +				die("Too many file descriptors in --pass-fds");
> > +			}
> > +			keep_fds[keep_fds_cnt++] = (unsigned int)val;
> > +			if (*endptr == ',') {
> > +				p = endptr + 1;
> > +				if (*p == '\0') {
> > +					die("Invalid --pass-fds option: trailing comma");
> > +				}
> > +			} else if (*endptr == '\0') {
> > +				p = endptr;
> > +			} else {
> > +				die("Invalid character in --pass-fds option: %s", pass_fds_arg);
> > +			}
> > +		}
> >  	}
> >  
> > +	/* Sort keep_fds in ascending order */
> > +	for (int i = 1; i < keep_fds_cnt; i++) {
> > +		unsigned int key = keep_fds[i];
> > +		int j = i - 1;
> > +		while (j >= 0 && keep_fds[j] > key) {
> > +			keep_fds[j + 1] = keep_fds[j];
> > +			j--;
> > +		}
> > +		keep_fds[j + 1] = key;
> > +	}
> > +
> > +	/* Remove duplicates */
> > +	int write_idx = 0;
> > +	for (int i = 0; i < keep_fds_cnt; i++) {
> > +		if (i == 0 || keep_fds[i] != keep_fds[i - 1]) {
> > +			keep_fds[write_idx++] = keep_fds[i];
> > +		}
> > +	}
> > +	keep_fds_cnt = write_idx;
> > +
> > +	/* Close ranges */
> > +	unsigned int current = STDERR_FILENO + 1;
> > +	for (int i = 0; i < keep_fds_cnt; i++) {
> > +		unsigned int f = keep_fds[i];
> > +		if (current < f) {
> > +			int call_rc = close_range(current, f - 1, CLOSE_RANGE_UNSHARE);
> > +			if (call_rc != 0)
> > +				rc = call_rc;
> > +		}
> > +		current = f + 1;
> > +	}
> > +	if (keep_fds_cnt == 0 || keep_fds[keep_fds_cnt - 1] < ~0U) {
> > +		int call_rc = close_range(current, ~0U, CLOSE_RANGE_UNSHARE);
> > +		if (call_rc != 0)
> > +			rc = call_rc;
> > +	}
> > +
> > +	/* Note: We assume that the parent process did not set FD_CLOEXEC on the
> > +	 * passed file descriptors, so we do not explicitly clear it here.
> > +	 */
> > +
> >  	if (rc) {
> >  		if (errno == ENOSYS || errno == EINVAL) {
> >  			/* This probably means close_range() or the
> > -- 
> > 2.52.0
> >   
>