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>
Subject: Re: [PATCH] feat: Pass open files to child in pasta mode
Date: Fri, 31 Jul 2026 12:18:20 +0000	[thread overview]
Message-ID: <DM3PR11MB871373A3A2D6F1F07916FE5DDAC82@DM3PR11MB8713.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20260731060341.302560-1-rlawrence@tamu.edu>

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

Clang-tidy caught two problems in snapshot_initial_fds(). A new patch version will be sent.


  *
Unsafe atoi to be replaced with strtol
  *
Unsafe realloc result to be checked before assignment to arr, not after

________________________________
From: Richard Lawrence <rlawrence@tamu.edu>
Sent: Friday, July 31, 2026 1:03 AM
To: passt-dev@passt.top <passt-dev@passt.top>
Cc: Lawrence, Richard E <rlawrence@tamu.edu>
Subject: [PATCH] feat: Pass open files to child in pasta mode

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.

Changes:
- `conf_tap_fd()` is now called directly by `conf()`.
- The tap fd is relocated to a number at least 3, rather than exactly 3.
- `snapshot_initial_fds()` memorizes inherited fds early in startup.
- Inherited fds are discovered by reading from `/proc` if available.
- `isolate_fds()` is now called after `conf()` in `main()`.

Signed-off-by: Richard Lawrence <rlawrence@tamu.edu>
---
 conf.c      |  39 +++++++++-------
 conf.h      |   1 -
 isolation.c | 128 +++++++++++++++++++++++++++++++++++-----------------
 isolation.h |   9 +++-
 passt.c     |   7 ++-
 5 files changed, 122 insertions(+), 62 deletions(-)

diff --git a/conf.c b/conf.c
index faf2681..6bfdfb6 100644
--- a/conf.c
+++ b/conf.c
@@ -1169,26 +1169,20 @@ static void conf_sock_listen(const struct ctx *c)
 }

 /**
- * conf_tap_fd() - Read tap fd as supplied by -F command line option
- * @argc:      Argument count
- * @argv:      Command line options
+ * conf_tap_fd() - Read and relocate tap fd as supplied by -F command line option
+ * @fdarg:     String containing fd
+ *
+ * Should:
+ *  - move the --fd descriptor out of the range 0-2
  *
  * Return: fd number from --fd option, or -1 if not supplied
  */
-int conf_tap_fd(int argc, char **argv)
+int conf_tap_fd(const char *fdarg)
 {
-       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);
+       const char *p;
+       int new_fd;
+       int fd;

         if (!fdarg)
                 return -1;
@@ -1197,7 +1191,18 @@ int conf_tap_fd(int argc, char **argv)
         if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) || val > INT_MAX)
                 die("Invalid --fd: %s", fdarg);

-       return val;
+       fd = (int)val;
+       if (fd >= 0 && fd < 3) {
+               new_fd = fcntl(fd, F_DUPFD, 3);
+
+               if (new_fd < 0)
+                       die_perror("Could not relocate --fd descriptor");
+
+               close(fd);
+               fd = new_fd;
+       }
+
+       return fd;
 }

 /**
@@ -1625,7 +1630,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() */
+                       c->fd_tap = conf_tap_fd(optarg);
                         c->one_off = true;
                         *c->sock_path = 0;
                         break;
diff --git a/conf.h b/conf.h
index 19bf9bc..16f9718 100644
--- a/conf.h
+++ b/conf.h
@@ -7,7 +7,6 @@
 #define CONF_H

 enum passt_modes conf_mode(int argc, char *argv[]);
-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);
diff --git a/isolation.c b/isolation.c
index 94cbe7f..bedc4ce 100644
--- a/isolation.c
+++ b/isolation.c
@@ -24,19 +24,13 @@
  * done anything we need to do with those resources, so we have
  * multiple stages of self-isolation.  In order these are:
  *
- * 1a. isolate_initial()
+ * 1. 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).
  *
- * 1b. isolate_fds()
- * ================
- *
- * Executed immediately after isolate_initial().  Closes any leaked
- * files we might have inherited from the parent process.
- *
  * 2. isolate_user()
  * =================
  *
@@ -44,14 +38,20 @@
  * operate in.  Sets our final UID & GID, and enters the correct user
  * namespace.
  *
- * 3. isolate_prefork()
+ * 3. isolate_fds()
+ * ================
+ *
+ * Executed after conf().  Closes any leaked
+ * files we might have inherited from the parent process.
+ *
+ * 4. isolate_prefork()
  * ====================
  *
  * Executed after all setup, but before daemonising (fork()ing into
  * the background).  Uses mount namespace and pivot_root() to remove
  * our access to the filesystem.
  *
- * 4. isolate_postfork()
+ * 5. isolate_postfork()
  * =====================
  *
  * Executed immediately after daemonizing, but before entering the
@@ -61,6 +61,7 @@
  * runtime operation.
  */

+#include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <grp.h>
@@ -77,6 +78,7 @@
 #include <unistd.h>
 #include <sys/mount.h>
 #include <sys/prctl.h>
+#include <sys/resource.h>
 #include <sys/socket.h>
 #include <sys/syscall.h>
 #include <sys/types.h>
