diff --git a/src/libs/kata-sys-util/src/mount.rs b/src/libs/kata-sys-util/src/mount.rs index 3c6f5f2616..522ce3acb1 100644 --- a/src/libs/kata-sys-util/src/mount.rs +++ b/src/libs/kata-sys-util/src/mount.rs @@ -103,6 +103,8 @@ pub enum Error { MountOptionTooBig, #[error("Path for mountpoint is null")] NullMountPointPath, + #[error("Invalid Propagation type Flag")] + InvalidPgMountFlag, #[error("Faile to open file {0} by path, {1}")] OpenByPath(PathBuf, io::Error), #[error("Can not read metadata of {0}, {1}")] @@ -227,7 +229,13 @@ pub fn bind_remount>(dst: P, readonly: bool) -> Result<()> { do_rebind_mount(dst, readonly, MsFlags::empty()) } -/// Bind mount `src` to `dst` in slave mode, optionally in readonly mode if `readonly` is true. +/// Bind mount `src` to `dst` with a custom propagation type, optionally in readonly mode if +/// `readonly` is true. +/// +/// Propagation type: MsFlags::MS_SHARED or MsFlags::MS_SLAVE +/// MsFlags::MS_SHARED is used to bind mount the sandbox path to enable `exec` (in case of FC +/// jailer). +/// MsFlags::MS_SLAVE is used on all other cases. /// /// # Safety /// Caller needs to ensure: @@ -238,6 +246,7 @@ pub fn bind_mount_unchecked, D: AsRef>( src: S, dst: D, readonly: bool, + pgflag: MsFlags, ) -> Result<()> { fail::fail_point!("bind_mount", |_| { Err(Error::FailureInject( @@ -268,8 +277,11 @@ pub fn bind_mount_unchecked, D: AsRef>( ) .map_err(|e| Error::BindMount(abs_src, dst.to_path_buf(), e))?; - // Change into slave propagation mode. - mount(Some(""), dst, Some(""), MsFlags::MS_SLAVE, Some("")) + // Change into the chosen propagation mode. + if !(pgflag == MsFlags::MS_SHARED || pgflag == MsFlags::MS_SLAVE) { + return Err(Error::InvalidPgMountFlag); + } + mount(Some(""), dst, Some(""), pgflag, Some("")) .map_err(|e| Error::Mount(PathBuf::new(), dst.to_path_buf(), e))?; // Optionally rebind into readonly mode. @@ -828,7 +840,7 @@ mod tests { Err(Error::InvalidPath(_)) )); - bind_mount_unchecked(tmpdir2.path(), tmpdir.path(), true).unwrap(); + bind_mount_unchecked(tmpdir2.path(), tmpdir.path(), true, MsFlags::MS_SLAVE).unwrap(); bind_remount(tmpdir.path(), true).unwrap(); umount_timeout(tmpdir.path().to_str().unwrap(), 0).unwrap(); } @@ -844,25 +856,26 @@ mod tests { dst.push("src"); assert!(matches!( - bind_mount_unchecked(Path::new(""), Path::new(""), false), + bind_mount_unchecked(Path::new(""), Path::new(""), false, MsFlags::MS_SLAVE), Err(Error::NullMountPointPath) )); assert!(matches!( - bind_mount_unchecked(tmpdir2.path(), Path::new(""), false), + bind_mount_unchecked(tmpdir2.path(), Path::new(""), false, MsFlags::MS_SLAVE), Err(Error::NullMountPointPath) )); assert!(matches!( bind_mount_unchecked( Path::new("/_does_not_exist_/___aahhhh"), Path::new("/tmp/_does_not_exist/___bbb"), - false + false, + MsFlags::MS_SLAVE ), Err(Error::InvalidPath(_)) )); let dst = create_mount_destination(tmpdir2.path(), &dst, tmpdir.path(), "bind").unwrap(); - bind_mount_unchecked(tmpdir2.path(), dst.as_ref(), true).unwrap(); - bind_mount_unchecked(&src, dst.as_ref(), false).unwrap(); + bind_mount_unchecked(tmpdir2.path(), dst.as_ref(), true, MsFlags::MS_SLAVE).unwrap(); + bind_mount_unchecked(&src, dst.as_ref(), false, MsFlags::MS_SLAVE).unwrap(); umount_all(dst.as_ref(), false).unwrap(); let mut src = tmpdir.path().to_owned(); @@ -871,7 +884,7 @@ mod tests { let mut dst = tmpdir.path().to_owned(); dst.push("file"); let dst = create_mount_destination(&src, &dst, tmpdir.path(), "bind").unwrap(); - bind_mount_unchecked(&src, dst.as_ref(), false).unwrap(); + bind_mount_unchecked(&src, dst.as_ref(), false, MsFlags::MS_SLAVE).unwrap(); assert!(dst.as_ref().is_file()); umount_timeout(dst.as_ref(), 0).unwrap(); } diff --git a/src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs b/src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs index 91f9406b88..61701da09f 100644 --- a/src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs +++ b/src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs @@ -16,11 +16,13 @@ use dragonball::{ api::v1::{BlockDeviceConfigInfo, BootSourceConfig, VcpuResizeInfo}, vm::VmConfigInfo, }; + use kata_sys_util::mount; use kata_types::{ capabilities::{Capabilities, CapabilityBits}, config::hypervisor::Hypervisor as HypervisorConfig, }; +use nix::mount::MsFlags; use persist::sandbox_persist::Persist; use shim_interface::KATA_PATH; use std::{collections::HashSet, fs::create_dir_all, path::PathBuf}; @@ -232,7 +234,8 @@ impl DragonballInner { } let jailed_location = [self.jailer_root.as_str(), dst].join("/"); - mount::bind_mount_unchecked(src, jailed_location.as_str(), false).context("bind_mount")?; + mount::bind_mount_unchecked(src, jailed_location.as_str(), false, MsFlags::MS_SLAVE) + .context("bind_mount")?; let mut abs_path = String::from("/"); abs_path.push_str(dst); diff --git a/src/runtime-rs/crates/resource/src/share_fs/sandbox_bind_mounts.rs b/src/runtime-rs/crates/resource/src/share_fs/sandbox_bind_mounts.rs index 13bd281033..8c166573d0 100644 --- a/src/runtime-rs/crates/resource/src/share_fs/sandbox_bind_mounts.rs +++ b/src/runtime-rs/crates/resource/src/share_fs/sandbox_bind_mounts.rs @@ -23,6 +23,7 @@ use anyhow::{anyhow, Context, Result}; use super::utils::{do_get_host_path, mkdir_with_permissions}; use kata_sys_util::{fs::get_base_name, mount}; use kata_types::mount::{SANDBOX_BIND_MOUNTS_DIR, SANDBOX_BIND_MOUNTS_RO, SANDBOX_BIND_MOUNTS_RW}; +use nix::mount::MsFlags; #[derive(Clone, Default, Debug)] pub struct SandboxBindMounts { @@ -101,14 +102,15 @@ impl SandboxBindMounts { // mount -o bind,ro host_shared mount_dest // host_shared: ${bindmount} - mount::bind_mount_unchecked(Path::new(bindmount), &mount_dest, true).map_err(|e| { - for p in &mounted_list { - nix::mount::umount(p).unwrap_or_else(|x| { - format!("do umount failed: {:?}", x); - }); - } - e - })?; + mount::bind_mount_unchecked(Path::new(bindmount), &mount_dest, true, MsFlags::MS_SLAVE) + .map_err(|e| { + for p in &mounted_list { + nix::mount::umount(p).unwrap_or_else(|x| { + format!("do umount failed: {:?}", x); + }); + } + e + })?; // default sandbox bind mounts mode is ro. if bindmount_mode == SANDBOX_BIND_MOUNTS_RO { diff --git a/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs.rs b/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs.rs index 9a5676bdfc..a1a1ba25c4 100644 --- a/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs.rs +++ b/src/runtime-rs/crates/resource/src/share_fs/share_virtio_fs.rs @@ -18,6 +18,7 @@ use hypervisor::{ Hypervisor, ShareFsDeviceConfig, }; use kata_sys_util::mount; +use nix::mount::MsFlags; use super::{utils, PASSTHROUGH_FS_DIR}; @@ -45,7 +46,7 @@ pub(crate) async fn prepare_virtiofs( let host_rw_dest = utils::get_host_rw_shared_path(id); utils::ensure_dir_exist(&host_rw_dest)?; - mount::bind_mount_unchecked(&host_rw_dest, &host_ro_dest, true) + mount::bind_mount_unchecked(&host_rw_dest, &host_ro_dest, true, MsFlags::MS_SLAVE) .context("bind mount shared_fs directory")?; let share_fs_device = ShareFsDevice { diff --git a/src/runtime-rs/crates/resource/src/share_fs/utils.rs b/src/runtime-rs/crates/resource/src/share_fs/utils.rs index 3300c74ef3..0b019e9c56 100644 --- a/src/runtime-rs/crates/resource/src/share_fs/utils.rs +++ b/src/runtime-rs/crates/resource/src/share_fs/utils.rs @@ -11,6 +11,7 @@ use std::{ use anyhow::Result; use kata_sys_util::mount; +use nix::mount::MsFlags; use super::*; @@ -45,7 +46,7 @@ pub(crate) fn share_to_guest( is_rafs: bool, ) -> Result { let host_dest = do_get_host_path(target, sid, cid, is_volume, false); - mount::bind_mount_unchecked(source, &host_dest, readonly) + mount::bind_mount_unchecked(source, &host_dest, readonly, MsFlags::MS_SLAVE) .with_context(|| format!("failed to bind mount {} to {}", source, &host_dest))?; // bind mount remount event is not propagated to mount subtrees, so we have