Update nix to v0.31.2 & associated API changes

No longer NO_FOLLOW_SYMLINK in linkat() in blit_element_item to prevent
EINVAL errors. This mirrors what was previously required for io-uring
and rustix experiments.

Avoid using deprecated flock API and adjust as neccessary

linkat now needs directory fds passed to it to more closly match the
syscall, adjust as neccessary.

No longer close() after entering a new container, this appears to no
longer be neccessary now we're using an &OwnedFd instead of i32.

Author: joebonrichie

Cargo.lock

@@ -46,7 +46,7 @@ libsqlite3-sys = { version = "0.35.0", features = ["bundled"] }
 log = "0.4.22"
 nc = "0.9.7"
 nom = "7.1.3"
-nix = { version = "0.27.1", features = [
+nix = { version = "0.31.1", features = [
     "user",
     "fs",
     "sched",

Cargo.toml

@@ -12,7 +12,11 @@ use std::{
 use fs_err as fs;
 use futures_util::{StreamExt, TryStreamExt, stream};
 use moss::{runtime, util};
-use nix::unistd::{LinkatFlags, linkat};
+use nix::{
+    fcntl::{AtFlags, OFlag, open},
+    sys::stat::Mode,
+    unistd::linkat,
+};
 use sha2::{Digest, Sha256};
 use stone_recipe::upstream;
 use thiserror::Error;
@@ -150,8 +154,14 @@ impl Installed {
             Installed::Plain { name, path, .. } => {
                 let target = dest_dir.join(name);
 
+                let old_parent = path.parent().unwrap_or(Path::new("."));
+                let new_parent = target.parent().unwrap_or(Path::new("."));
+
+                let old_dirfd = open(old_parent, OFlag::O_DIRECTORY | OFlag::O_CLOEXEC, Mode::empty())?;
+                let new_dirfd = open(new_parent, OFlag::O_DIRECTORY | OFlag::O_CLOEXEC, Mode::empty())?;
+
                 // Attempt hard link
-                let link_result = linkat(None, path, None, &target, LinkatFlags::NoSymlinkFollow);
+                let link_result = linkat(old_dirfd, path, new_dirfd, &target, AtFlags::AT_SYMLINK_NOFOLLOW);
 
                 // Copy instead
                 if link_result.is_err() {
@@ -544,4 +554,6 @@ pub enum Error {
     Io(#[from] io::Error),
     #[error("git")]
     Git(#[from] git::GitError),
+    #[error("nix")]
+    Nix(#[from] nix::errno::Errno),
 }

boulder/src/build/upstream.rs

@@ -2,8 +2,8 @@
 //
 // SPDX-License-Identifier: MPL-2.0
 
-use std::io;
-use std::os::fd::AsRawFd;
+use std::io::{self};
+use std::os::fd::OwnedFd;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::ptr::addr_of_mut;
@@ -24,7 +24,7 @@ use nix::sys::signal::{SaFlags, SigAction, SigHandler, Signal, kill, sigaction};
 use nix::sys::signalfd::SigSet;
 use nix::sys::stat::{Mode, umask};
 use nix::sys::wait::{WaitStatus, waitpid};
-use nix::unistd::{Pid, Uid, close, pipe, pivot_root, read, sethostname, tcsetpgrp, write};
+use nix::unistd::{Pid, Uid, pipe, pivot_root, read, sethostname, tcsetpgrp, write};
 use snafu::{ResultExt, Snafu};
 
 use self::idmap::idmap;
@@ -132,6 +132,7 @@ impl Container {
 
         // Pipe to synchronize parent & child
         let sync = pipe().context(NixSnafu)?;
+        let (read_fd, write_fd) = sync;
 
         let mut flags =
             CloneFlags::CLONE_NEWNS | CloneFlags::CLONE_NEWPID | CloneFlags::CLONE_NEWIPC | CloneFlags::CLONE_NEWUTS;
@@ -144,24 +145,26 @@ impl Container {
             flags |= CloneFlags::CLONE_NEWNET;
         }
 
-        let clone_cb = Box::new(|| match enter(&self, sync, &mut f) {
-            Ok(_) => 0,
-            // Write error back to parent process
-            Err(error) => {
-                let error = format_error(error);
-                let mut pos = 0;
+        let clone_cb = Box::new(|| {
+            let write_fd = write_fd.try_clone().unwrap();
 
-                while pos < error.len() {
-                    let Ok(len) = write(sync.1, &error.as_bytes()[pos..]) else {
-                        break;
-                    };
+            match enter(&self, &read_fd, &mut f) {
+                Ok(_) => 0,
+                Err(error) => {
+                    let error = format_error(error);
+                    let mut pos = 0;
 
-                    pos += len;
-                }
+                    while pos < error.len() {
+                        let Ok(len) = write(&write_fd, &error.as_bytes()[pos..]) else {
+                            break;
+                        };
+                        pos += len;
+                    }
 
-                let _ = close(sync.1);
+                    let _ = drop(write_fd);
 
-                1
+                    1
+                }
             }
         });
         let pid = unsafe { clone(clone_cb, &mut *addr_of_mut!(STACK), flags, Some(SIGCHLD)) }.context(NixSnafu)?;
@@ -172,9 +175,9 @@ impl Container {
         }
 
         // Allow child to continue
-        write(sync.1, &[Message::Continue as u8]).context(NixSnafu)?;
+        write(&write_fd, &[Message::Continue as u8]).context(NixSnafu)?;
         // Write no longer needed
-        close(sync.1).context(NixSnafu)?;
+        drop(write_fd);
 
         if self.ignore_host_sigint {
             ignore_sigint().context(NixSnafu)?;
@@ -193,7 +196,7 @@ impl Container {
                 let mut buffer = [0u8; 1024];
 
                 loop {
-                    let len = read(sync.0, &mut buffer).context(NixSnafu)?;
+                    let len = read(&read_fd, &mut buffer).context(NixSnafu)?;
 
                     if len == 0 {
                         break;
@@ -215,7 +218,7 @@ impl Container {
 }
 
 /// Reenter the container
-fn enter<E>(container: &Container, sync: (i32, i32), mut f: impl FnMut() -> Result<(), E>) -> Result<(), ContainerError>
+fn enter<E>(container: &Container, sync: &OwnedFd, mut f: impl FnMut() -> Result<(), E>) -> Result<(), ContainerError>
 where
     E: std::error::Error + Send + Sync + 'static,
 {
@@ -224,12 +227,9 @@ where
 
     // Wait for continue message
     let mut message = [0u8; 1];
-    read(sync.0, &mut message).context(ReadContinueMsgSnafu)?;
+    read(sync, &mut message).context(ReadContinueMsgSnafu)?;
     assert_eq!(message[0], Message::Continue as u8);
 
-    // Close unused read end
-    close(sync.0).context(CloseReadFdSnafu)?;
-
     setup(container)?;
 
     f().boxed().context(RunSnafu)
@@ -347,7 +347,7 @@ fn bind_mount(source: &Path, target: &Path, read_only: bool) -> Result<(), Conta
     unsafe {
         let inner = || {
             // Bind mount to fd
-            let fd = open_tree(AT_FDCWD, source, OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC).map_err(Errno::from_i32)?;
+            let fd = open_tree(AT_FDCWD, source, OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC).map_err(Errno::from_raw)?;
 
             // Set rd flag if applicable
             if read_only {
@@ -365,11 +365,11 @@ fn bind_mount(source: &Path, target: &Path, read_only: bool) -> Result<(), Conta
                     &attr as *const mount_attr_t as usize,
                     size_of::<mount_attr_t>(),
                 )
-                .map_err(Errno::from_i32)?;
+                .map_err(Errno::from_raw)?;
             }
 
             // Move detached mount to target
-            move_mount(fd, Path::new(""), AT_FDCWD, target, MOVE_MOUNT_F_EMPTY_PATH).map_err(Errno::from_i32)?;
+            move_mount(fd, Path::new(""), AT_FDCWD, target, MOVE_MOUNT_F_EMPTY_PATH).map_err(Errno::from_raw)?;
 
             Ok(())
         };
@@ -427,7 +427,7 @@ pub fn set_term_fg(pgid: Pid) -> Result<(), nix::Error> {
         )?
     };
     // Set term fg to pid
-    let res = tcsetpgrp(io::stdin().as_raw_fd(), pgid);
+    let res = tcsetpgrp(io::stdin(), pgid);
     // Set up old handler
     unsafe { sigaction(Signal::SIGTTOU, &prev_handler)? };
 
@@ -492,6 +492,8 @@ pub enum Error {
     // FIXME: Replace with more fine-grained variants
     #[snafu(display("nix"))]
     Nix { source: nix::Error },
+    #[snafu(display("io"))]
+    Io { source: io::Error },
 }
 
 #[derive(Debug, Snafu)]
@@ -508,8 +510,6 @@ enum ContainerError {
     SetPDeathSig { source: nix::Error },
     #[snafu(display("wait for continue message"))]
     ReadContinueMsg { source: nix::Error },
-    #[snafu(display("close read end of pipe"))]
-    CloseReadFd { source: nix::Error },
     #[snafu(display("sethostname"))]
     SetHostname { source: nix::Error },
     #[snafu(display("pivot_root"))]

crates/container/src/lib.rs

@@ -12,7 +12,7 @@
 use std::{
     borrow::Borrow,
     fmt, io,
-    os::{fd::RawFd, unix::fs::symlink},
+    os::{fd::OwnedFd, unix::fs::symlink},
     path::{Path, PathBuf},
     time::{Duration, Instant},
 };
@@ -23,7 +23,7 @@ use futures_util::{StreamExt, TryStreamExt, stream};
 use itertools::Itertools;
 use nix::{
     errno::Errno,
-    fcntl::{self, OFlag},
+    fcntl::{self, AtFlags, OFlag},
     libc::{AT_FDCWD, RENAME_EXCHANGE, SYS_renameat2, syscall},
     sys::stat::{Mode, fchmodat, mkdirat},
     unistd::{close, linkat, mkdir, symlinkat},
@@ -979,7 +979,7 @@ pub fn blit_root(installation: &Installation, tree: &vfs::Tree<PendingFile>, bli
                         .into_par_iter()
                         .map(|child| {
                             let _guard = current_span.enter();
-                            blit_element(root_dir, cache_fd, child, &progress)
+                            blit_element(&root_dir, &cache_fd, child, &progress)
                         })
                         .try_reduce(BlitStats::default, |a, b| Ok(a.merge(b)))?,
                 );
@@ -1010,8 +1010,8 @@ pub fn blit_root(installation: &Installation, tree: &vfs::Tree<PendingFile>, bli
 /// Care is taken to retain the directory file descriptor to avoid costly path
 /// resolution at runtime.
 fn blit_element(
-    parent: RawFd,
-    cache: RawFd,
+    parent: &OwnedFd,
+    cache: &OwnedFd,
     element: Element<'_, PendingFile>,
     progress: &ProgressBar,
 ) -> Result<BlitStats, Error> {
@@ -1047,7 +1047,7 @@ fn blit_element(
                     .into_par_iter()
                     .map(|child| {
                         let _guard = current_span.enter();
-                        blit_element(newdir, cache, child, progress)
+                        blit_element(&newdir, cache, child, progress)
                     })
                     .try_reduce(BlitStats::default, |a, b| Ok(a.merge(b)))?,
             );
@@ -1073,8 +1073,8 @@ fn blit_element(
 /// * `subpath` - the base name of the new inode
 /// * `item`    - New inode being recorded
 fn blit_element_item(
-    parent: RawFd,
-    cache: RawFd,
+    parent: &OwnedFd,
+    cache: &OwnedFd,
     subpath: &str,
     item: &PendingFile,
     stats: &mut BlitStats,
@@ -1105,17 +1105,11 @@ fn blit_element_item(
                 }
                 // Regular file
                 _ => {
-                    linkat(
-                        Some(cache),
-                        fp.to_str().unwrap(),
-                        Some(parent),
-                        subpath,
-                        nix::unistd::LinkatFlags::NoSymlinkFollow,
-                    )?;
+                    linkat(cache, fp.to_str().unwrap(), parent, subpath, AtFlags::empty())?;
 
                     // Fix permissions
                     fchmodat(
-                        Some(parent),
+                        parent,
                         subpath,
                         Mode::from_bits_truncate(item.layout.mode),
                         nix::sys::stat::FchmodatFlags::NoFollowSymlink,
@@ -1126,7 +1120,7 @@ fn blit_element_item(
             stats.num_files += 1;
         }
         StonePayloadLayoutFile::Symlink(source, _) => {
-            symlinkat(source.as_str(), Some(parent), subpath)?;
+            symlinkat(source.as_str(), parent, subpath)?;
             stats.num_symlinks += 1;
         }
         StonePayloadLayoutFile::Directory(_) => {