Skip to content

Commit ed308b2

Browse files
committed
Use rustix for cmd extension
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).
1 parent 59a67f4 commit ed308b2

File tree

3 files changed

+16
-9
lines changed

3 files changed

+16
-9
lines changed

lib/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ indicatif = "0.16.0"
2626
once_cell = "1.9"
2727
libc = "0.2.92"
2828
nix = "0.23"
29+
rustix = "0.31.3"
2930
oci-spec = "0.5.0"
3031
openat = "0.1.20"
3132
openat-ext = "0.2.0"

lib/src/cmdext.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
1-
use std::os::unix::prelude::{CommandExt, RawFd};
1+
use rustix::fd::{FromRawFd, IntoRawFd};
2+
use rustix::io::OwnedFd;
3+
use std::os::unix::prelude::CommandExt;
4+
use std::sync::Arc;
25

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

911
#[allow(unsafe_code)]
1012
impl CommandRedirectionExt for std::process::Command {
11-
fn take_fd_n(&mut self, fd: i32, target: i32) -> &mut Self {
13+
fn take_fd_n(&mut self, fd: Arc<OwnedFd>, target: i32) -> &mut Self {
1214
unsafe {
1315
self.pre_exec(move || {
14-
nix::unistd::dup2(fd, target as RawFd)
15-
.map(|_r| ())
16-
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, format!("{}", e)))
16+
let target = rustix::io::OwnedFd::from_raw_fd(target);
17+
rustix::io::dup2(&*fd, &target)?;
18+
// Intentionally leak into the child.
19+
let _ = target.into_raw_fd();
20+
Ok(())
1721
});
1822
}
1923
self

lib/src/tar/write.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@ use anyhow::{anyhow, Context};
1313
use camino::{Utf8Component, Utf8Path, Utf8PathBuf};
1414
use ostree::gio;
1515
use ostree::prelude::FileExt;
16+
use rustix::fd::FromFd;
1617
use std::collections::BTreeMap;
1718
use std::convert::TryInto;
1819
use std::io::{BufWriter, Write};
19-
use std::os::unix::prelude::AsRawFd;
2020
use std::path::Path;
2121
use std::process::Stdio;
22+
use std::sync::Arc;
2223
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite};
2324
use tracing::instrument;
2425

@@ -197,13 +198,14 @@ pub async fn write_tar(
197198
};
198199
let mut c = std::process::Command::new("ostree");
199200
let repofd = repo.dfd_as_file()?;
201+
let repofd = Arc::new(rustix::io::OwnedFd::from_into_fd(repofd));
200202
{
201203
let c = c
202204
.stdin(Stdio::piped())
203205
.stdout(Stdio::piped())
204206
.stderr(Stdio::piped())
205207
.args(&["commit"]);
206-
c.take_fd_n(repofd.as_raw_fd(), 3);
208+
c.take_fd_n(repofd.clone(), 3);
207209
c.arg("--repo=/proc/self/fd/3");
208210
if let Some(sepolicy) = sepolicy.as_ref() {
209211
c.arg("--selinux-policy");

0 commit comments

Comments
 (0)