A couple of ideas, rigorously not tested:
On Thu, 6 Aug 2026 13:20:27 +1000
David Gibson
On Mon, Aug 03, 2026 at 08:34:19PM +0000, Lawrence, Richard E wrote:
Howdy David,
Why enumerate inherited fds? Well, I first tried using close_range() from the tap fd up to max, as you suggested, and I found that it was closing file descriptors that passt had itself opened (like the log file), which is obviously not correct.
Oh. Right. Of course. Because this has moved later, we've now opened a bunch of stuff before it runs, meaning that closing everything no longer works.
I considered enumerating all the possible file descriptors that passt *might* have opened, and pass them to isolate_fds as fds_to_keep: it turns out that there are around 10 or so file descriptors that may or may not have been opened before the call to isolate_fds. I decided I didn't like that strategy, because it would create a maintenance burden:
Right, that doesn't seem practical.
...maybe we could make it slightly more practical by, either: 1. keeping all the files we open for configuration / logging / control path purposes (it's not many and they are clearly documented in struct ctx, unless I missed some) in a separate struct living inside ctx, maybe even as an array indexed by an enum. At that point, it would be clear what those files are, and if somebody forgets to use that struct or array in the future, they'll see their change presumably not working at all as we'll close whatever new file descriptor right away. So I'm not that worried by the maintenance burden because it looks rather fool-proof (this is regardless of the separate struct / array I'm proposing) 2. introducing a mandatory (or optional) wrapper around open(), which would open a file and store its descriptor number in an array, along with a wrapper for close() which would check that array and drop the item if it's present Otherwise:
every time someone tinkers with a file descriptor or reorders the startup sequence, they would have to remember to also check the list of file descriptors to keep. And in particular, some of the file descriptors are non-intuitive. Maybe not everyone realizes that a socket counts a file descriptor. What if there's a bug in someone's new feature where sockets are just disappearing without a trace? I would hate to waste someone's time chasing a subtle bug like that. So, I decided that enumerating fds_to_close at runtime time would be less of a maintenance burden than enumerating fds_to_keep in code.
I also considered reordering the startup so that forking happens before opening any files, so isolate fds can run with only the tap fd as its exclusion list, but I didn't see how to make that change without significant refactoring. It might end up being necessary, but I can't make that decision on my own. I don't understand the code well enough. Interested in talking that through?
Fair enough. My best guess is that this would be the best approach, but as you say it needs pretty in depth understanding of the code. And, as you've seen I've now guessed wrong several times about the best way to approach things.
Let me have a look into this and see if I can come up with something that actually works.
...if this is becoming too complicated, I would almost suggest adding a command line option to keep all files open *only* in the case where pasta spawns a command. I'm not enthusiastic about it, but given the complexity we risk introducing otherwise, I wouldn't find it outrageous either.
I don't understand your comment about conf_tap_fd becoming static. My implementation parses -F at the same time as the other arguments (not early anymore). Did you mean that you would inline the string to int conversion in conf()? That seems reasonable. I just felt that conf_tap_fd was overall too complex to inline in its current form.
I just meant that since it is now only used in conf.c, it can become local to that module, a static function in C terminology.
While relocating tap fd to 3+ could easily happen later, I still don't think it should happen during isolation, because it actually conceptually has nothing to do with isolation. It's about avoiding a corner case that would cause tap device chatter accidentally being printed to std err or something like that. Isolation should be focused on closing unused fds, not on managing fds that are in use. Perhaps we could relocate the tap fd to 3+ in main, around the same time that we are populating 0—2, for clarity (since those two steps need to coordinate).
I was not aware that dynamic memory would be off the table. My main reason for doing it that way is because I didn't want a very large array (max fds) to be always in memory even though most of the time it would be unused. I will have to re-think the strategy. Maybe there is another way to avoid a large unused array.
That array isn't particularly large compared to many others we already have. And even those are usually only a small fraction of our total effective memory usage - most of that comes in the form of kernel memory for sockets and buffers. In principle we could also use MADV_DONTNEED to discard the allocated memory once we're done with it.
Thank you for pointing out the flaw in my logic regarding the max open files limit. I will have to think harder about a correct way to close fds.
I do not believe that my patch disables your feature of populating fds 0—2 with devnull. That happens in main, not in isolate fds. My implementation of enumerating the inherited fds excludes 0—2.
You're right, sorry. I got muddled because it was inside isolate_fds() in a bunch of draft versions of my patches (I eventually realised that wouldn't quite work).
I hope to hear from you again soon Richard
PS. In case anyone else who's reading along has concerns about the potential performance hit caused by abandoning close_range(), I have an argument prepared to explain why that is not a serious concern.
So, fwiw, the concern isn't the cost of close() on the actually open files - as a one time cose that will generally be trivial. The conern is discovering the open fds: 2^31 close()s or other syscalls to discover if there's an fd there _would_ be too much. Using /proc/self/fd is an interesting approach. It's not portable, but then neither is close_range() (although FreeBSD does have close_range() apparently). Working out how to size things is the tricky bit with the /proc/self/fd approach, though.
[...]
-- Stefano