diff --git a/crates/composefs/Cargo.toml b/crates/composefs/Cargo.toml index 8bd5c085..61be2192 100644 --- a/crates/composefs/Cargo.toml +++ b/crates/composefs/Cargo.toml @@ -29,6 +29,7 @@ tempfile = { version = "3.8.0", optional = true, default-features = false } xxhash-rust = { version = "0.8.2", default-features = false, features = ["xxh32"] } zerocopy = { version = "0.8.0", default-features = false, features = ["derive", "std"] } zstd = { version = "0.13.0", default-features = false } +rand = { version = "0.9.1", default-features = true } [dev-dependencies] insta = "1.42.2" diff --git a/crates/composefs/src/repository.rs b/crates/composefs/src/repository.rs index 7898d79d..f11a4904 100644 --- a/crates/composefs/src/repository.rs +++ b/crates/composefs/src/repository.rs @@ -12,8 +12,8 @@ use anyhow::{bail, ensure, Context, Result}; use once_cell::sync::OnceCell; use rustix::{ fs::{ - fdatasync, flock, linkat, mkdirat, open, openat, readlinkat, symlinkat, AtFlags, Dir, - FileType, FlockOperation, Mode, OFlags, CWD, + fdatasync, flock, linkat, mkdirat, open, openat, readlinkat, AtFlags, Dir, FileType, + FlockOperation, Mode, OFlags, CWD, }, io::{Errno, Result as ErrnoResult}, }; @@ -26,7 +26,7 @@ use crate::{ }, mount::{composefs_fsmount, mount_at}, splitstream::{DigestMap, SplitStreamReader, SplitStreamWriter}, - util::{proc_self_fd, Sha256Digest}, + util::{filter_errno, proc_self_fd, replace_symlinkat, Sha256Digest}, }; /// Call openat() on the named subdirectory of "dirfd", possibly creating it first. @@ -309,7 +309,7 @@ impl Repository { let stream_path = format!("streams/{}", hex::encode(sha256)); let object_id = writer.done()?; let object_path = Self::format_object_path(&object_id); - self.ensure_symlink(&stream_path, &object_path)?; + self.symlink(&stream_path, &object_path)?; if let Some(name) = reference { let reference_path = format!("streams/refs/{name}"); @@ -358,7 +358,7 @@ impl Repository { let object_id = writer.done()?; let object_path = Self::format_object_path(&object_id); - self.ensure_symlink(&stream_path, &object_path)?; + self.symlink(&stream_path, &object_path)?; object_id } }; @@ -379,9 +379,11 @@ impl Repository { let filename = format!("streams/{name}"); let file = File::from(if let Some(verity_hash) = verity { - self.open_with_verity(&filename, verity_hash)? + self.open_with_verity(&filename, verity_hash) + .with_context(|| format!("Opening ref 'streams/{name}'"))? } else { - self.openat(&filename, OFlags::RDONLY)? + self.openat(&filename, OFlags::RDONLY) + .with_context(|| format!("Opening ref 'streams/{name}'"))? }); SplitStreamReader::new(file) @@ -414,7 +416,7 @@ impl Repository { let object_path = Self::format_object_path(&object_id); let image_path = format!("images/{}", object_id.to_hex()); - self.ensure_symlink(&image_path, &object_path)?; + self.symlink(&image_path, &object_path)?; if let Some(reference) = name { let ref_path = format!("images/refs/{reference}"); @@ -434,7 +436,9 @@ impl Repository { /// Returns the fd of the image and whether or not verity should be /// enabled when mounting it. fn open_image(&self, name: &str) -> Result<(OwnedFd, bool)> { - let image = self.openat(&format!("images/{name}"), OFlags::RDONLY)?; + let image = self + .openat(&format!("images/{name}"), OFlags::RDONLY) + .with_context(|| format!("Opening ref 'images/{name}'"))?; if name.contains("/") { return Ok((image, true)); @@ -499,14 +503,8 @@ impl Repository { relative.push(target_component); } - symlinkat(relative, &self.repository, name) - } - - pub fn ensure_symlink>(&self, name: P, target: &str) -> ErrnoResult<()> { - self.symlink(name, target).or_else(|e| match e { - Errno::EXIST => Ok(()), - _ => Err(e), - }) + // Atomically replace existing symlink + replace_symlinkat(&relative, &self.repository, name) } fn read_symlink_hashvalue(dirfd: &OwnedFd, name: &CStr) -> Result { @@ -552,15 +550,28 @@ impl Repository { fn gc_category(&self, category: &str) -> Result> { let mut objects = HashSet::new(); - let category_fd = self.openat(category, OFlags::RDONLY | OFlags::DIRECTORY)?; + let Some(category_fd) = filter_errno( + self.openat(category, OFlags::RDONLY | OFlags::DIRECTORY), + Errno::NOENT, + ) + .context("Opening {category} dir in repository")? + else { + return Ok(objects); + }; - let refs = openat( - &category_fd, - "refs", - OFlags::RDONLY | OFlags::DIRECTORY, - Mode::empty(), - )?; - Self::walk_symlinkdir(refs, &mut objects)?; + if let Some(refs) = filter_errno( + openat( + &category_fd, + "refs", + OFlags::RDONLY | OFlags::DIRECTORY, + Mode::empty(), + ), + Errno::NOENT, + ) + .context("Opening {category}/refs dir in repository")? + { + Self::walk_symlinkdir(refs, &mut objects)?; + } for item in Dir::read_from(&category_fd)? { let entry = item?; diff --git a/crates/composefs/src/util.rs b/crates/composefs/src/util.rs index d3c2bee6..30c74f45 100644 --- a/crates/composefs/src/util.rs +++ b/crates/composefs/src/util.rs @@ -1,8 +1,17 @@ +use rand::{distr::Alphanumeric, Rng}; use std::{ io::{Error, ErrorKind, Read, Result}, - os::fd::{AsFd, AsRawFd}, + os::{ + fd::{AsFd, AsRawFd, OwnedFd}, + unix::ffi::OsStrExt, + }, + path::Path, }; +use rustix::{ + fs::{readlinkat, renameat, symlinkat, unlinkat, AtFlags}, + io::{Errno, Result as ErrnoResult}, +}; use tokio::io::{AsyncRead, AsyncReadExt}; /// Formats a string like "/proc/self/fd/3" for the given fd. This can be used to work with kernel @@ -97,6 +106,66 @@ pub fn parse_sha256(string: impl AsRef) -> Result { Ok(value) } +pub(crate) fn filter_errno( + result: rustix::io::Result, + ignored: Errno, +) -> ErrnoResult> { + match result { + Ok(result) => Ok(Some(result)), + Err(err) if err == ignored => Ok(None), + Err(err) => Err(err), + } +} + +fn generate_tmpname(prefix: &str) -> String { + let rand_string: String = rand::rng() + .sample_iter(&Alphanumeric) + .take(12) + .map(char::from) + .collect(); + format!("{}{}", prefix, rand_string) +} + +pub(crate) fn replace_symlinkat( + target: impl AsRef, + dirfd: &OwnedFd, + name: impl AsRef, +) -> ErrnoResult<()> { + let name = name.as_ref(); + let target = target.as_ref(); + + // Step 1: try to create the symlink + if filter_errno(symlinkat(target, dirfd, name), Errno::EXIST)?.is_some() { + return Ok(()); + }; + + // Step 2: the symlink already exists. Maybe it already has the correct target? + if let Some(current_target) = filter_errno(readlinkat(dirfd, name, []), Errno::NOENT)? { + if current_target.into_bytes() == target.as_os_str().as_bytes() { + return Ok(()); + } + } + + // Step 3: full atomic replace path + for _ in 0..16 { + let tmp_name = generate_tmpname(".symlink-"); + if filter_errno(symlinkat(target, dirfd, &tmp_name), Errno::EXIST)?.is_none() { + // This temporary filename already exists, try another + continue; + } + + match renameat(dirfd, &tmp_name, dirfd, name) { + Ok(_) => return Ok(()), + Err(e) => { + let _ = unlinkat(dirfd, tmp_name, AtFlags::empty()); + return Err(e); + } + } + } + + Err(Errno::EXIST) +} + #[cfg(test)] mod test { use similar_asserts::assert_eq;