public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: "Lawrence, Richard E" <rlawrence@tamu.edu>
To: "passt-dev@passt.top" <passt-dev@passt.top>
Cc: "jbash@jbash.com" <jbash@jbash.com>
Subject: Re: [PATCH v2] feat: Add cli option '--pass-fds' for pasta mode.
Date: Sun, 19 Jul 2026 19:14:00 +0000	[thread overview]
Message-ID: <DM3PR11MB87131A9BAF123969BC0744F0DAC42@DM3PR11MB8713.namprd11.prod.outlook.com> (raw)
In-Reply-To: <DM3PR11MB871351EF40D8980F4A54F8B7DAC52@DM3PR11MB8713.namprd11.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 10906 bytes --]

Nitpicks:

  *
Literal 1024 used as the max pass-fds is magic. It should either be dynamic, or defined as a preprocessor directive.
  *
It is at least computationally awkward and possibly even unsafe to parse fd and pass-fds independently from the rest of the args. If the arg structure is ever extended to support an unordered mix of keyword and positional arguments, then these independent implementations become a maintenance burden.
  *
In conf_pass_fds(), the die statement only includes the particular substring that triggered the failure, which could result in a misleading or unhelpful error message.

________________________________
From: Lawrence, Richard E <rlawrence@tamu.edu>
Sent: Saturday, July 18, 2026 11:52 AM
To: passt-dev@passt.top <passt-dev@passt.top>
Cc: jbash@jbash.com <jbash@jbash.com>
Subject: Re: [PATCH v2] feat: Add cli option '--pass-fds' for pasta mode.

Nitpicks:

  *
we use STDERR_FILENO to represent 2, so the target of fcntl should be STDERR_FILENO + 1 rather than literal 3.
  *
I think it would be clearer if prev_fd was declared but not initially defined, and then set to STDERR_FILENO just before the loop, so that a comment like /* keep standard streams */ would not be among a bunch of variable declarations, but would be located near where the variable is first used.
  *
I think it would be clearer if min_fd was named next_fd by analogy to prev_fd.

________________________________
From: Richard Lawrence <rlawrence@tamu.edu>
Sent: Saturday, July 18, 2026 11:14 AM
To: passt-dev@passt.top <passt-dev@passt.top>
Cc: jbash@jbash.com <jbash@jbash.com>; Lawrence, Richard E <rlawrence@tamu.edu>; Lawrence, Richard E <rlawrence@tamu.edu>
Subject: [PATCH v2] feat: Add cli option '--pass-fds' for pasta mode.

From: Richard Lawrence <rarensu@tamu.edu>

When pasta mode is used to launch an executable (`pasta [COMMAND]`) and that executable accepts inputs in the form of arbitrary file descriptors (such as `bwrap`), then passt should not stand in the way of the parent process handing off those file descriptors to the child process. See bug 204 for additional discussion.

The `pass-fds` option accepts a comma-separated list of file
descriptor numbers. `conf_pass_fds()` parses the command line argument,  then `isolate_fds()` skips closing the specified fds by calling
`close_range()` on the gaps between them.

Additionally, the tap fd is safely relocated to the lowest unused fd number which it at least 3, to avoid accidentally overwriting an existing fd.

Signed-off-by: Richard Lawrence <rlawrence@tamu.edu>
---
 conf.c      | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 conf.h      |  1 +
 isolation.c | 68 ++++++++++++++++++++++++++++++++++++++++++-----------
 passt.1     |  5 ++++
 4 files changed, 125 insertions(+), 15 deletions(-)

diff --git a/conf.c b/conf.c
index 0fcba5c..3246735 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);
 }
@@ -1183,6 +1185,63 @@ int conf_tap_fd(int argc, char **argv)
         return val;
 }

