I recently got into building a wayland client in rust on linux to get a feel for both linux windowing and rust IPC. I ended up getting stuck on step #1 which was to open a unix socket to a wayland compositor process because part of the requirement for properly doing that involved checking an optional environment variable and (potentially) using the int that would presumably be the value associated with it to acquire a file descriptor (see here). I aimed to do this in a safe (or at least sound way) to no avail. The end result of that thread was a link to the github issue I/O safety forbids the "pass FD via env var" pattern which details the issues with file descriptors and rust's safety model.
I've spent a bit of time trying to find crates that could actually perform file descriptor inheritance in a sound thread safe way and have come up empty. Some failing examples:
Wayland-rs creates an ownedfd from an env var containing an int then removes the environment variable in a thread unsafe way conn.rs:53, not only is this explicitly unsound but its also open to two threads getting owned instances of the same fd.
command-fds takes ownership of all fds at program start via a similar procedure and to add to it assumes a (non-portable) procfs interface inherited.rs:59. At least it has a safety comment though ._.
And, funny enough, if you use both of those crates at the same time there is no sound way to call into both of them because command-fds would grab all inherited fds as owned at program start and wayland-rs would then grab the inherited wayland connection as owned on init and at any time if one of those drop then the io guarantee for the other that owned fd's arent closed until drop is violated. This could easily be a much bigger issue if another fd was then opened that claimed the same raw fd number and the wayland connection started sending draw commands for user secrets to an untrusted file descriptor.
Question
The current API seems unsafe for two main reasons:
because inheriting file descriptors is both possible and frequent in all major operating systems, yet rusts' abstractions make the feature universally unsafe so either a developer must give up a fairly major part of I/O or they must break the guarantees that rust makes about file descriptors, and
try_clone exists and is a "safe" function. In other words two threads could get access to a single file description and modify it simultaneously which, while technically within the posix spec, would still result in race conditions if both tried to edit/seek/modify their separate "owned" instances at the same time.
is incomplete. The true underlying data structure is the file description (at least in the unix case idk whats going on with windows) and the abstraction that rust uses which treats the descriptor as the underlying resource is at odds with that. In this case it seems like a better abstraction would be something which exposes file descriptions as if descriptors were Arc's pointing to them, but thats perhaps a little over complicated and refactoring std::io is a bit off topic.
All this is to say, why? Why does rust attempt to make safety guarantees about file operations when those very same operations seem (at least to me) inherently unsafe? (I guess there could be a case to be made about ergonomics but if that's the reason then ergonomics cost me a week of learning the hard way )
PS: A max of 5 links in a post for new users seems a bit too restrictive but maybe thats just me
PPS: Thank you for the link limit bump
I've encountered the same problem with make job servers which also pass FDs via env vars.
IMO the kind of solution we need is to have something in std that grabs all open FDs at startup, and then libraries can then use std's APIs to claim particular FDs. That said, that only works well if everyone agrees to use that API, and that falls apart if Rust isn't called at startup, e.g. because Rust isn't used to implement main (like with SDL on some platforms e.g. Android).
Two different crates' unsafe functions for getting the FDs from the env vars:
Yeah this is a rather unsafe but unfortunately common pattern. It seems you already found the issue where we discuss this, including all the problems caused by that pattern.
The descriptor is the only thing we get from the OS so we don't have a choice in the matter. The fact that OwnedFd corresponds to Arc<_> means we are already modeling the description as good as we can -- the _ is a stand-in for the description type.
It is not clear which alternative you have in mind. If the alternative you are thinking of is "treat it as safe to perform arbitrary operations on arbitrary file descriptors given as integers": enforcing I/O safety has found multiple extremely subtle bugs in Rust code and in non-Rust code linked with Rust code -- bugs where libraries closed "random" FDs they had no right to close. I think it's working.
But sadly POSIX was designed with a C mindset of "just be careful", and without any consideration for local reasoning or explicit tracking of who's allowed to do what with which FD when. Ideally there'd be a way to mark a file description as "shared by the entire process, must not be closed"; the OS would enforce that and would also only allow this for FDs that have been present when the process got started. Then we could safely create BorrowedFd for such FDs. But with the APIs we have I am not sure what a safe treatment of global FDs could look like.
So I don't think Rust picked the wrong abstraction here. I think POSIX has a big gap here that makes applications prone to FD mismanagement, and Rust merely exposes this underlying flaw.
That would be great if POSIX has a portable way of doing this that's not terribly slow.
At least on most OSes we could register "life before main" hooks to initialize std. That does not work for dylibs but those are pretty unsafe anyway...
You have to consider that programs operate on a lot of exclusively owned resources, e.g. mmap'd database files. Interfering with those can lead to UB.
To be able to do those things safely the hand-wavy rule is "don't touch FDs you have no right to touch". IO-safety rules are a bit more formalized version of that.
For an inherited resource someone has to decide whether it's a single-owner that grabs it at program startup or a shared resource. Someone has to make that decision, Rust can't make it for you and the implicit IPC protocols are underspecified in that area, so the programmer has to make a choice. This results in the need for unsafe code where the programmer makes the choice and then has to ensure that it is upheld for the rest of the program.
try_clone exists and is a "safe" function. In other words two threads could get access to a single file description and modify it simultaneously which, while technically within the posix spec, would still result in race conditions if both tried to edit/seek/modify their separate "owned" instances at the same time.
You don't need try_clone for that, you can share a &File across threads and get concurrent access to the same fd.
This isn't unsafe and it's perfectly fine in many cases such as O_APPEND files, datagram sockets, eventfd, ...
because inheriting file descriptors is both possible and frequent in all major operating systems, yet rusts' abstractions make the feature universally unsafe so either a developer must give up a fairly major part of I/O or they must break the guarantees that rust makes about file descriptors, and
It's somewhat possible, but hard to get right. File descriptors are shared global program state. So all parts of the program have to agree on ownership.
And if you're coordinating through environment variables then the FD state needs to stay consistent with the environment variables.
So recommended patterns would be:
use unix domain sockets in the filesystem or abstract-namespace, then use SCM_RIGHTS for passing to a single receiver
or on linux:
pass as a path to /proc/<other process id>/fd/N instead of inheritance
use pidfd_getfd()
do the setup early during single-threaded main
add safety nets such as including dev/ino in the environment variable so that a child can crosscheck
Afaik systemd do this in some cases
use BorrowedFd<'static> instead of OwnedFd if it's a shared resource
in cases where you take ownership
if you set O_CLOEXEC also unset the environment variables
also use try_clone_to_owned/try_clone to renumber the Fd and close the original to make incorrect shared access more obvious
Life before main is also pretty fishy, all those hooks are run in effectively arbitrary order, which may have something else run before std can grab the FDs. also IIRC Chromium is intentionally removing as much Life-before-main stuff as they can, I vaguely recall seeing a bug where they were complaining that std had one of those for something and they wanted it removed.
Yeah all we can do is best-effort here without better OS support.
If they prefer an unsafe function to manually initialize this kind of state early on, that should also work, but it should not be at the detriment of everyone else who could benefit from doing this automatically.
I'm not sure I see the problem with having a small block of unsafe near the start of main to handle this case. If you need to clear environment variables after taking the FD (as was previously mentioned) there isn't any non-unsafe way to do it even.
In general it seems people are too scared of unsafe and the environment can never be made 100% safe. You need to acknowledge that some parts are just out of scope (see /proc/self/mem on Linux for example).
In a pure Rust binary I think it would be silly to require unsafe for this. At the very least, the pre-main code in libstd should run that initialization function. And then I guess we can unsafely expose it for cases where Rust is not in charge of main, if people are so uncomfortable with life-before-main linker tricks. (But note that std does use life-before-main already, for init_lse (whatever that is) and to initialize the global variable that backs env::args.)
And just to be clear about what the proposal would be: this initialization function would basically record which FDs are open at program startup (some OSes have an efficient way to do that, for others we'd scan the first N FDs for some not-too-big N). And then there would be two public safe functions:
One function to grab a BorrowedFd<'static> for an FD number. This also permanently marks that FD as "borrowed". FDs 0..3 get marked as borrowed immediately.
One function to grab an OwnedFd for an FD number. This errors if the FD is already borrowed, and also removes it from the list so it cannot be grabbed again.
I think it would be silly to require unsafe for this.
I have been in jobs where it would have been completely forbidden. Sure you could probably get an exemption but a single unsafe block can and will cause a lot of headaches.
this initialization function would basically record which FDs are open at program startup
I think the main issue here is that posix doesn’t specify a way to query the open file descriptors at program startup or otherwise. Sure a pre main function could scan the entire int32 range but that’s impractical and not doing that makes any security guarantee more like a security suggestion with caveats. Then there’s file descriptors coming in from an incompatible ffi boundary; anyone implementing code against that boundary would have to have some way to register their fd against the std registry which seems a bit to over complicated just to use a file descriptor.
I am well aware of all those issues, they have all been mentioned already in the issue you referenced in the OP. (Also mentioned there is that some OSes have APIs which are more efficient than scanning the entire int32 range. And the tradeoff of only scanning the first 1024 FDs or so, in the hopes that passed-in FDs are usually low. Not sure how realistic that hope is though.)
No better proposal has been made so far though, so maybe we should implement what we can rather than wait another 4 years in the hopes that a better solution magically appears.
Maybe I’m just reading it wrong, or maybe the naming is throwing me off but it seems like trying to implement an owned file description as an arc that can be easily cloned still pointing and allowing un mutexed access to the same resource is maybe a bit misleading.
It is not clear which alternative you have in mind.
What I was thinking when I wrote this was to have a more abstract (non clonable) description type that represents an actual file description but implements its functions (posix and platform) via references to a description which under the hood use an fd stored as a member of the description with libc/syscalls. References to this type could be thought of as a descriptor which would also bring the descriptions more inline with the rest of the languages’ sync primitives to arc+mutex a description to properly synchronize cursor movement/io across its references (at least for non inherited descriptions).
Inherited descriptions would require an unsafe call to a global registry which would allow
descriptions owned by user code would be able to blacklist acquisitions by the registry with hooks in their constructor and drop functions
if the rust registry knows about a description then it can manage access to it by handing out a arc mutex description rather than an un mutexed file descriptor (iirc descriptors can be compared with fstat to check equality of underlying descriptions so a request for an fd number that has a description matching something already tracked it could fold that new fd into the existing description somehow)
Obviously this is a bit hand wavy and I can try iterating on some code later to get something more concrete but I think in the interest of telling someone when they’re doing something that might cause a race condition it’s at least marginally better.
The Arc is supposed to represent the refcounting that happens in the kernel. duping the FD corresponds to cloning the Arc.
So if FileDescription is the internal kernel object, an "owned file descriptor" (OwnedFd) is pretty close to an Arc<FileDescription>, I would say?
Ah I think I see. Basically, a way to represent "I own this file descriptor and the refcount in the kernel is 1". Like a UniqueArc.
That could exist, and in fact I originally thought that's what OwnedFd is meant to be, but it turned out I was wrong and the libs team wanted OwnedFd to be allowed to have an arbitrary refcount. A UniqueFd type could be added if we thought it was useful, but so far a use hasn't really come up. In particular, note that if you as a library create your OwnedFd in a way that you know it is unique, and you never hand out borrows for it, you know that it's actually a UniqueFd and hence can do reasoning based on that. (IOW, UniqueFd could be written as a regular crate.)
I don't see how this would help with the "global FD sharing" problem though. You seem to suggest we should add Rust Mutex around these things, but I don't see which problems that would solve. The kernel already does refcounting and locking for us, it would be wasteful to duplicate that in userspace.
This isn't unsafe and it's perfectly fine in many cases such as O_APPEND files, datagram sockets, eventfd, ...
In certain cases yes unsynchronized access is fine and derived types that specifically represent things like datagram sockets can and should expose that kind of behavior for speed improvements but it's not necessarily safe for all file descriptors but std::io doesn't tell you that.
File descriptors are shared global program state.
This is part of my confusion on the ownership system. Conceptually to me fd's are essentially an array and in each spot of that array is a (conceptually potentially null but with none of the risks because of kernel guards) pointer to a kernel structure. Under that conceptualization the array is the shared resource and the descriptors are more like internal state (small nitpicking I know but it comes into play when you have a thread close and reopen the lowest sequentially numbered descriptor which basically swaps that descriptor for anyone still holding it). Under that idea the kernel does sync for the structure but everyone has ownership everywhere all of the time. Does rust even have a decent way of abstracting that under its type/safety system?
That slowdown doesn't seem warranted to me, I have written several tools where I optimise the total runtime of the entire binary (e.g. they are being called from prompt or input hooks in bash or zsh and might run on every single keystroke), to the point where it is s measurable speedup to mem::forget any heap allocations at the end and let the OS clean it up. I would not be happy with linear scanning, (or even extra syscalls to look in /proc/self/fd on Linux). I would for sure hope there is a way to opt out.
In Rust terms the table itself is owned by libc/the kernel, you're not allowed to manipulate it directly and need to go through sanctioned APIs to get entries in and out of the table.
Individual rust functions only gain ownership of slots in the table, for the lifetime that they own an OwnedFd, obtained through open, dup, recvmsg (scm_rights) and similar things that fill into an empty slot.
So any C functions that manipulate the whole file descriptor table, e.g. close_range(2) are highly unsafe and almost-always unsound.
but it's not necessarily safe for all file descriptors but std::io doesn't tell you that.
It is safe as Rust defines it. std doesn't provide a way to warn you about potential correctness issues, but there are crates that could help with that, such as positioned-io
You know that std contains a lot of unsafe? As does tokio, and loads of other dependencies you might use. It seems insane that a company would trust some random third party open source developer more than their own engineers.
Yes it does. It's called OwnedFd and BorrowedFd.
These are made-up abstractions of course, but so is Box.
We could have picked a different abstraction, and you seem to say that we should have, but it's unclear which problems that would have avoided.
So if FileDescription is the internal kernel object, an "owned file descriptor" (OwnedFd) is pretty close to an Arc<FileDescription>, I would say?
Yes! Except that you can directly operate on an Owned fd or any of its references in a mutable way without a mutex (this specific gripe with the current types is not really part of the inheritance problem but it was still something I was unsure of how to reason about)
Very similar yes! I think the actual type would need to be able to store all the kernel references (ie file descriptors pointing to the same description) in order to allow certain operations through ffi to work (think an ffi returning and later closing (and thus expecting) the same exact fd which it dup'd from an existing fd. again, nitpicking, but mentioning for completeness) and the creation of file descriptors would be a bit more controlled than OwnedFd::try_clone() currently allows.
I guess it would be more accurate that my problems with the current api are two fold,
I think given the difficulties around inherited fd's the api would better serve user code if it was more open about the specific safety caveats rather than providing "safe" functions that would allow two threads to mutate the same description unsynchronized, and the second would be that if the safety guarantees are relaxed on those points a registry for inherited fd's could be managed by the language in an incremental way rather than doing a full inventory at program start and hoping unsafe/ffi code doesn't interact poorly with that inventory.
std and tokio have way more eyes on correctness of the code than a single companies' small team working on a private project, of course they would trust that more than their own developers, not to mention the fact that if something does go wrong because of that unsafe block then the developer that actually committed it would be under scrutiny and nobody wants to be in that position. If the issue was a language problem then it gets much harder to assign blame to an individual let alone one within the company.
)
These are made-up abstractions of course, but so is