Howdy Stefano, I had sent another message to the PATCH v2 thread, but it seems you missed it. Copying inline below (>>) so you don't have to go hunting for it. ## Re: long option index.

...

It turns out the minimum index for a long option is 2, not 1. conf() already correctly does it that way.

(continued...) It's because long option index 1 is reserved for positional arguments or something like that (forgot the details). ## Re: relocating fd_tap conditionally

...

I think I misunderstood your question about unconditionally relocating the fd_tap. Please clarify: are you suggesting that we should skip relocating fd_tap when we also skip closing leaked file descriptors? That design would not cause any of the bugs I imagined. For example, it would never affect passt mode because skipping is forbidden in passt mode anyways. If the user does tap | pasta --fd 0 --pass-fds cmd then the fd_tap stays on stdin, so the spawned command gets it on stdin; that is technically a feature ("--pass-fds preserves file descriptors"), but I still think it's very weird behavior.

(continued...) But, given your more recent comments, I think I am still misunderstanding your question about unconditionally relocating fd_tap. So I need to ask a different question instead. Please clarify: is it ever OK to leave the fd_tap on 0? If we can leave fd_tap on 0 in pasta mode with pass-fds, then that greatly simpifies the logic. ## Re: dup2 vs fcntl Please allow me to clarify: the reason I switched from dup2 to fcntl is because I imagined a scenario where the user wants to both: 1. pass the tap fd to pasta on 0, and also 2. pass something else through to bwrap on 3. For example: ``` exec 3<<<"Hello from FD 3" && tap | pasta --fd 0 --pass-fds bwrap --bind-fd 3 /hello bash -c 'cat /hello | send-it-to-the-internet-or-something-idk' ``` In that scenario, using dup2 is a major problem. It closes the existing FD 3 and puts the tap there instead! So the user is now accidentally sending the tap to the internet instead of the data they intended to pass on 3. One possible solution is to leave the tap on 0 instead. (previously...)

...

If you do insist that this weird behavior is in fact the correct behavior, then we can demote the fcntl call back to plain dup2 with a comment that explains it is safe to close and reuse fd 3 since the user did not request to pass-fds. This comment is necessary to prevent a future bug if someone refactors the code carelessly (dup2 closes fd 3 when user wants to pass fd 3). Please let me know if that change is still what you want.

I call this the "unsafe" option. I can imagine that leaving the tap on 0 could cause bugs, and I imagine that leaving dup2 in the code could re-introduce the clobbering bug at a later time. ## New Proposal I propose to always relocate the tap above 2, to avoid bugs, but the system call we use can be conditional: * dup2 when we're closing fds (because it's the easiest) and * fcntl when we're leaving them open (because it's the safest) that seems like the best of both worlds. ## Re: style changes I agree with your suggested style changes. Richard Lawrence ________________________________ From: Stefano Brivio Sent: Wednesday, October 28, 2026 5:38 PM To: Lawrence, Richard E Cc: passt-dev@passt.top Subject: Re: [PATCH v3] pasta: Add --pass-fds option to skip closing file descriptors On Mon, 26 Oct 2026 12: 03: 45 -0500 Richard Lawrence wrote: > When pasta spawns a command (`pasta [options] -- [command]`), all open file > descriptors inherited from the parent process except standard streams ZjQcmQRYFpfptBannerStart This Message Is From an External Sender This message came from outside your organization. ZjQcmQRYFpfptBannerEnd On Mon, 26 Oct 2026 12:03:45 -0500 Richard Lawrence 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->pasta_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://urldefense.com/v3/__https://bugs.passt.top/show_bug.cgi?id=204__;!!K... Signed-off-by: Richard Lawrence --- conf.c | 56 +++++++++++++++++++++++++--------------- conf.h | 2 +- isolation.c | 73 +++++++++++++++++++++++++++++++++-------------------- isolation.h | 2 +- passt.1 | 5 ++++ passt.c | 2 +- passt.h | 2 ++ 7 files changed, 91 insertions(+), 51 deletions(-)

diff --git a/conf.c b/conf.c index 149a4c7..30edef9 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,41 @@ 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 + * Should: + * - Set the fd_tap and pasta_pass_fds values in the execution context

Those "Should:" comments are something specific in isolation.c where we have clearly defined subsequent steps. Here it doesn't make a lot of sense, the comment as you changed it above ("Read fds...") already makes this very clear. More comments means more to read, so they should be worth it.

*/ -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 },

Harmless, but still mildly confusing: why 2 instead of 1?

+ { 0 }, + }; int name;

+ c->fd_tap = -1; + c->pasta_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; /* May be overwritten in isolate_fd() */

Not just in isolate_fds(). Same consideration here: this comment doesn't really add much, so I'd rather drop it.

+ } else if (name == 2) { + c->pasta_pass_fds = true; + } + } while (name != -1); }

/** @@ -1317,6 +1324,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 +1575,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 +1616,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 +1942,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..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..766c297 100644 --- a/isolation.c +++ b/isolation.c @@ -250,46 +250,63 @@ 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; + conf_fds(c, argc, argv);

- fd = conf_tap_fd(argc, argv); + if (c->fd_tap >= 0 && c->fd_tap < STDERR_FILENO + 1) {

STDERR_FILENO + 1 used to be 'close_from' but now you're dropping that variable for some reason I didn't understand. However, this changes the behaviour (in a way that's rather unexpected as it's not mentioned in the commit message): instead of moving the --fd descriptor unconditionally, it's only done if the descriptor number is less than 3. Is this additional condition needed for Bubblewrap usage? If it is: - you could keep the simplification made possible by commit fd5b0807f5d0 ("isolation: Move --fd descriptor to a number of our choosing") in the general case, that is, move it unconditionally to 3, so that close_range() is simple - on --pass-fds, you don't need to call close_range(), so that simplification is not needed, and there's no need to move the file descriptor to 3 if it's already => 3 So this could simply be: /* Move the passed fd to a more convenient location (3): * - on --pass-fds, only if it might conflict with standard streams * - otherwise, unconditionally, to keep close_range() below simple */ if (c->fd_tap >= 0 && (!c->pass_fds || c->fd_tap < close_from)) { ...and all the rest would remain as is (except that 'fd' becomes 'c->fd_tap'), until:

+ /* Move the tap fd to a safer location */ + int new_fd = fcntl(c->fd_tap, F_DUPFD, STDERR_FILENO + 1);

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

...here, where we could simply return early on c->pass_fds (can we drop 'pasta' from there? I think it's redundant). Right?

- 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 unless --pass-fds is used */ + if (!c->pasta_pass_fds) { + int rc = 0; + /* Keep standard streams and the tap fd (if given) */ + int keep = c->fd_tap >= STDERR_FILENO ? c->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; } - }

- return fd; + /* 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"); + } + } + } }

/** diff --git a/isolation.h b/isolation.h index ec47038..fe8d7d5 100644 --- a/isolation.h +++ b/isolation.h @@ -11,7 +11,7 @@ #include

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

As I was suggesting on v2, quoting: --- Does it really need to have 'pasta' in the name? Sure, it's pasta-only, but this is already documented in man page, usage, and conf(), so we could probably save some typing by calling it 'pass_fds'. ---

* @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];

-- Stefano