From 018f18a7f0d32a4c0759d173dc0a75adcd8e0850 Mon Sep 17 00:00:00 2001 From: Ryan Bottriell Date: Wed, 15 May 2024 14:27:30 -0700 Subject: [PATCH 1/2] Use SLAVE mount flags in spfs Signed-off-by: Ryan Bottriell --- crates/spfs/src/env.rs | 29 ++++++++++++++++++++++------- crates/spfs/src/status_unix.rs | 2 +- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/crates/spfs/src/env.rs b/crates/spfs/src/env.rs index 1c2a1a9408..5c93569429 100644 --- a/crates/spfs/src/env.rs +++ b/crates/spfs/src/env.rs @@ -326,27 +326,42 @@ impl RuntimeConfigurator where MountNamespace: __private::CurrentThreadIsInMountNamespace, { - /// Privatize mounts in the current namespace, so that new mounts and changes + /// Remount key existing mount points so that new mounts and changes /// to existing mounts don't propagate to the parent namespace. - pub async fn privatize_existing_mounts(&self) -> Result<()> { + /// + /// We use MS_SLAVE for system mounts because we still want mount and + /// unmount events from the system to propagate into this new namespace. + /// We privatize any existing /spfs mount, though because we are likely + /// to replace it and don't want to affect any parent runtime. + pub async fn remove_mount_propagation(&self) -> Result<()> { use nix::mount::{mount, MsFlags}; - tracing::debug!("privatizing existing mounts..."); + tracing::debug!("disable sharing of new mounts..."); - let mut res = mount(NONE, "/", NONE, MsFlags::MS_PRIVATE, NONE); + let mut res = mount(NONE, "/", NONE, MsFlags::MS_SLAVE, NONE); if let Err(err) = res { return Err(Error::wrap_nix( err, - "Failed to privatize existing mount: /", + "Failed to remove propagation from existing mount: /", )); } + if self.is_mounted("/spfs").await? { + res = mount(NONE, "/spfs", NONE, MsFlags::MS_PRIVATE, NONE); + if let Err(err) = res { + return Err(Error::wrap_nix( + err, + "Failed to privatize existing mount: /spfs", + )); + } + } + if self.is_mounted("/tmp").await? { - res = mount(NONE, "/tmp", NONE, MsFlags::MS_PRIVATE, NONE); + res = mount(NONE, "/tmp", NONE, MsFlags::MS_SLAVE, NONE); if let Err(err) = res { return Err(Error::wrap_nix( err, - "Failed to privatize existing mount: /tmp", + "Failed to remove propagation from existing mount: /tmp", )); } } diff --git a/crates/spfs/src/status_unix.rs b/crates/spfs/src/status_unix.rs index a006b9b96f..df7fa7f831 100644 --- a/crates/spfs/src/status_unix.rs +++ b/crates/spfs/src/status_unix.rs @@ -221,7 +221,7 @@ pub async fn initialize_runtime(rt: &mut runtime::Runtime) -> Result { From 179c9efdbef36e68667a107290da29a5cd7b35c6 Mon Sep 17 00:00:00 2001 From: Ryan Bottriell Date: Wed, 15 May 2024 18:01:50 -0700 Subject: [PATCH 2/2] Ignore ENOENT errors when checking if paths are mounted It's safe to say that paths that do not exist are not mounted for all of the purposes in the spfs setup logic Signed-off-by: Ryan Bottriell --- crates/spfs/src/env.rs | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/crates/spfs/src/env.rs b/crates/spfs/src/env.rs index 5c93569429..eb4a28311b 100644 --- a/crates/spfs/src/env.rs +++ b/crates/spfs/src/env.rs @@ -283,6 +283,8 @@ where } /// Check if the identified directory is an active mount point. + /// + /// Returns false in the case where the path or its parent do not exist. async fn is_mounted>(&self, target: P) -> Result { let target = target.into(); let parent = match target.parent() { @@ -292,11 +294,9 @@ where // A new thread created while holding _guard will be inside the same // mount namespace... - let stat_parent_thread = - std::thread::spawn(move || nix::sys::stat::stat(&parent).map_err(Error::from)); + let stat_parent_thread = std::thread::spawn(move || nix::sys::stat::stat(&parent)); - let stat_target_thread = - std::thread::spawn(move || nix::sys::stat::stat(&target).map_err(Error::from)); + let stat_target_thread = std::thread::spawn(move || nix::sys::stat::stat(&target)); let (st_parent, st_target) = tokio::task::spawn_blocking(move || { let st_parent = stat_parent_thread.join(); @@ -306,9 +306,22 @@ where .await?; let st_parent = - st_parent.map_err(|_| Error::String("Failed to stat parent".to_owned()))??; + match st_parent.map_err(|_| Error::String("Failed to stat parent".to_owned()))? { + // the parent not existing means the child also doesn't exist + // and so cannot be considered as mounted + Err(nix::errno::Errno::ENOENT) => { + return Ok(false); + } + r => r?, + }; let st_target = - st_target.map_err(|_| Error::String("Failed to stat target".to_owned()))??; + match st_target.map_err(|_| Error::String("Failed to stat target".to_owned()))? { + // a non-existent directory is considered not mounted + Err(nix::errno::Errno::ENOENT) => { + return Ok(false); + } + r => r?, + }; Ok(st_target.st_dev != st_parent.st_dev) } @@ -346,8 +359,8 @@ where )); } - if self.is_mounted("/spfs").await? { - res = mount(NONE, "/spfs", NONE, MsFlags::MS_PRIVATE, NONE); + if self.is_mounted(SPFS_DIR).await? { + res = mount(NONE, SPFS_DIR, NONE, MsFlags::MS_PRIVATE, NONE); if let Err(err) = res { return Err(Error::wrap_nix( err,