@@ -248,47 +250,89 @@ void isolate_initial(void)
         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
+/**
+ * snapshot_initial_fds() - Snapshot initial file descriptors inherited from parent
+ * other than standard streams (stdin, stdout, stderr)
+ * @ifds:      Snapshot struct of initial file descriptors to populate
  */
-int isolate_fds(int argc, char **argv)
+void snapshot_initial_fds(struct initial_fd_snapshot *ifds)
 {
-       int fd, close_from = STDERR_FILENO + 1;
+       struct dirent *entry;
+       size_t capacity = 16;
+       int max_fd = 1024;
+       struct rlimit rl;
+       int dir_fd;
+       DIR *dir;
+       int fd;
+
+       ifds->arr = NULL;
+       ifds->count = 0;
+
+       dir = opendir("/proc/self/fd");
+       if (dir) {
+               dir_fd = dirfd(dir);
+               ifds->arr = malloc(sizeof(int) * capacity);
+               if (!ifds->arr)
+                       die_perror("Failed to allocate memory for inherited fds");
+
+               while ((entry = readdir(dir)) != NULL) {
+                       if (entry->d_name[0] < '0' || entry->d_name[0] > '9')
+                               continue;
+
+                       fd = atoi(entry->d_name);
+                       /* Ignore stdin/stdout/stderr and the opendir handle itself */
+                       if (fd > STDERR_FILENO && fd != dir_fd) {
+                               if (ifds->count >= capacity) {
+                                       capacity *= 2;
+                                       ifds->arr = realloc(ifds->arr,
+                                                           sizeof(int) * capacity);
+                                       if (!ifds->arr)
+                                               die_perror("Failed to reallocate memory for inherited fds");
+                               }
+                               ifds->arr[ifds->count++] = fd;
+                       }
+               }
+               closedir(dir);
+               return;
+       }
+
+       /* Fallback for environments without /proc (e.g. minimal chroots) */
+       if (getrlimit(RLIMIT_NOFILE, &rl) == 0 && rl.rlim_cur != RLIM_INFINITY)
+               max_fd = (int)rl.rlim_cur;

-       fd = conf_tap_fd(argc, argv);
+       ifds->arr = malloc(sizeof(int) * max_fd);
+       if (!ifds->arr)
+               die_perror("Failed to allocate memory for inherited fds fallback");

-       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++;
+       for (fd = STDERR_FILENO + 1; fd < max_fd; fd++) {
+               if (fcntl(fd, F_GETFD) >= 0)
+                       ifds->arr[ifds->count++] = fd;
         }
+}

-       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");
-               }
+/**
+ * isolate_fds() - Close leaked files from the parent process
+ * @ifds:      Snapshot of initial file descriptors
+ * @keep_fd:   File descriptor to keep open, if any
+ *
+ * Should:
+ *  - close all file descriptors that were open at startup, except for keep_fd
+ */
+void isolate_fds(struct initial_fd_snapshot *ifds, int keep_fd)
+{
+       size_t i;
+
+       if (!ifds || !ifds->arr)
+               return;
+
+       for (i = 0; i < ifds->count; i++) {
+               if (ifds->arr[i] != keep_fd)
+                       close(ifds->arr[i]);
         }

-       return fd;
+       free(ifds->arr);
+       ifds->arr = NULL;
+       ifds->count = 0;
 }

 /**
diff --git a/isolation.h b/isolation.h
index ec47038..8ada988 100644
--- a/isolation.h
+++ b/isolation.h
@@ -8,10 +8,17 @@
 #define ISOLATION_H

 #include <stdbool.h>
+#include <stddef.h>
 #include <unistd.h>

+struct initial_fd_snapshot {
+       int *arr;
+       size_t count;
+};
+
 void isolate_initial(void);
-int isolate_fds(int argc, char **argv);
+void snapshot_initial_fds(struct initial_fd_snapshot *ifds);
+void isolate_fds(struct initial_fd_snapshot *ifds, int keep_fd);
 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.c b/passt.c
index 5054551..c4335bf 100644
--- a/passt.c
+++ b/passt.c
@@ -335,6 +335,7 @@ int main(int argc, char **argv)
         struct epoll_event events[NUM_EPOLL_EVENTS];
         int nfds, devnull_fd = -1, fd;
         struct ctx *c = &passt_ctx;
+       struct initial_fd_snapshot ifds;
         struct rlimit limit;
         struct timespec now;
         struct sigaction sa;
@@ -344,8 +345,9 @@ int main(int argc, char **argv)

         arch_avx2_exec(argv);

+       snapshot_initial_fds(&ifds);
+
         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");
@@ -390,6 +392,9 @@ int main(int argc, char **argv)
         sock_probe_features(c);

         conf(c, argc, argv);
+
+       isolate_fds(&ifds, c->fd_tap);
+
         trace_init(c->trace);

         pasta_netns_quit_init(c);
--
2.52.0


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

      reply	other threads:[~2026-07-31 12:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-31  6:03 Richard Lawrence
2026-07-31 12:18 ` 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=DM3PR11MB871373A3A2D6F1F07916FE5DDAC82@DM3PR11MB8713.namprd11.prod.outlook.com \
    --to=rlawrence@tamu.edu \
    --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).