diff options
| -rw-r--r-- | src/builtins/command.rs | 2 | ||||
| -rw-r--r-- | src/pipeline/command.rs | 2 | ||||
| -rw-r--r-- | src/pipeline/mod.rs | 7 | ||||
| -rw-r--r-- | src/state/history/mod.rs | 6 |
4 files changed, 14 insertions, 3 deletions
diff --git a/src/builtins/command.rs b/src/builtins/command.rs index 2ea7f18..f3ec598 100644 --- a/src/builtins/command.rs +++ b/src/builtins/command.rs @@ -32,6 +32,8 @@ impl Command { self.io.set_stderr(fh); } + // Safety: see pre_exec in async_std::os::unix::process::CommandExt (this + // is just a wrapper) pub unsafe fn pre_exec<F>(&mut self, f: F) where F: 'static + FnMut() -> std::io::Result<()> + Send + Sync, diff --git a/src/pipeline/command.rs b/src/pipeline/command.rs index 408f845..2de1ff4 100644 --- a/src/pipeline/command.rs +++ b/src/pipeline/command.rs @@ -74,6 +74,8 @@ impl Command { } } + // Safety: see pre_exec in async_std::os::unix::process::CommandExt (this + // is just a wrapper) pub unsafe fn pre_exec<F>(&mut self, f: F) where F: 'static + FnMut() -> std::io::Result<()> + Send + Sync, diff --git a/src/pipeline/mod.rs b/src/pipeline/mod.rs index ba69f52..62e246f 100644 --- a/src/pipeline/mod.rs +++ b/src/pipeline/mod.rs @@ -16,6 +16,7 @@ mod command; pub use command::{Child, Command}; pub async fn run() -> anyhow::Result<i32> { + // Safety: we don't create File instances for fd 3 or 4 anywhere else let shell_read = unsafe { async_std::fs::File::from_raw_fd(3) }; let shell_write = unsafe { async_std::fs::File::from_raw_fd(4) }; @@ -234,9 +235,9 @@ async fn wait_children( fn pipe() -> anyhow::Result<(std::fs::File, std::fs::File)> { let (r, w) = nix::unistd::pipe2(nix::fcntl::OFlag::O_CLOEXEC)?; - // Safety: these file descriptors were just returned by pipe2 above, which - // means they must be valid otherwise that call would have returned an - // error + // Safety: these file descriptors were just returned by pipe2 above, and + // are only available in this function, so nothing else can be accessing + // them Ok((unsafe { std::fs::File::from_raw_fd(r) }, unsafe { std::fs::File::from_raw_fd(w) })) diff --git a/src/state/history/mod.rs b/src/state/history/mod.rs index 9f2d254..8b15916 100644 --- a/src/state/history/mod.rs +++ b/src/state/history/mod.rs @@ -622,6 +622,7 @@ async fn run_pipeline( nix::unistd::pipe2(nix::fcntl::OFlag::O_CLOEXEC).unwrap(); let (from_r, from_w) = nix::unistd::pipe2(nix::fcntl::OFlag::O_CLOEXEC).unwrap(); + // Safety: dup2 is an async-signal-safe function unsafe { cmd.pre_exec(move || { nix::unistd::dup2(to_r, 3)?; @@ -633,6 +634,8 @@ async fn run_pipeline( nix::unistd::close(to_r).unwrap(); nix::unistd::close(from_w).unwrap(); + // Safety: to_w was just opened above, was not used until now, and can't + // be used after this because we rebound the variable let mut to_w = unsafe { async_std::fs::File::from_raw_fd(to_w) }; to_w.write_all(&env.as_bytes()).await.unwrap(); drop(to_w); @@ -646,6 +649,9 @@ async fn run_pipeline( let read = async move { Res::Read( blocking::unblock(move || { + // Safety: from_r was just opened above and is only + // referenced in this closure, which takes ownership of it + // at the start and returns ownership of it at the end let fh = unsafe { std::fs::File::from_raw_fd(from_r) }; let event = bincode::deserialize_from(&fh); let _ = fh.into_raw_fd(); |