From 2a5b41b7e028e6c614af9e2195f3ada6ded03d63 Mon Sep 17 00:00:00 2001 From: Joey Riches Date: Sun, 1 Mar 2026 15:10:02 +0000 Subject: [PATCH] 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. --- Cargo.lock | 25 +++++++++---------- Cargo.toml | 2 +- boulder/src/build/upstream.rs | 16 +++++++++++-- crates/container/src/lib.rs | 40 +++++++++++++------------------ moss/src/client/mod.rs | 28 +++++++++------------- moss/src/installation/lockfile.rs | 21 ++++++++-------- moss/src/util.rs | 20 +++++++++++++--- 7 files changed, 83 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d9dbf089..f04a51c27 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -282,7 +282,7 @@ dependencies = [ "itertools 0.14.0", "mailparse", "moss", - "nix 0.27.1", + "nix 0.31.2", "path-clean", "rapidfuzz", "rayon", @@ -576,7 +576,7 @@ version = "0.26.1" dependencies = [ "fs-err", "nc", - "nix 0.27.1", + "nix 0.31.2", "snafu", "strum", ] @@ -1107,9 +1107,9 @@ checksum = "28dd6caf6059519a65843af8fe2a3ae298b14b80179855aeb4adc2c1934ee619" [[package]] name = "fs-err" -version = "3.1.1" +version = "3.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "88d7be93788013f265201256d58f04936a8079ad5dc898743aa20525f503b683" +checksum = "73fde052dbfc920003cfd2c8e2c6e6d4cc7c1091538c3a24226cec0665ab08c0" dependencies = [ "autocfg", "tokio", @@ -1686,9 +1686,9 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" [[package]] name = "libc" -version = "0.2.175" +version = "0.2.182" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a82ae493e598baaea5209805c49bbf2ea7de956d50d7da0da1164f9c6d28543" +checksum = "6800badb6cb2082ffd7b6a67e6125bb39f18782f793520caee8cb8846be06112" [[package]] name = "libredox" @@ -1887,7 +1887,7 @@ dependencies = [ "kdl", "libsqlite3-sys", "log", - "nix 0.27.1", + "nix 0.31.2", "os-info", "rayon", "reqwest", @@ -1922,26 +1922,27 @@ dependencies = [ [[package]] name = "nix" -version = "0.27.1" +version = "0.30.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2eb04e9c688eff1c89d72b407f168cf79bb9e867a9d3323ed6c01519eb9cc053" +checksum = "74523f3a35e05aba87a1d978330aef40f67b0304ac79c1c00b294c9830543db6" dependencies = [ "bitflags", "cfg-if", + "cfg_aliases", "libc", + "memoffset", ] [[package]] name = "nix" -version = "0.30.1" +version = "0.31.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "74523f3a35e05aba87a1d978330aef40f67b0304ac79c1c00b294c9830543db6" +checksum = "5d6d0705320c1e6ba1d912b5e37cf18071b6c2e9b7fa8215a1e8a7651966f5d3" dependencies = [ "bitflags", "cfg-if", "cfg_aliases", "libc", - "memoffset", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 1d27ea987..e2fbbddc1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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", diff --git a/boulder/src/build/upstream.rs b/boulder/src/build/upstream.rs index b3ee6c532..4514567e3 100644 --- a/boulder/src/build/upstream.rs +++ b/boulder/src/build/upstream.rs @@ -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), } diff --git a/crates/container/src/lib.rs b/crates/container/src/lib.rs index f84b0e3c6..e8475e689 100644 --- a/crates/container/src/lib.rs +++ b/crates/container/src/lib.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,23 +145,19 @@ impl Container { flags |= CloneFlags::CLONE_NEWNET; } - let clone_cb = Box::new(|| match enter(&self, sync, &mut f) { + let clone_cb = Box::new(|| match enter(&self, &read_fd, &mut f) { Ok(_) => 0, - // Write error back to parent process Err(error) => { let error = format_error(error); let mut pos = 0; while pos < error.len() { - let Ok(len) = write(sync.1, &error.as_bytes()[pos..]) else { + let Ok(len) = write(&write_fd, &error.as_bytes()[pos..]) else { break; }; - pos += len; } - let _ = close(sync.1); - 1 } }); @@ -172,9 +169,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 +190,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 +212,7 @@ impl Container { } /// Reenter the container -fn enter(container: &Container, sync: (i32, i32), mut f: impl FnMut() -> Result<(), E>) -> Result<(), ContainerError> +fn enter(container: &Container, sync: &OwnedFd, mut f: impl FnMut() -> Result<(), E>) -> Result<(), ContainerError> where E: std::error::Error + Send + Sync + 'static, { @@ -224,12 +221,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 +341,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 +359,11 @@ fn bind_mount(source: &Path, target: &Path, read_only: bool) -> Result<(), Conta &attr as *const mount_attr_t as usize, size_of::(), ) - .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 +421,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 +486,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 +504,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"))] diff --git a/moss/src/client/mod.rs b/moss/src/client/mod.rs index 879e5654a..9a771eb9f 100644 --- a/moss/src/client/mod.rs +++ b/moss/src/client/mod.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, 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, 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 { @@ -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(_) => { diff --git a/moss/src/installation/lockfile.rs b/moss/src/installation/lockfile.rs index b740246e6..1b18c89fc 100644 --- a/moss/src/installation/lockfile.rs +++ b/moss/src/installation/lockfile.rs @@ -4,14 +4,13 @@ use std::{ fmt, + fs::File, io::{self}, - os::fd::AsRawFd, path::PathBuf, sync::Arc, }; -use fs_err::File; -use nix::fcntl::{FlockArg, flock}; +use nix::fcntl::{Flock, FlockArg}; use thiserror::Error; /// An acquired file lock guaranteeing exclusive access @@ -21,7 +20,7 @@ use thiserror::Error; /// of this ref counted lock are dropped. #[derive(Debug, Clone)] #[allow(unused)] -pub struct Lock(Arc); +pub struct Lock(Arc>); /// Acquires a file lock at the provided path. If the file is currently /// locked, `block_msg` will be displayed and the function will block @@ -33,14 +32,14 @@ pub fn acquire(path: impl Into, block_msg: impl fmt::Display) -> Result let file = File::options().create(true).write(true).truncate(false).open(path)?; - match flock(file.as_raw_fd(), FlockArg::LockExclusiveNonblock) { - Ok(_) => {} - Err(nix::errno::Errno::EWOULDBLOCK) => { + let file = match Flock::lock(file, FlockArg::LockExclusiveNonblock) { + Ok(lock) => lock, + Err((file, nix::errno::Errno::EWOULDBLOCK)) => { println!("{block_msg}"); - flock(file.as_raw_fd(), FlockArg::LockExclusive)?; + Flock::lock(file, FlockArg::LockExclusive).map_err(|(_, e)| e)? } - Err(e) => Err(e)?, - } + Err((_, e)) => return Err(e.into()), + }; Ok(Lock(Arc::new(file))) } @@ -50,5 +49,5 @@ pub enum Error { #[error("io")] Io(#[from] io::Error), #[error("obtaining exclusive file lock")] - Flock(#[from] nix::Error), + Flock(#[from] nix::errno::Errno), } diff --git a/moss/src/util.rs b/moss/src/util.rs index ce309921a..df760585a 100644 --- a/moss/src/util.rs +++ b/moss/src/util.rs @@ -5,13 +5,17 @@ use std::{ io::{self, Read, Seek, Write}, num::NonZeroUsize, - os::unix::fs::symlink, + os::{fd::AsFd, unix::fs::symlink}, path::{Path, PathBuf}, thread, }; use fs_err as fs; -use nix::unistd::{LinkatFlags, linkat}; +use nix::{ + fcntl::{AtFlags, OFlag, open}, + sys::stat::Mode, + unistd::linkat, +}; use sha2::{Digest, Sha256}; use stone::{StoneDecodedPayload, StoneReadError}; use url::Url; @@ -98,7 +102,17 @@ pub fn list_dirs(dir: &Path) -> io::Result> { pub fn hardlink_or_copy(from: &Path, to: &Path) -> io::Result<()> { // Attempt hard link - let link_result = linkat(None, from, None, to, LinkatFlags::NoSymlinkFollow); + let old_parent = from.parent().unwrap_or(Path::new(".")); + let new_parent = to.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())?; + let link_result = linkat( + old_dirfd.as_fd(), + from, + new_dirfd.as_fd(), + to, + AtFlags::AT_SYMLINK_NOFOLLOW, + ); // Copy instead if link_result.is_err() {