diff --git a/src/agent/rustjail/src/container.rs b/src/agent/rustjail/src/container.rs index c7241ecea9..859d2ccab2 100644 --- a/src/agent/rustjail/src/container.rs +++ b/src/agent/rustjail/src/container.rs @@ -844,10 +844,7 @@ impl BaseContainer for LinuxContainer { unistd::close(old_pid_ns); }); - let mut pidns = None; - if !p.init { - pidns = Some(get_pid_namespace(&self.logger, linux)?); - } + let pidns = get_pid_namespace(&self.logger, linux)?; if pidns.is_some() { sched::setns(pidns.unwrap(), CloneFlags::CLONE_NEWPID) @@ -1071,12 +1068,11 @@ fn update_namespaces(logger: &Logger, spec: &mut Spec, init_pid: RawFd) -> Resul Ok(()) } -fn get_pid_namespace(logger: &Logger, linux: &Linux) -> Result { +fn get_pid_namespace(logger: &Logger, linux: &Linux) -> Result> { for ns in &linux.namespaces { if ns.r#type == "pid" { if ns.path == "" { - error!(logger, "pid ns path is empty"); - return Err(ErrorKind::ErrorCode("pid ns path is empty".to_string()).into()); + return Ok(None); } let fd = match fcntl::open(ns.path.as_str(), OFlag::O_CLOEXEC, Mode::empty()) { @@ -1093,7 +1089,7 @@ fn get_pid_namespace(logger: &Logger, linux: &Linux) -> Result { } }; - return Ok(fd); + return Ok(Some(fd)); } } diff --git a/src/agent/src/namespace.rs b/src/agent/src/namespace.rs index f1e4b34ef9..374a67d91e 100644 --- a/src/agent/src/namespace.rs +++ b/src/agent/src/namespace.rs @@ -65,6 +65,11 @@ impl Namespace { self } + pub fn as_pid(mut self) -> Self { + self.ns_type = NamespaceType::PID; + self + } + pub fn set_root_dir(mut self, dir: &str) -> Self { self.persistent_ns_dir = dir.to_string(); self diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index 4176d4be1d..5e704be158 100644 --- a/src/agent/src/rpc.rs +++ b/src/agent/src/rpc.rs @@ -79,6 +79,7 @@ impl agentService { let cid = req.container_id.clone(); let mut oci_spec = req.OCI.clone(); + let use_sandbox_pidns = req.get_sandbox_pidns(); let sandbox; let mut s; @@ -121,7 +122,7 @@ impl agentService { s.container_mounts.insert(cid.clone(), m); } - update_container_namespaces(&s, &mut oci)?; + update_container_namespaces(&s, &mut oci, use_sandbox_pidns)?; // Add the root partition to the device cgroup to prevent access update_device_cgroup(&mut oci)?; @@ -162,6 +163,7 @@ impl agentService { ctr.start(p)?; + s.update_shared_pidns(&ctr)?; s.add_container(ctr); info!(sl!(), "created container!"); @@ -1478,7 +1480,11 @@ pub fn start>(s: Arc>, host: S, port: u16) -> ttr // path set by the spec, since we will always ignore it. Indeed, it makes no // sense to rely on the namespace path provided by the host since namespaces // are different inside the guest. -fn update_container_namespaces(sandbox: &Sandbox, spec: &mut Spec) -> Result<()> { +fn update_container_namespaces( + sandbox: &Sandbox, + spec: &mut Spec, + sandbox_pidns: bool, +) -> Result<()> { let linux = match spec.linux.as_mut() { None => { return Err( @@ -1488,14 +1494,8 @@ fn update_container_namespaces(sandbox: &Sandbox, spec: &mut Spec) -> Result<()> Some(l) => l, }; - let mut pidNs = false; - let namespaces = linux.namespaces.as_mut_slice(); for namespace in namespaces.iter_mut() { - if namespace.r#type == NSTYPEPID { - pidNs = true; - continue; - } if namespace.r#type == NSTYPEIPC { namespace.path = sandbox.shared_ipcns.path.clone(); continue; @@ -1505,13 +1505,19 @@ fn update_container_namespaces(sandbox: &Sandbox, spec: &mut Spec) -> Result<()> continue; } } + // update pid namespace + let mut pid_ns = LinuxNamespace::default(); + pid_ns.r#type = NSTYPEPID.to_string(); - if !pidNs && !sandbox.sandbox_pid_ns { - let mut pid_ns = LinuxNamespace::default(); - pid_ns.r#type = NSTYPEPID.to_string(); - linux.namespaces.push(pid_ns); + // Use shared pid ns if useSandboxPidns has been set in either + // the create_sandbox request or create_container request. + // Else set this to empty string so that a new pid namespace is + // created for the container. + if sandbox_pidns && sandbox.sandbox_pidns.is_some() { + pid_ns.path = String::from(sandbox.sandbox_pidns.as_ref().unwrap().path.as_str()); } + linux.namespaces.push(pid_ns); Ok(()) } diff --git a/src/agent/src/sandbox.rs b/src/agent/src/sandbox.rs index 1f0cb28bd9..5d7a993042 100644 --- a/src/agent/src/sandbox.rs +++ b/src/agent/src/sandbox.rs @@ -7,9 +7,11 @@ use crate::linux_abi::*; use crate::mount::{get_mount_fs_type, remove_mounts, TYPEROOTFS}; use crate::namespace::Namespace; +use crate::namespace::NSTYPEPID; use crate::network::Network; use libc::pid_t; use netlink::{RtnlHandle, NETLINK_ROUTE}; +use oci::LinuxNamespace; use protocols::agent::OnlineCPUMemRequest; use regex::Regex; use rustjail::cgroups; @@ -34,10 +36,10 @@ pub struct Sandbox { pub pci_device_map: HashMap, pub shared_utsns: Namespace, pub shared_ipcns: Namespace, + pub sandbox_pidns: Option, pub storages: HashMap, pub running: bool, pub no_pivot_root: bool, - pub sandbox_pid_ns: bool, pub sender: Option>, pub rtnl: Option, } @@ -58,10 +60,10 @@ impl Sandbox { pci_device_map: HashMap::new(), shared_utsns: Namespace::new(&logger), shared_ipcns: Namespace::new(&logger), + sandbox_pidns: None, storages: HashMap::new(), running: false, no_pivot_root: fs_type.eq(TYPEROOTFS), - sandbox_pid_ns: false, sender: None, rtnl: Some(RtnlHandle::new(NETLINK_ROUTE, 0).unwrap()), }) @@ -191,6 +193,30 @@ impl Sandbox { self.containers.insert(c.id.clone(), c); } + pub fn update_shared_pidns(&mut self, c: &LinuxContainer) -> Result<()> { + // Populate the shared pid path only if this is an infra container and + // sandbox_pidns has not been passed in the create_sandbox request. + // This means a separate pause process has not been created. We treat the + // first container created as the infra container in that case + // and use its pid namespace in case pid namespace needs to be shared. + if self.sandbox_pidns.is_none() && self.containers.len() == 0 { + let init_pid = c.init_process_pid; + if init_pid == -1 { + return Err(ErrorKind::ErrorCode(String::from( + "Failed to setup pid namespace: init container pid is -1", + )) + .into()); + } + + let mut pid_ns = Namespace::new(&self.logger).as_pid(); + pid_ns.path = format!("/proc/{}/ns/pid", init_pid); + + self.sandbox_pidns = Some(pid_ns); + } + + Ok(()) + } + pub fn get_container(&mut self, id: &str) -> Option<&mut LinuxContainer> { self.containers.get_mut(id) } @@ -553,4 +579,21 @@ mod tests { s.add_container(linux_container); assert!(s.get_container("some_id").is_some()); } + #[test] + fn update_shared_pidns() { + skip_if_not_root!(); + let logger = slog::Logger::root(slog::Discard, o!()); + let mut s = Sandbox::new(&logger).unwrap(); + let test_pid = 9999; + + let mut linux_container = create_linuxcontainer(); + linux_container.init_process_pid = test_pid; + + s.update_shared_pidns(&linux_container).unwrap(); + + assert!(s.sandbox_pidns.is_some()); + + let ns_path = format!("/proc/{}/ns/pid", test_pid); + assert_eq!(s.sandbox_pidns.unwrap().path, ns_path); + } }