From 1c30403e96e800a4c2240f894d0842b7b1dfacbd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 2 May 2025 08:37:15 -0400 Subject: [PATCH] install: Do a dynamic mount for /var/tmp Closes: https://github.com/bootc-dev/bootc/issues/1292 Basically we were doing the `/proc/1/root/var/tmp` trick for `/var/tmp` because we didn't have the dynamic bind mount infrastructure before. Now we do, so use it instead. The specific motivation is that Go in some cases uses `EvalSymlinks` which gets confused by the `/proc//root` magic links. Also, this deletes a lot of code. Signed-off-by: Colin Walters --- lib/src/install.rs | 68 +++++++++------------------------------------- 1 file changed, 13 insertions(+), 55 deletions(-) diff --git a/lib/src/install.rs b/lib/src/install.rs index 9329b8552..395293c19 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -1103,22 +1103,11 @@ fn require_host_userns() -> Result<()> { Ok(()) } -// Ensure the `/var` directory exists. -fn ensure_var() -> Result<()> { - std::fs::create_dir_all("/var")?; - Ok(()) -} - -/// We want to have proper /tmp and /var/tmp without requiring the caller to set them up -/// in advance by manually specifying them via `podman run -v /tmp:/tmp` etc. -/// Unfortunately, it's quite complex right now to "gracefully" dynamically reconfigure -/// the mount setup for a container. See https://brauner.io/2023/02/28/mounting-into-mount-namespaces.html -/// So the brutal hack we do here is to rely on the fact that we're running in the host -/// pid namespace, and so the magic link for /proc/1/root will escape our mount namespace. -/// We can't bind mount though - we need to symlink it so that each calling process -/// will traverse the link. -#[context("Linking tmp mounts to host")] -pub(crate) fn setup_tmp_mounts() -> Result<()> { +/// Ensure that /tmp is a tmpfs because in some cases we might perform +/// operations which expect it (as it is on a proper host system). +/// Ideally we have people run this container via podman run --read-only-tmpfs +/// actually. +pub(crate) fn setup_tmp_mount() -> Result<()> { let st = rustix::fs::statfs("/tmp")?; if st.f_type == libc::TMPFS_MAGIC { tracing::trace!("Already have tmpfs /tmp") @@ -1130,42 +1119,6 @@ pub(crate) fn setup_tmp_mounts() -> Result<()> { .quiet() .run()?; } - - // Point our /var/tmp at the host, via the /proc/1/root magic link - for path in ["/var/tmp"].map(Utf8Path::new) { - if path.try_exists()? { - let st = rustix::fs::statfs(path.as_std_path()).context(path)?; - if st.f_type != libc::OVERLAYFS_SUPER_MAGIC { - tracing::trace!("Already have {path} with f_type={}", st.f_type); - continue; - } - } - let target = format!("/proc/1/root/{path}"); - let tmp = format!("{path}.tmp"); - // Ensure idempotence in case we're re-executed - if path.is_symlink() { - continue; - } - tracing::debug!("Retargeting {path} to host"); - if path.try_exists()? { - std::os::unix::fs::symlink(&target, &tmp) - .with_context(|| format!("Symlinking {target} to {tmp}"))?; - let cwd = rustix::fs::CWD; - rustix::fs::renameat_with( - cwd, - path.as_os_str(), - cwd, - &tmp, - rustix::fs::RenameFlags::EXCHANGE, - ) - .with_context(|| format!("Exchanging {path} <=> {tmp}"))?; - std::fs::rename(&tmp, format!("{path}.old")) - .with_context(|| format!("Renaming old {tmp}"))?; - } else { - std::os::unix::fs::symlink(&target, path) - .with_context(|| format!("Symlinking {target} to {path}"))?; - }; - } Ok(()) } @@ -1293,11 +1246,16 @@ async fn prepare_install( }; tracing::debug!("Target image reference: {target_imgref}"); - // A bit of basic global state setup + // We need to access devices that are set up by the host udev bootc_mount::ensure_mirrored_host_mount("/dev")?; + // We need to read our own container image (and any logically bound images) + // from the host container store. bootc_mount::ensure_mirrored_host_mount("/var/lib/containers")?; - ensure_var()?; - setup_tmp_mounts()?; + // In some cases we may create large files, and it's better not to have those + // in our overlayfs. + bootc_mount::ensure_mirrored_host_mount("/var/tmp")?; + // We also always want /tmp to be a proper tmpfs on general principle. + setup_tmp_mount()?; // Allocate a temporary directory we can use in various places to avoid // creating multiple. let tempdir = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;