diff --git a/src/agent/rustjail/src/cgroups/fs/mod.rs b/src/agent/rustjail/src/cgroups/fs/mod.rs index b7f9d9056b..bb21d04623 100644 --- a/src/agent/rustjail/src/cgroups/fs/mod.rs +++ b/src/agent/rustjail/src/cgroups/fs/mod.rs @@ -65,7 +65,7 @@ pub struct Manager { #[serde(skip)] cgroup: cgroups::Cgroup, #[serde(skip)] - pod_cgroup: cgroups::Cgroup, + pod_cgroup: Option, #[serde(skip)] devcg_whitelist: bool, } @@ -135,7 +135,9 @@ impl CgroupManager for Manager { ); // apply resources - self.pod_cgroup.apply(pod_res)?; + if let Some(pod_cg) = self.pod_cgroup.as_ref() { + pod_cg.apply(pod_res)?; + } self.cgroup.apply(res)?; Ok(()) @@ -1034,64 +1036,70 @@ impl Manager { pub fn new( cpath: &str, spec: &Spec, - devcg_info: Arc>, + devcg_info: Option>>, ) -> Result { let (paths, mounts) = Self::get_paths_and_mounts(cpath)?; // Do not expect poisoning lock - let mut devices_group_info = devcg_info.write().unwrap(); + let mut devices_group_info = devcg_info.as_ref().map(|i| i.write().unwrap()); + let pod_cgroup: Option; - // Cgroup path of parent of container - let pod_cpath = PathBuf::from(cpath) - .parent() - .context("Get parent of cgroup path")? - .display() - .to_string(); + if let Some(devices_group_info) = devices_group_info.as_mut() { + // Cgroup path of parent of container + let pod_cpath = PathBuf::from(cpath) + .parent() + .context("Get parent of cgroup path")? + .display() + .to_string(); - // Create a cgroup for the pod if not exists. - // Note that this step MUST be done before the pause container's - // cgroup created, since children inherit upper nodes' rules, which - // have excessive permission. You'll feel painful to shrink upper - // nodes' permission. - let pod_cg = load_cgroup(cgroups::hierarchies::auto(), &pod_cpath); + // Create a cgroup for the pod if not exists. + // Note that this step MUST be done before the pause container's + // cgroup created, since children inherit upper nodes' rules, which + // have excessive permission. You'll feel painful to shrink upper + // nodes' permission. + pod_cgroup = Some(load_cgroup(cgroups::hierarchies::auto(), &pod_cpath)); + let pod_cg = pod_cgroup.as_ref().unwrap(); - // The default is blacklist mode. - let is_whitelist_mode = Self::devcg_whitelist_rule(spec).unwrap_or(false); - if devices_group_info.inited { - // Do nothing for whitelist except for updating whitelist - // field of devices_group_info. - debug!(sl(), "Devices cgroup has been initialzied."); + // The default is blacklist mode. + let is_whitelist_mode = Self::devcg_whitelist_rule(spec).unwrap_or(false); + if devices_group_info.inited { + // Do nothing for whitelist except for updating whitelist + // field of devices_group_info. + debug!(sl(), "Devices cgroup has been initialzied."); - // Set allowed all devices to pod cgroup - if !devices_group_info.whitelist && is_whitelist_mode { - info!( + // Set allowed all devices to pod cgroup + if !devices_group_info.whitelist && is_whitelist_mode { + info!( sl(), "Pod devices cgroup is changed to whitelist mode, devices_group_info = {:?}", devices_group_info ); - Self::setup_whitelist_mode(&pod_cg) - .with_context(|| format!("Setup whitelist mode for {}", pod_cpath))?; - devices_group_info.whitelist = true; + Self::setup_whitelist_mode(pod_cg) + .with_context(|| format!("Setup whitelist mode for {}", pod_cpath))?; + devices_group_info.whitelist = true; + } + } else { + // This is the first container (aka pause container) + debug!(sl(), "Started to init devices cgroup"); + + pod_cg.create().context("Create pod cgroup")?; + + // Setup blacklist for pod cgroup + if !is_whitelist_mode { + // Setup blacklist rule and allowed default devices for + // blacklist. + Self::setup_blacklist_mode(pod_cg) + .with_context(|| format!("Setup blacklist mode for {}", pod_cpath))?; + } else { + Self::setup_whitelist_mode(pod_cg) + .with_context(|| format!("Setup whitelist mode for {}", pod_cpath))?; + devices_group_info.whitelist = true; + } + + devices_group_info.inited = true } } else { - // This is the first container (aka pause container) - debug!(sl(), "Started to init devices cgroup"); - - pod_cg.create().context("Create pod cgroup")?; - - // Setup blacklist for pod cgroup - if !is_whitelist_mode { - // Setup blacklist rule and allowed default devices for - // blacklist. - Self::setup_blacklist_mode(&pod_cg) - .with_context(|| format!("Setup blacklist mode for {}", pod_cpath))?; - } else { - Self::setup_whitelist_mode(&pod_cg) - .with_context(|| format!("Setup whitelist mode for {}", pod_cpath))?; - devices_group_info.whitelist = true; - } - - devices_group_info.inited = true + pod_cgroup = None; } // Create a cgroup for the container. @@ -1099,9 +1107,11 @@ impl Manager { // The rules of container cgroup are copied from its parent, which // contains some permissions that the container doesn't need. // Therefore, resetting the container's devices cgroup is required. - if !devices_group_info.whitelist { - Self::setup_blacklist_mode(&cg) - .with_context(|| format!("Setup blacklist mode for {}", cpath))?; + if let Some(devices_group_info) = devices_group_info.as_ref() { + if !devices_group_info.whitelist { + Self::setup_blacklist_mode(&cg) + .with_context(|| format!("Setup blacklist mode for {}", cpath))?; + } } Ok(Self { @@ -1110,8 +1120,10 @@ impl Manager { // rels: paths, cpath: cpath.to_string(), cgroup: cg, - pod_cgroup: pod_cg, - devcg_whitelist: devices_group_info.whitelist, + pod_cgroup, + devcg_whitelist: devices_group_info + .map(|info| info.whitelist) + .unwrap_or(false), }) } @@ -1123,21 +1135,13 @@ impl Manager { pub fn new_read_only(cpath: &str) -> Result { let (paths, mounts) = Self::get_paths_and_mounts(cpath)?; - // Cgroup path of parent of container - let pod_cpath = PathBuf::from(cpath) - .parent() - .context("Get parent of cgroup path")? - .display() - .to_string(); - - let pod_cg = load_cgroup(cgroups::hierarchies::auto(), &pod_cpath); let cg = load_cgroup(cgroups::hierarchies::auto(), cpath); Ok(Self { paths, mounts, cpath: cpath.to_string(), - pod_cgroup: pod_cg, + pod_cgroup: None, cgroup: cg, devcg_whitelist: false, }) @@ -1489,8 +1493,9 @@ mod tests { }), ..Default::default() }; - managers - .push(Manager::new(&tc.cpath[cid], &spec, sandbox.devcg_info.clone()).unwrap()); + managers.push( + Manager::new(&tc.cpath[cid], &spec, Some(sandbox.devcg_info.clone())).unwrap(), + ); let devcg_info = sandbox.devcg_info.read().unwrap(); assert!(devcg_info.inited); @@ -1501,7 +1506,6 @@ mod tests { ); drop(devcg_info); - // TODO: Assertions go here let pod_devices_list = Command::new("cat") .arg("/sys/fs/cgroup/devices/kata-agent-fs-manager-test/devices.list") .output() @@ -1526,7 +1530,8 @@ mod tests { managers .iter() .for_each(|manager| manager.cgroup.delete().unwrap()); - managers[0].pod_cgroup.delete().unwrap(); + // The pod_cgroup must not be None + managers[0].pod_cgroup.as_ref().unwrap().delete().unwrap(); } } } diff --git a/src/agent/rustjail/src/cgroups/mock.rs b/src/agent/rustjail/src/cgroups/mock.rs index 117bd25223..ce1cfb2bc7 100644 --- a/src/agent/rustjail/src/cgroups/mock.rs +++ b/src/agent/rustjail/src/cgroups/mock.rs @@ -78,7 +78,7 @@ impl Manager { pub fn new( cpath: &str, _spec: &Spec, - _devcg_info: Arc>, + _devcg_info: Option>>, ) -> Result { Ok(Self { paths: HashMap::new(), diff --git a/src/agent/rustjail/src/container.rs b/src/agent/rustjail/src/container.rs index f7f5e1282b..897c5a3c86 100644 --- a/src/agent/rustjail/src/container.rs +++ b/src/agent/rustjail/src/container.rs @@ -1494,7 +1494,7 @@ impl LinuxContainer { pub fn new + Display + Clone>( id: T, base: T, - devcg_info: Arc>, + devcg_info: Option>>, config: Config, logger: &Logger, ) -> Result { @@ -1775,7 +1775,7 @@ mod tests { LinuxContainer::new( "some_id", &dir.path().join("rootfs").to_str().unwrap(), - Arc::new(RwLock::new(DevicesCgroupInfo::default())), + None, create_dummy_opts(), &slog_scope::logger(), ), @@ -1805,14 +1805,10 @@ mod tests { #[test] fn test_linuxcontainer_pause() { let ret = new_linux_container_and_then(|mut c: LinuxContainer| { - c.cgroup_manager = Box::new( - FsManager::new( - "", - &Spec::default(), - Arc::new(RwLock::new(DevicesCgroupInfo::default())), - ) - .map_err(|e| anyhow!(format!("fail to create cgroup manager with path: {:}", e)))?, - ); + c.cgroup_manager = + Box::new(FsManager::new("", &Spec::default(), None).map_err(|e| { + anyhow!(format!("fail to create cgroup manager with path: {:}", e)) + })?); c.pause().map_err(|e| anyhow!(e)) }); @@ -1834,14 +1830,10 @@ mod tests { #[test] fn test_linuxcontainer_resume() { let ret = new_linux_container_and_then(|mut c: LinuxContainer| { - c.cgroup_manager = Box::new( - FsManager::new( - "", - &Spec::default(), - Arc::new(RwLock::new(DevicesCgroupInfo::default())), - ) - .map_err(|e| anyhow!(format!("fail to create cgroup manager with path: {:}", e)))?, - ); + c.cgroup_manager = + Box::new(FsManager::new("", &Spec::default(), None).map_err(|e| { + anyhow!(format!("fail to create cgroup manager with path: {:}", e)) + })?); // Change status to paused, this way we can resume it c.status.transition(ContainerState::Paused); c.resume().map_err(|e| anyhow!(e)) diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index b12b3a8265..bb7e2a943a 100644 --- a/src/agent/src/rpc.rs +++ b/src/agent/src/rpc.rs @@ -277,7 +277,7 @@ impl AgentService { let mut ctr: LinuxContainer = LinuxContainer::new( cid.as_str(), CONTAINER_BASE, - s.devcg_info.clone(), + Some(s.devcg_info.clone()), opts, &sl(), )?; @@ -1988,7 +1988,6 @@ fn load_kernel_module(module: &protocols::agent::KernelModule) -> Result<()> { #[cfg(test)] mod tests { - use std::sync::RwLock; use std::time::{SystemTime, UNIX_EPOCH}; use super::*; @@ -1996,7 +1995,6 @@ mod tests { use nix::mount; use nix::sched::{unshare, CloneFlags}; use oci::{Hook, Hooks, Linux, LinuxDeviceCgroup, LinuxNamespace, LinuxResources}; - use rustjail::cgroups::DevicesCgroupInfo; use tempfile::{tempdir, TempDir}; use test_utils::{assert_result, skip_if_not_root}; use ttrpc::{r#async::TtrpcContext, MessageHeader}; @@ -2074,7 +2072,7 @@ mod tests { LinuxContainer::new( "some_id", dir.path().join("rootfs").to_str().unwrap(), - Arc::new(RwLock::new(DevicesCgroupInfo::default())), + None, create_dummy_opts(), &slog_scope::logger(), ) diff --git a/src/agent/src/sandbox.rs b/src/agent/src/sandbox.rs index a1a857a646..8137939c8e 100644 --- a/src/agent/src/sandbox.rs +++ b/src/agent/src/sandbox.rs @@ -887,7 +887,7 @@ mod tests { let container = LinuxContainer::new( "some_id", dir.path().join("rootfs").to_str().unwrap(), - Arc::new(RwLock::new(DevicesCgroupInfo::default())), + None, create_dummy_opts(), &slog_scope::logger(), ) diff --git a/src/tools/runk/Cargo.lock b/src/tools/runk/Cargo.lock index 73f9db2d58..c8884f898e 100644 --- a/src/tools/runk/Cargo.lock +++ b/src/tools/runk/Cargo.lock @@ -292,9 +292,9 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "cgroups-rs" -version = "0.3.2" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b098e7c3a70d03c288fa0a96ccf13e770eb3d78c4cc0e1549b3c13215d5f965" +checksum = "1fb3af90c8d48ad5f432d8afb521b5b40c2a2fce46dd60e05912de51c47fba64" dependencies = [ "libc", "log", diff --git a/src/tools/runk/libcontainer/src/container.rs b/src/tools/runk/libcontainer/src/container.rs index f8ddcdfe5f..c7c3e068be 100644 --- a/src/tools/runk/libcontainer/src/container.rs +++ b/src/tools/runk/libcontainer/src/container.rs @@ -361,6 +361,7 @@ pub fn create_linux_container( .map(|s| s.to_string()) .ok_or_else(|| anyhow!("failed to convert bundle path"))? .as_str(), + None, config, logger, )?; @@ -384,6 +385,7 @@ pub fn load_linux_container( .to_str() .map(|s| s.to_string()) .ok_or_else(|| anyhow!("failed to convert a root path"))?, + None, status.config.clone(), logger, )?;