diff --git a/lib/src/install.rs b/lib/src/install.rs index 9fbd368e3..9ea824909 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -28,6 +28,7 @@ use camino::Utf8Path; use camino::Utf8PathBuf; use cap_std::fs::{Dir, MetadataExt}; use cap_std_ext::cap_std; +use cap_std_ext::cap_std::fs::FileType; use cap_std_ext::cap_std::fs_utf8::DirEntry as DirEntryUtf8; use cap_std_ext::cap_tempfile::TempDir; use cap_std_ext::cmdext::CapStdExtCommandExt; @@ -56,7 +57,7 @@ use crate::progress_jsonl::ProgressWriter; use crate::spec::ImageReference; use crate::store::Storage; use crate::task::Task; -use crate::utils::sigpolicy_from_opts; +use crate::utils::{open_dir_noxdev, sigpolicy_from_opts}; /// The toplevel boot directory const BOOT: &str = "boot"; @@ -1558,14 +1559,22 @@ fn require_empty_rootdir(rootfs_fd: &Dir) -> Result<()> { } /// Remove all entries in a directory, but do not traverse across distinct devices. +/// If mount_err is true, then an error is returned if a mount point is found; +/// otherwise it is silently ignored. #[context("Removing entries (noxdev)")] -fn remove_all_in_dir_no_xdev(d: &Dir) -> Result<()> { - let parent_dev = d.dir_metadata()?.dev(); +fn remove_all_in_dir_no_xdev(d: &Dir, mount_err: bool) -> Result<()> { for entry in d.entries()? { let entry = entry?; - let entry_dev = entry.metadata()?.dev(); - if entry_dev == parent_dev { - d.remove_all_optional(entry.file_name())?; + let name = entry.file_name(); + let etype = entry.file_type()?; + if etype == FileType::dir() { + if let Some(subdir) = open_dir_noxdev(d, &name)? { + remove_all_in_dir_no_xdev(&subdir, mount_err)?; + } else if mount_err { + anyhow::bail!("Found unexpected mount point {name:?}"); + } + } else { + d.remove_file_optional(&name)?; } } anyhow::Ok(()) @@ -1576,13 +1585,15 @@ fn clean_boot_directories(rootfs: &Dir) -> Result<()> { let bootdir = crate::utils::open_dir_remount_rw(rootfs, BOOT.into()).context("Opening /boot")?; // This should not remove /boot/efi note. - remove_all_in_dir_no_xdev(&bootdir)?; + remove_all_in_dir_no_xdev(&bootdir, false)?; + // TODO: Discover the ESP the same way bootupd does it; we should also + // support not wiping the ESP. if ARCH_USES_EFI { if let Some(efidir) = bootdir .open_dir_optional(crate::bootloader::EFI_DIR) .context("Opening /boot/efi")? { - remove_all_in_dir_no_xdev(&efidir)?; + remove_all_in_dir_no_xdev(&efidir, false)?; } } Ok(()) @@ -1730,14 +1741,8 @@ pub(crate) async fn install_to_filesystem( Some(ReplaceMode::Wipe) => { let rootfs_fd = rootfs_fd.try_clone()?; println!("Wiping contents of root"); - tokio::task::spawn_blocking(move || { - for e in rootfs_fd.entries()? { - let e = e?; - rootfs_fd.remove_all_optional(e.file_name())?; - } - anyhow::Ok(()) - }) - .await??; + tokio::task::spawn_blocking(move || remove_all_in_dir_no_xdev(&rootfs_fd, true)) + .await??; } Some(ReplaceMode::Alongside) => clean_boot_directories(&rootfs_fd)?, None => require_empty_rootdir(&rootfs_fd)?, diff --git a/lib/src/lints.rs b/lib/src/lints.rs index 654b6d837..9c0942cbc 100644 --- a/lib/src/lints.rs +++ b/lib/src/lints.rs @@ -11,8 +11,6 @@ use cap_std_ext::cap_std; use cap_std_ext::dirext::CapStdExtDirExt as _; use fn_error_context::context; -use crate::utils::openat2_with_retry; - /// Reference to embedded default baseimage content that should exist. const BASEIMAGE_REF: &str = "usr/share/doc/bootc/baseimage/base"; @@ -72,25 +70,6 @@ fn check_kernel(root: &Dir) -> Result<()> { Ok(()) } -/// Open the target directory, but return Ok(None) if this would cross a mount point. -fn open_dir_noxdev( - parent: &Dir, - path: impl AsRef, -) -> std::io::Result> { - use rustix::fs::{Mode, OFlags, ResolveFlags}; - match openat2_with_retry( - parent, - path, - OFlags::CLOEXEC | OFlags::DIRECTORY | OFlags::NOFOLLOW, - Mode::empty(), - ResolveFlags::NO_XDEV | ResolveFlags::BENEATH, - ) { - Ok(r) => Ok(Some(Dir::reopen_dir(&r)?)), - Err(e) if e == rustix::io::Errno::XDEV => Ok(None), - Err(e) => return Err(e.into()), - } -} - fn check_utf8(dir: &Dir) -> Result<()> { for entry in dir.entries()? { let entry = entry?; @@ -109,7 +88,7 @@ fn check_utf8(dir: &Dir) -> Result<()> { "/{strname}: Found non-utf8 symlink target" ); } else if ifmt.is_dir() { - let Some(subdir) = open_dir_noxdev(dir, entry.file_name())? else { + let Some(subdir) = crate::utils::open_dir_noxdev(dir, entry.file_name())? else { continue; }; if let Err(err) = check_utf8(&subdir) { @@ -181,17 +160,6 @@ mod tests { Ok(tempdir) } - #[test] - fn test_open_noxdev() -> Result<()> { - let root = Dir::open_ambient_dir("/", cap_std::ambient_authority())?; - // This hard requires the host setup to have /usr/bin on the same filesystem as / - let usr = Dir::open_ambient_dir("/usr", cap_std::ambient_authority())?; - assert!(open_dir_noxdev(&usr, "bin").unwrap().is_some()); - // Requires a mounted /proc, but that also seems ane. - assert!(open_dir_noxdev(&root, "proc").unwrap().is_none()); - Ok(()) - } - #[test] fn test_var_run() -> Result<()> { let root = &fixture()?; diff --git a/lib/src/utils.rs b/lib/src/utils.rs index 42fbd1654..7bb3eccc7 100644 --- a/lib/src/utils.rs +++ b/lib/src/utils.rs @@ -110,6 +110,25 @@ pub(crate) fn open_dir_remount_rw(root: &Dir, target: &Utf8Path) -> Result root.open_dir(target).map_err(anyhow::Error::new) } +/// Open the target directory, but return Ok(None) if this would cross a mount point. +pub fn open_dir_noxdev( + parent: &Dir, + path: impl AsRef, +) -> std::io::Result> { + use rustix::fs::{Mode, OFlags, ResolveFlags}; + match openat2_with_retry( + parent, + path, + OFlags::CLOEXEC | OFlags::DIRECTORY | OFlags::NOFOLLOW, + Mode::empty(), + ResolveFlags::NO_XDEV | ResolveFlags::BENEATH, + ) { + Ok(r) => Ok(Some(Dir::reopen_dir(&r)?)), + Err(e) if e == rustix::io::Errno::XDEV => Ok(None), + Err(e) => return Err(e.into()), + } +} + /// Given a target path, remove its immutability if present #[context("Removing immutable flag from {target}")] pub(crate) fn remove_immutability(root: &Dir, target: &Utf8Path) -> Result<()> { @@ -223,6 +242,8 @@ pub(crate) fn digested_pullspec(image: &str, digest: &str) -> String { #[cfg(test)] mod tests { + use cap_std_ext::cap_std; + use super::*; #[test] @@ -269,4 +290,15 @@ mod tests { SignatureSource::ContainerPolicyAllowInsecure ); } + + #[test] + fn test_open_noxdev() -> Result<()> { + let root = Dir::open_ambient_dir("/", cap_std::ambient_authority())?; + // This hard requires the host setup to have /usr/bin on the same filesystem as / + let usr = Dir::open_ambient_dir("/usr", cap_std::ambient_authority())?; + assert!(open_dir_noxdev(&usr, "bin").unwrap().is_some()); + // Requires a mounted /proc, but that also seems ane. + assert!(open_dir_noxdev(&root, "proc").unwrap().is_none()); + Ok(()) + } }