+/**
+ * conf_pass_fds() - Read fds as supplied by --pass-fds command line option
+ * @argc:      Argument count
+ * @argv:      Command line options
+ * @fds:       Array where we store the parsed fds
+ * @max_fds:   Maximum size of the array
+ *
+ * Return: number of parsed fds, or -1 if option not specified
+ */
+int conf_pass_fds(int argc, char **argv, int *fds, int max_fds)
+{
+       const struct option opt[] = { { "pass-fds", required_argument, NULL, 33 },
+                                     { 0 }, };
+       const char *fdsarg = NULL;
+       int name, fds_cnt = 0;
+       int old_opterr;
+
+       old_opterr = opterr;
+       opterr = 0;
+       optind = 0;
+       do {
+               name = getopt_long(argc, argv, "-:", opt, NULL);
+               if (name == 33)
+                       fdsarg = optarg;
+       } while (name != -1);
+       opterr = old_opterr;
+
+       if (!fdsarg)
+               return -1;
+
+       while (*fdsarg) {
+               unsigned long val;
+               char *endptr;
+
+               val = strtoul(fdsarg, &endptr, 10);
+               if (fdsarg == endptr)
+                       die("Invalid --pass-fds option: %s", fdsarg);
+
+               if (val > INT_MAX)
+                       die("Invalid file descriptor in --pass-fds: %lu", val);
+
+               if (fds_cnt >= max_fds)
+                       die("Too many file descriptors in --pass-fds");
+
+               fds[fds_cnt++] = (int)val;
+
+               if (*endptr == ',')
+                       fdsarg = endptr + 1;
+               else if (*endptr == '\0')
+                       fdsarg = endptr;
+               else
+                       die("Invalid character in --pass-fds option: %s", fdsarg);
+       }
+
+       return fds_cnt;
+}
+
 /**
  * conf_addr() - Configure guest address with -a option
  * @c:         Execution context
@@ -1331,6 +1390,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:";
@@ -1572,6 +1632,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/conf.h b/conf.h
index 19bf9bc..3489f35 100644
--- a/conf.h
+++ b/conf.h
@@ -7,6 +7,7 @@
 #define CONF_H

 enum passt_modes conf_mode(int argc, char *argv[]);
+int conf_pass_fds(int argc, char **argv, int *fds, int max_fds);
 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);
diff --git a/isolation.c b/isolation.c
index 94cbe7f..0632f7f 100644
--- a/isolation.c
+++ b/isolation.c
@@ -249,32 +249,72 @@ void isolate_initial(void)
 }

 /*
- * isolate_fds() - Close leaked files, but not --fd, stdin, stdout, stderr
+ * isolate_fds() - Close leaked files, but not --fd, --pass-fds, standard streams
  * @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, --fd, and --pass-fds
  *  - 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;
+       int prev_fd = STDERR_FILENO; // Keep standard streams
+       int fds[1024 + 1];
+       int fds_cnt = 0;
+       int tap_fd;
+       int rc = 0;
+
+       tap_fd = conf_tap_fd(argc, argv);
+       if (tap_fd >= 0 && tap_fd < 3) {
+               /* Move the passed fd to a more convenient location */
+               int new_fd = fcntl(tap_fd, F_DUPFD, 3);
+
+               if (new_fd < 0)
+                       die_perror("Could not relocate --fd descriptor");

-       fd = conf_tap_fd(argc, argv);
+               close(tap_fd);
+               tap_fd = new_fd;
+       }

-       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 (tap_fd >= 0)
+               /* Keep the tap fd */
+               fds[fds_cnt++] = tap_fd;
+
+       rc = conf_pass_fds(argc, argv, fds + fds_cnt, 1024);
+       if (rc > 0)
+               /* Keep the pass-fds */
+               fds_cnt += rc;
+
+       rc = 0;
+
+       while (1) {
+               int min_fd = -1;
+
+               /* Find the next-lowest fd to keep */
+               for (int i = 0; i < fds_cnt; i++) {
+                       if (fds[i] > prev_fd && (min_fd == -1 || fds[i] < min_fd))
+                               min_fd = fds[i];
+               }
+
+               if (min_fd == -1)
+                       break;
+
+               if (min_fd > prev_fd + 1) {
+                       /* Close fds between two kept fds */
+                       if (close_range(prev_fd + 1, min_fd - 1, CLOSE_RANGE_UNSHARE))
+                               rc = -1;
+               }
+               prev_fd = min_fd;
         }
+
+       /* Close all other fds */
+       if (close_range(prev_fd + 1, ~0U, CLOSE_RANGE_UNSHARE))
+               rc = -1;

-       if (close_range(close_from, ~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
@@ -288,7 +328,7 @@ int isolate_fds(int argc, char **argv)
                 }
         }

-       return fd;
+       return tap_fd;
 }

 /**
diff --git a/passt.1 b/passt.1
index 995590a..bcd3aff 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
--
2.52.0


[-- Attachment #2: Type: text/html, Size: 25379 bytes --]

      reply	other threads:[~2026-07-19 19:14 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-18 16:14 Richard Lawrence
2026-07-18 16:52 ` Lawrence, Richard E
2026-07-19 19:14   ` Lawrence, Richard E [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM3PR11MB87131A9BAF123969BC0744F0DAC42@DM3PR11MB8713.namprd11.prod.outlook.com \
    --to=rlawrence@tamu.edu \
    --cc=jbash@jbash.com \
    --cc=passt-dev@passt.top \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).