Have rustix automatically set `CLOEXEC`?

[sunfishcode]

CLOEXEC is a per-fd flag that tells the OS to automatically close the fd at an execve-family call. This prevents fds from being implicitly leaked into child processes, which can otherwise cause surprising and even security-relevant bugs. I am considering changing rustix's stance to set CLOEXEC on fds it creates in safe APIs, and more broadly, proposing CLOEXEC to be considered a requirement for safety in Rust in general.

On some OS's, it's not always possible to set CLOEXEC atomically. As std does, we'd set it as soon as possible, and explain the gap by saying that those OS's behave as if execve sometimes has the side effect of attaching a debugger which executes some commands outside of Rust's semantics. That's not pretty, but ultimately it's a reflection of the underlying platform, so I think it's an acceptable tradeoff.

Rust's std already sets CLOEXEC on fds it creates, so this change wouldn't imply that any code in std needs to change. But I will submit a PR documenting this as an I/O safety guarantee in the std documentation (including a mention of the non-atomic situation).

If that PR is accepted, then I'd expect to make the following changes in rustix: openat and others would implicitly add the O_CLOEXEC flag, and OFlags::CLOEXEC would no longer exist. On OS's that need it, rustix would implicitly do additional system calls to set CLOEXEC in fds that can't have it set atomically. fcntl_setfd, dup etc. would become unsafe. fcntl_dupfd_cloexec would be renamed to fcntl_dupfd. And execveat would be made a public function in the process module (still unsafe, but only because of the char*'s). This will all require a semver bump.

I won't be able to do the actual work for this for a while, but I wanted to file this now in case anyone here has any feedback.

[notgull]

This sounds good. I think we also need to set WSA_FLAG_OVERLAPPED/WSA_FLAG_NO_INHERIT on Windows and NOSIGPIPE on macOS for similar reasons. I can implement this at some point.

[sunfishcode]

I've now submitted rust-lang/rust#107338 as a PR in the std documentation, to hopefully seek consensus on this before proceeding.

[joshtriplett]

Given a change like this, what usage pattern do you expect rustix users to use when they do want a file descriptor to be inherited?

[cgwalters]

Given a change like this, what usage pattern do you expect rustix users to use when they do want a file descriptor to be inherited?

A lot of related discussion in #100

(I think the tidbit that Rust's Command API does not default to closing all descriptors being different from e.g. Python and glib's has a lot of implicit effects in the ecosystem)

As that issue aludes to, right now there's an API for this in cap-std-ext - it'd likely make sense to lower this to rustix I think.

Ultimately I think the API should be expressed directly in terms of transferring ownership to another actor (like Command) and not just an ambient property of the FD (set/unset cloexec).

[sunfishcode]

Agreed. The base level is that rustix users would use fcntl_setfd or dup2, which would become unsafe. We may want to adjust how rustix exposes some of these things, but that's the basic idea.

The next level is something liike @cgwalters' take_fd_n, which allows users to pass types containing fds and have the library do the dup2/fcntls as part of doing the spawn. That eliminates the unsafe in user code in the parent.

In the fullness of time, I'd also like to explore the next level after that, eliminating the unsafe due to the from_raw_fd in user code in the child process too. This requires a convention between parent and child, so we may need to think of this as an entirely new API layer, that maybe also handles things like serializing/deserializing typed values across the parent/child boundary.

[sunfishcode]

In trying to put words to my intuition in rust-lang/rust#107338, I realized that it's not as clear as I thought. I've now closed that, and I no longer think I/O safety is the right mechanism to use for talking about O_CLOEXEC.

Another way of putting it is that I was concerned about child processes breaking encapsulation of libraries in the parent process, but I think the way to think about it is that if a fd exists without O_CLOEXEC, it's already exposed, as if there were a global set of fds and it was added to the set. A global set adds process awareness and isn't pretty, but ultimately this is how Unix works.

That said, we can still consider whether rustix should still take on the responsibility of automatically setting O_CLOEXEC. Is it worthwhile? I can see both sides.

[sunfishcode]

I think the answer is no, though I'm open to reopening this if anyone has a different perspective. Because not all operating systems support setting O_CLOEXEC atomically in all the places, rustix would have to decide between doing a separate syscall to set the O_CLOEXEC in some cases, or documenting that some platforms don't set it, and it feels like it would be too confusing.