jobserver-rs icon indicating copy to clipboard operation
jobserver-rs copied to clipboard

Fix IO safety in `unix.rs`

Open beetrees opened this issue 7 months ago • 3 comments

As well as using BorrowedFd as opposed to c_int in more places, this PR fixes two related bugs:

  • Unix Client::configure() wouldn't ensure the file descriptors passed to Command::configure remained valid until the command was actually spawned. This meant that if the Client was dropped before the Command was spawned, the set_cloexec call could end up hitting an unrelated file descriptor (which the spawned process would then treat as a jobserver).
  • Unix string_arg uses the FDs from ClientCreationArg::Fds to put in the environment variables, but before this PR Client::configure would use the FDs from self.read/self.write instead. This would result in the wrong FDs getting passed to set_cloexec. This was basically harmless (apart from having the use-after-drop problem) as the only time the two sets of FDs differ is when the jobserver has been inherited from the environment, in which case the FDs are going to be not-CLOEXEC anyway.

I've also removed the unsafe from the definitions of Client::mk and Client::from_fds as they don't appear to have any safety requirements.

beetrees avatar Jul 21 '24 11:07 beetrees