Skip to content

Commit

Permalink
Use rustix for cmd extension
Browse files Browse the repository at this point in the history
This is safer and may actually fix a race condition I've seen sometimes
in CI runs.  Part of investigating using rustix (and cap-std) in
our section of the ecosystem (xref rust-lang/rfcs#2610).
  • Loading branch information
cgwalters committed Jan 4, 2022
1 parent 2280d18 commit 89d3727
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 9 deletions.
1 change: 1 addition & 0 deletions lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ indicatif = "0.16.0"
once_cell = "1.9"
libc = "0.2.92"
nix = "0.23"
rustix = "0.31.3"
oci-spec = "0.5.0"
openat = "0.1.20"
openat-ext = "0.2.0"
Expand Down
18 changes: 11 additions & 7 deletions lib/src/cmdext.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
use std::os::unix::prelude::{CommandExt, RawFd};
use rustix::fd::{FromRawFd, IntoRawFd};
use rustix::io::OwnedFd;
use std::os::unix::prelude::CommandExt;
use std::sync::Arc;

pub(crate) trait CommandRedirectionExt {
/// Pass a file descriptor into the target process.
/// IMPORTANT: `fd` must be valid (i.e. cannot be closed) until after [`std::Process::Command::spawn`] or equivalent is invoked.
fn take_fd_n(&mut self, fd: i32, target: i32) -> &mut Self;
fn take_fd_n(&mut self, fd: Arc<OwnedFd>, target: i32) -> &mut Self;
}

#[allow(unsafe_code)]
impl CommandRedirectionExt for std::process::Command {
fn take_fd_n(&mut self, fd: i32, target: i32) -> &mut Self {
fn take_fd_n(&mut self, fd: Arc<OwnedFd>, target: i32) -> &mut Self {
unsafe {
self.pre_exec(move || {
nix::unistd::dup2(fd, target as RawFd)
.map(|_r| ())
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, format!("{}", e)))
let target = rustix::io::OwnedFd::from_raw_fd(target);
rustix::io::dup2(&*fd, &target)?;
// Intentionally leak into the child.
let _ = target.into_raw_fd();
Ok(())
});
}
self
Expand Down
6 changes: 4 additions & 2 deletions lib/src/tar/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ use anyhow::{anyhow, Context};
use camino::{Utf8Component, Utf8Path, Utf8PathBuf};
use ostree::gio;
use ostree::prelude::FileExt;
use rustix::fd::FromFd;
use std::collections::BTreeMap;
use std::convert::TryInto;
use std::io::{BufWriter, Write};
use std::os::unix::prelude::AsRawFd;
use std::path::Path;
use std::process::Stdio;
use std::sync::Arc;
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite};
use tracing::instrument;

Expand Down Expand Up @@ -197,13 +198,14 @@ pub async fn write_tar(
};
let mut c = std::process::Command::new("ostree");
let repofd = repo.dfd_as_file()?;
let repofd = Arc::new(rustix::io::OwnedFd::from_into_fd(repofd));
{
let c = c
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.args(&["commit"]);
c.take_fd_n(repofd.as_raw_fd(), 3);
c.take_fd_n(repofd.clone(), 3);
c.arg("--repo=/proc/self/fd/3");
if let Some(sepolicy) = sepolicy.as_ref() {
c.arg("--selinux-policy");
Expand Down

0 comments on commit 89d3727

Please sign in to comment.