From 8713e219a4ee38432a38351a6400c4486483945d Mon Sep 17 00:00:00 2001 From: yihuaf Date: Tue, 2 May 2023 20:55:36 +0000 Subject: [PATCH 1/7] introduced libcontainer error Signed-off-by: yihuaf --- Cargo.lock | 1 + crates/libcontainer/Cargo.toml | 1 + crates/libcontainer/src/lib.rs | 54 ++++++++++++++++++- .../process/container_intermediate_process.rs | 11 ++-- crates/libcontainer/src/process/fork.rs | 19 ++++--- 5 files changed, 73 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 988ca2d0c..b3ce39b8c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1972,6 +1972,7 @@ dependencies = [ "serde_json", "serial_test", "syscalls", + "thiserror", ] [[package]] diff --git a/crates/libcontainer/Cargo.toml b/crates/libcontainer/Cargo.toml index 130d79c56..56b109134 100644 --- a/crates/libcontainer/Cargo.toml +++ b/crates/libcontainer/Cargo.toml @@ -44,6 +44,7 @@ syscalls = "0.6.10" rust-criu = "0.4.0" clone3 = "0.2.3" regex = "1.7.3" +thiserror = "1.0.24" [dev-dependencies] oci-spec = { version = "^0.6.0", features = ["proptests", "runtime"] } diff --git a/crates/libcontainer/src/lib.rs b/crates/libcontainer/src/lib.rs index 4ab5fbd59..4bd7245cd 100644 --- a/crates/libcontainer/src/lib.rs +++ b/crates/libcontainer/src/lib.rs @@ -8,11 +8,61 @@ pub mod notify_socket; pub mod process; pub mod rootfs; pub mod rootless; +#[cfg(feature = "libseccomp")] +pub mod seccomp; pub mod signal; pub mod syscall; pub mod tty; pub mod utils; pub mod workload; +use std::{any::Any, result::Result as StdResult}; +use thiserror::Error as ThisError; -#[cfg(feature = "libseccomp")] -pub mod seccomp; +pub type Result = StdResult; + +#[derive(ThisError, Debug)] +pub enum LibcontainerError { + #[error("unknown fatal error {0}")] + UnknownLegacy(#[from] anyhow::Error), + #[error("unknown fatal error {0}")] + UnknownWithMsg(String), + #[error("unknown fatal error")] + Unknown, + #[error("io error: {0}")] + Io(#[from] std::io::Error), + #[error("io error: {0}")] + UnixIo(#[from] nix::errno::Errno), + #[error("failed to clone process using clone3")] + CloneFailed{ + errno: nix::errno::Errno, + child_name: String, + }, + #[error("failed to add task {pid} to cgroup manager")] + CgroupAdd { + pid: nix::unistd::Pid, + err: anyhow::Error, + }, + #[error("failed to apply resource limits to cgroup")] + CgroupApply(anyhow::Error), + #[error("failed to get proc state")] + Procfs(#[from] procfs::ProcError), +} + +impl From> for LibcontainerError { + fn from(e: Box) -> Self { + if e.downcast_ref::().is_none() { + match e.downcast_ref::<&'static str>() { + Some(s) => LibcontainerError::UnknownWithMsg(s.to_string()), + None => match e.downcast_ref::() { + Some(s) => LibcontainerError::UnknownWithMsg(s.into()), + None => LibcontainerError::Unknown, + }, + } + } else { + match e.downcast::() { + Ok(ae) => *ae, + Err(_) => LibcontainerError::Unknown, + } + } + } +} diff --git a/crates/libcontainer/src/process/container_intermediate_process.rs b/crates/libcontainer/src/process/container_intermediate_process.rs index 290a786c1..284a4accd 100644 --- a/crates/libcontainer/src/process/container_intermediate_process.rs +++ b/crates/libcontainer/src/process/container_intermediate_process.rs @@ -1,5 +1,6 @@ +use crate::{LibcontainerError, Result}; use crate::{namespaces::Namespaces, process::channel, process::fork}; -use anyhow::{Context, Error, Result}; +use anyhow::{Context}; use libcgroups::common::CgroupManager; use nix::unistd::{close, write}; use nix::unistd::{Gid, Pid, Uid}; @@ -110,7 +111,7 @@ pub fn container_intermediate_process( write(exec_notify_fd, buf.as_bytes())?; close(exec_notify_fd)?; } - Err(e) + Err(LibcontainerError::UnknownLegacy(e)) } } })?; @@ -144,11 +145,11 @@ fn apply_cgroups< cmanager: &C, resources: Option<&LinuxResources>, init: bool, -) -> Result<(), Error> { +) -> Result<()> { let pid = Pid::from_raw(Process::myself()?.pid()); cmanager .add_task(pid) - .with_context(|| format!("failed to add task {pid} to cgroup manager"))?; + .map_err(|err| LibcontainerError::CgroupAdd { pid, err })?; if let Some(resources) = resources { if init { @@ -161,7 +162,7 @@ fn apply_cgroups< cmanager .apply(&controller_opt) - .context("failed to apply resource limits to cgroup")?; + .map_err(|err| LibcontainerError::CgroupApply(err))?; } } diff --git a/crates/libcontainer/src/process/fork.rs b/crates/libcontainer/src/process/fork.rs index 3703ed056..74ebfce28 100644 --- a/crates/libcontainer/src/process/fork.rs +++ b/crates/libcontainer/src/process/fork.rs @@ -1,4 +1,4 @@ -use anyhow::{Context, Result}; +use crate::{LibcontainerError, Result}; use libc::SIGCHLD; use nix::unistd::Pid; use prctl; @@ -18,7 +18,7 @@ pub fn container_clone_sibling Result>(child_name: &str, cb: // the exit signal bits in the glibc wrapper. clone.flag_parent(); - container_clone(child_name, cb, clone).with_context(|| "failed to clone sibling process") + container_clone(child_name, cb, clone) } // A simple clone wrapper to clone3 so we can share this logic in different @@ -34,7 +34,14 @@ fn container_clone Result>( // cloned process, run the callback function, and exit with the same exit // code returned by the callback. If there was any error when trying to run // callback, exit with -1 - match unsafe { clone_cmd.call().with_context(|| "failed to run clone3")? } { + match unsafe { + clone_cmd + .call() + .map_err(|err| LibcontainerError::CloneFailed { + errno: nix::errno::from_i32(err.0), + child_name: child_name.to_string(), + })? + } { 0 => { prctl::set_name(child_name).expect("failed to set name"); // Inside the cloned process @@ -62,7 +69,7 @@ pub fn container_fork Result>(child_name: &str, cb: F) -> Re let mut clone = clone3::Clone3::default(); clone.exit_signal(SIGCHLD as u64); - container_clone(child_name, cb, clone).with_context(|| "failed to fork process") + container_clone(child_name, cb, clone) } #[cfg(test)] @@ -70,7 +77,7 @@ mod test { use crate::process::channel::channel; use super::*; - use anyhow::{bail, Result}; + use anyhow::{bail, Context, Result}; use nix::sys::wait::{waitpid, WaitStatus}; use nix::unistd; @@ -89,7 +96,7 @@ mod test { #[test] fn test_container_err_fork() -> Result<()> { - let pid = container_fork("test:child", || bail!(""))?; + let pid = container_fork("test:child", || Err(LibcontainerError::Unknown))?; match waitpid(pid, None).expect("wait pid failed.") { WaitStatus::Exited(p, status) => { assert_eq!(pid, p); From ac992a06444187ceafce5951306c64d8c2b139e0 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Wed, 3 May 2023 01:04:28 +0000 Subject: [PATCH 2/7] implemented syscall error Signed-off-by: yihuaf --- crates/libcontainer/src/syscall/linux.rs | 196 +++++++++++++-------- crates/libcontainer/src/syscall/mod.rs | 106 +++++++++++ crates/libcontainer/src/syscall/syscall.rs | 3 +- crates/libcontainer/src/syscall/test.rs | 40 ++--- 4 files changed, 252 insertions(+), 93 deletions(-) diff --git a/crates/libcontainer/src/syscall/linux.rs b/crates/libcontainer/src/syscall/linux.rs index 84209be14..c3adf44a7 100644 --- a/crates/libcontainer/src/syscall/linux.rs +++ b/crates/libcontainer/src/syscall/linux.rs @@ -1,5 +1,4 @@ //! Implements Command trait for Linux systems -use anyhow::{anyhow, bail, Context, Error, Result}; use caps::{CapSet, CapsHashSet}; use libc::{c_char, setdomainname, uid_t}; use nix::fcntl; @@ -23,7 +22,7 @@ use std::sync::Arc; use std::{any::Any, mem, path::Path, ptr}; use syscalls::{syscall, Sysno, Sysno::close_range}; -use super::Syscall; +use super::{Result, Syscall, SyscallError}; use crate::syscall::syscall::CloseRange; use crate::{capabilities, utils}; @@ -75,9 +74,9 @@ pub enum MountAttrOption { } impl FromStr for MountAttrOption { - type Err = Error; + type Err = SyscallError; - fn from_str(option: &str) -> Result { + fn from_str(option: &str) -> std::result::Result { match option { "rro" => Ok(MountAttrOption::MountArrtRdonly(false, MOUNT_ATTR_RDONLY)), "rrw" => Ok(MountAttrOption::MountArrtRdonly(true, MOUNT_ATTR_RDONLY)), @@ -122,7 +121,7 @@ impl FromStr for MountAttrOption { MOUNT_ATTR_NOSYMFOLLOW, )), // No support for MOUNT_ATTR_IDMAP yet (needs UserNS FD) - _ => Err(anyhow!("Unexpected option.")), + _ => Err(SyscallError::UnexpectedMountAttrOption(option.to_string())), } } } @@ -193,7 +192,7 @@ impl LinuxSyscall { } fn emulate_close_range(preserve_fds: i32) -> Result<()> { - let open_fds = Self::get_open_fds().with_context(|| "failed to obtain opened fds")?; + let open_fds = Self::get_open_fds()?; // Include stdin, stdout, and stderr for fd 0, 1, and 2 respectively. let min_fd = preserve_fds + 3; let to_be_cleaned_up_fds: Vec = open_fds @@ -214,9 +213,10 @@ impl LinuxSyscall { fn get_open_fds() -> Result> { const PROCFS_FD_PATH: &str = "/proc/self/fd"; utils::ensure_procfs(Path::new(PROCFS_FD_PATH)) - .with_context(|| format!("{PROCFS_FD_PATH} is not the actual procfs"))?; + .map_err(|_| SyscallError::NotProcfs(PROCFS_FD_PATH.to_string()))?; - let fds: Vec = fs::read_dir(PROCFS_FD_PATH)? + let fds: Vec = fs::read_dir(PROCFS_FD_PATH) + .map_err(SyscallError::GetOpenFdsFailed)? .filter_map(|entry| match entry { Ok(entry) => Some(entry.path()), Err(_) => None, @@ -248,7 +248,14 @@ impl Syscall for LinuxSyscall { /// Function to set given path as root path inside process fn pivot_rootfs(&self, path: &Path) -> Result<()> { // open the path as directory and read only - let newroot = open(path, OFlag::O_DIRECTORY | OFlag::O_RDONLY, Mode::empty())?; + let newroot = + open(path, OFlag::O_DIRECTORY | OFlag::O_RDONLY, Mode::empty()).map_err(|errno| { + SyscallError::PivotRootFailed { + errno, + msg: format!("failed to open {:?}", path), + path: format!("{:?}", path), + } + })?; // make the given path as the root directory for the container // see https://man7.org/linux/man-pages/man2/pivot_root.2.html, specially the notes @@ -258,7 +265,11 @@ impl Syscall for LinuxSyscall { // this path. This is done, as otherwise, we will need to create a separate temporary directory under the new root path // so we can move the original root there, and then unmount that. This way saves the creation of the temporary // directory to put original root directory. - pivot_root(path, path)?; + pivot_root(path, path).map_err(|errno| SyscallError::PivotRootFailed { + errno, + msg: format!("failed to pivot root to {:?}", path), + path: format!("{:?}", path), + })?; // Make the original root directory rslave to avoid propagating unmount event to the host mount namespace. // We should use MS_SLAVE not MS_PRIVATE according to https://github.com/opencontainers/runc/pull/1500. @@ -268,32 +279,51 @@ impl Syscall for LinuxSyscall { None::<&str>, MsFlags::MS_SLAVE | MsFlags::MS_REC, None::<&str>, - )?; + ) + .map_err(|errno| SyscallError::PivotRootFailed { + errno, + msg: format!("failed to make root directory rslave"), + path: format!("{:?}", path), + })?; // Unmount the original root directory which was stacked on top of new root directory // MNT_DETACH makes the mount point unavailable to new accesses, but waits till the original mount point // to be free of activity to actually unmount // see https://man7.org/linux/man-pages/man2/umount2.2.html for more information - umount2("/", MntFlags::MNT_DETACH)?; - // Change directory to root - fchdir(newroot)?; + umount2("/", MntFlags::MNT_DETACH).map_err(|errno| SyscallError::PivotRootFailed { + errno, + msg: format!("failed to unmount old root directory"), + path: format!("{:?}", path), + })?; + // Change directory to the new root + fchdir(newroot).map_err(|errno| SyscallError::PivotRootFailed { + errno, + msg: format!("failed to change directory to new root"), + path: format!("{:?}", path), + })?; + Ok(()) } /// Set namespace for process fn set_ns(&self, rawfd: i32, nstype: CloneFlags) -> Result<()> { - nix::sched::setns(rawfd, nstype)?; + nix::sched::setns(rawfd, nstype).map_err(SyscallError::SetNamespaceFailed)?; Ok(()) } /// set uid and gid for process fn set_id(&self, uid: Uid, gid: Gid) -> Result<()> { - if let Err(e) = prctl::set_keep_capabilities(true) { - bail!("set keep capabilities returned {}", e); - }; + prctl::set_keep_capabilities(true).map_err(|errno| { + SyscallError::PrctlSetKeepCapabilitesFailed { + errno: nix::errno::from_i32(errno), + value: true, + } + })?; // args : real *id, effective *id, saved set *id respectively - unistd::setresgid(gid, gid, gid)?; - unistd::setresuid(uid, uid, uid)?; + unistd::setresgid(gid, gid, gid) + .map_err(|errno| SyscallError::SetRealGidFailed { errno, gid })?; + unistd::setresuid(uid, uid, uid) + .map_err(|errno| SyscallError::SetRealUidFailed { errno, uid })?; // if not the root user, reset capabilities to effective capabilities, // which are used by kernel to perform checks @@ -301,17 +331,19 @@ impl Syscall for LinuxSyscall { if uid != Uid::from_raw(0) { capabilities::reset_effective(self)?; } - if let Err(e) = prctl::set_keep_capabilities(false) { - bail!("set keep capabilities returned {}", e); - }; + prctl::set_keep_capabilities(false).map_err(|errno| { + SyscallError::PrctlSetKeepCapabilitesFailed { + errno: nix::errno::from_i32(errno), + value: false, + } + })?; Ok(()) } /// Disassociate parts of execution context // see https://man7.org/linux/man-pages/man2/unshare.2.html for more information fn unshare(&self, flags: CloneFlags) -> Result<()> { - unshare(flags)?; - Ok(()) + unshare(flags).map_err(SyscallError::UnshareFailed) } /// Set capabilities for container process @@ -339,10 +371,10 @@ impl Syscall for LinuxSyscall { /// Sets hostname for process fn set_hostname(&self, hostname: &str) -> Result<()> { - if let Err(e) = sethostname(hostname) { - bail!("Failed to set {} as hostname. {:?}", hostname, e) - } - Ok(()) + sethostname(hostname).map_err(|errno| SyscallError::SetHostnameFailed { + errno, + hostname: hostname.to_string(), + }) } /// Sets domainname for process (see @@ -351,18 +383,17 @@ impl Syscall for LinuxSyscall { let ptr = domainname.as_bytes().as_ptr() as *const c_char; let len = domainname.len(); let res = unsafe { setdomainname(ptr, len) }; - match res { 0 => Ok(()), - -1 => bail!( - "Failed to set {} as domainname. {}", - domainname, - std::io::Error::last_os_error() - ), - _ => bail!( - "Failed to set {} as domainname. unexpected error occor.", - domainname - ), + -1 => Err(SyscallError::SetDomainnameFailed { + errno: nix::errno::Errno::last(), + domainname: domainname.to_string(), + }), + + _ => Err(SyscallError::SetDomainnameFailed { + errno: nix::errno::Errno::UnknownErrno, + domainname: domainname.to_string(), + }), } } @@ -379,9 +410,12 @@ impl Syscall for LinuxSyscall { #[cfg(target_env = "musl")] let res = unsafe { libc::setrlimit(rlimit.typ() as i32, rlim) }; - if let Err(e) = Errno::result(res).map(drop) { - bail!("Failed to set {:?}. {:?}", rlimit.typ(), e) - } + Errno::result(res) + .map(drop) + .map_err(|errno| SyscallError::SetRlimitFailed { + errno, + rlimit: rlimit.typ(), + })?; Ok(()) } @@ -420,7 +454,10 @@ impl Syscall for LinuxSyscall { } fn chroot(&self, path: &Path) -> Result<()> { - unistd::chroot(path)?; + unistd::chroot(path).map_err(|errno| SyscallError::ChrootFailed { + errno, + path: path.to_path_buf(), + })?; Ok(()) } @@ -433,38 +470,48 @@ impl Syscall for LinuxSyscall { flags: MsFlags, data: Option<&str>, ) -> Result<()> { - match mount(source, target, fstype, flags, data) { - Ok(_) => Ok(()), - Err(e) => Err(anyhow!(e)), - } + mount(source, target, fstype, flags, data).map_err(|errno| SyscallError::MountFailed { + errno, + mount_source: source.map(|p| p.to_path_buf()), + mount_target: target.to_path_buf(), + fstype: fstype.map(|s| s.to_string()), + flags, + data: data.map(|s| s.to_string()), + }) } fn symlink(&self, original: &Path, link: &Path) -> Result<()> { - match symlink(original, link) { - Ok(_) => Ok(()), - Err(e) => Err(anyhow!(e)), - } + symlink(original, link).map_err(|err| SyscallError::SymlinkFailed { + err, + old_path: original.to_path_buf(), + new_path: link.to_path_buf(), + }) } fn mknod(&self, path: &Path, kind: SFlag, perm: Mode, dev: u64) -> Result<()> { - match mknod(path, kind, perm, dev) { - Ok(_) => Ok(()), - Err(e) => Err(anyhow!(e)), - } + mknod(path, kind, perm, dev).map_err(|errno| SyscallError::MknodFailed { + errno, + path: path.to_path_buf(), + kind, + perm, + dev, + }) } fn chown(&self, path: &Path, owner: Option, group: Option) -> Result<()> { - match chown(path, owner, group) { - Ok(_) => Ok(()), - Err(e) => Err(anyhow!(e)), - } + chown(path, owner, group).map_err(|errno| SyscallError::ChownFailed { + errno, + path: path.to_path_buf(), + owner, + group, + }) } fn set_groups(&self, groups: &[Gid]) -> Result<()> { - match setgroups(groups) { - Ok(_) => Ok(()), - Err(e) => Err(anyhow!(e)), - } + setgroups(groups).map_err(|errno| SyscallError::SetGroupsFailed { + groups: groups.to_vec(), + errno, + }) } fn close_range(&self, preserve_fds: i32) -> Result<()> { @@ -483,7 +530,10 @@ impl Syscall for LinuxSyscall { // kernel 5.11. If the kernel is older we emulate close_range in userspace. Self::emulate_close_range(preserve_fds) } - Err(e) => bail!(e), + Err(e) => Err(SyscallError::CloseRangeFailed { + preserve_fds, + errno: e, + }), } } @@ -495,12 +545,12 @@ impl Syscall for LinuxSyscall { mount_attr: &MountAttr, size: libc::size_t, ) -> Result<()> { - let path_pathbuf = pathname.to_path_buf(); - let path_str = path_pathbuf.to_str(); - let path_c_string = match path_str { - Some(path_str) => CString::new(path_str)?, - None => bail!("Invalid filename"), - }; + let path_c_string = pathname + .to_path_buf() + .to_str() + .ok_or(SyscallError::InvalidFilename(pathname.to_path_buf())) + .map(CString::new)? + .map_err(|_| SyscallError::InvalidFilename(pathname.to_path_buf()))?; let result = unsafe { // TODO: nix/libc crate hasn't supported mount_setattr system call yet. // TODO: @krisnova migrate all youki to libc::SYS_mount_setattr @@ -518,7 +568,11 @@ impl Syscall for LinuxSyscall { match result { Ok(_) => Ok(()), - Err(e) => bail!(e), + Err(e) => Err(SyscallError::MountSetattrFailed { + pathname: pathname.to_path_buf(), + flags, + errno: e, + }), } } } diff --git a/crates/libcontainer/src/syscall/mod.rs b/crates/libcontainer/src/syscall/mod.rs index 543997e8e..6b2f57cc6 100644 --- a/crates/libcontainer/src/syscall/mod.rs +++ b/crates/libcontainer/src/syscall/mod.rs @@ -8,3 +8,109 @@ pub mod syscall; pub mod test; pub use syscall::Syscall; +#[derive(Debug, thiserror::Error)] +pub enum SyscallError { + #[error("unexpected mount attr option: {0}")] + UnexpectedMountAttrOption(String), + #[error("set keep capabilities to {value} returned {errno}")] + PrctlSetKeepCapabilitesFailed { + errno: nix::errno::Errno, + value: bool, + }, + #[error("set hostname to {hostname} returned {errno}")] + SetHostnameFailed{ + errno: nix::errno::Errno, + hostname: String, + }, + #[error("set domainname to {domainname} returned {errno}")] + SetDomainnameFailed{ + errno: nix::errno::Errno, + domainname: String, + }, + #[error("{0} is not an actual procfs")] + NotProcfs(String), + #[error("failed to get open fds: {0}")] + GetOpenFdsFailed(std::io::Error), + #[error("failed to pivot root")] + PivotRootFailed{ + path: String, + msg: String, + errno: nix::errno::Errno, + }, + #[error("failed to setns: {0}")] + SetNamespaceFailed(nix::errno::Errno), + #[error("failed to set real gid to {gid}: {errno}")] + SetRealGidFailed{ + errno: nix::errno::Errno, + gid: nix::unistd::Gid, + }, + #[error("failed to set real uid to {uid}: {errno}")] + SetRealUidFailed { + errno: nix::errno::Errno, + uid: nix::unistd::Uid, + }, + #[error("failed to unshare: {0}")] + UnshareFailed(nix::errno::Errno), + #[error("failed to set capabilities: {0}")] + SetCapsFailed(#[from] caps::errors::CapsError), + #[error("failed to set rlimit {rlimit:?}: {errno}")] + SetRlimitFailed { + errno: nix::errno::Errno, + rlimit: oci_spec::runtime::LinuxRlimitType, + }, + #[error("failed to chroot to {path:?}: {errno}")] + ChrootFailed { + path: std::path::PathBuf, + errno: nix::errno::Errno, + }, + #[error("mount failed")] + MountFailed { + mount_source: Option, + mount_target: std::path::PathBuf, + fstype: Option, + flags: nix::mount::MsFlags, + data: Option, + errno: nix::errno::Errno, + }, + #[error("symlink failed")] + SymlinkFailed { + old_path: std::path::PathBuf, + new_path: std::path::PathBuf, + err: std::io::Error, + }, + #[error("mknod failed")] + MknodFailed { + path: std::path::PathBuf, + kind: nix::sys::stat::SFlag, + perm: nix::sys::stat::Mode, + dev: nix::sys::stat::dev_t, + errno: nix::errno::Errno, + }, + #[error("chown failed")] + ChownFailed { + path: std::path::PathBuf, + owner: Option, + group: Option, + errno: nix::errno::Errno, + }, + #[error("setgroups failed")] + SetGroupsFailed { + groups: Vec, + errno: nix::errno::Errno, + }, + #[error("close range failed")] + CloseRangeFailed { + preserve_fds: i32, + errno: syscalls::Errno, + }, + #[error("invalid filename: {0:?}")] + InvalidFilename(std::path::PathBuf), + #[error("mount_setattr failed")] + MountSetattrFailed { + pathname: std::path::PathBuf, + flags: u32, + errno: syscalls::Errno, + }, +} + +type Result = std::result::Result; diff --git a/crates/libcontainer/src/syscall/syscall.rs b/crates/libcontainer/src/syscall/syscall.rs index 94c1c617c..20c807650 100644 --- a/crates/libcontainer/src/syscall/syscall.rs +++ b/crates/libcontainer/src/syscall/syscall.rs @@ -2,8 +2,6 @@ //! necessary functions without having to worry about their //! implementation details use std::{any::Any, ffi::OsStr, path::Path, sync::Arc}; - -use anyhow::Result; use bitflags::bitflags; use caps::{CapSet, CapsHashSet}; use libc; @@ -18,6 +16,7 @@ use oci_spec::runtime::LinuxRlimit; use crate::syscall::{ linux::{LinuxSyscall, MountAttr}, + Result, test::TestHelperSyscall, }; diff --git a/crates/libcontainer/src/syscall/test.rs b/crates/libcontainer/src/syscall/test.rs index 2bf7f034f..c6c63f584 100644 --- a/crates/libcontainer/src/syscall/test.rs +++ b/crates/libcontainer/src/syscall/test.rs @@ -17,7 +17,7 @@ use nix::{ use oci_spec::runtime::LinuxRlimit; -use super::{linux, Syscall}; +use super::{linux, Syscall, Result}; #[derive(Clone, PartialEq, Eq, Debug)] pub struct MountArgs { @@ -46,7 +46,7 @@ pub struct ChownArgs { #[derive(Default)] struct Mock { values: Vec>, - ret_err: Option anyhow::Result<()>>, + ret_err: Option Result<()>>, ret_err_times: usize, } @@ -102,7 +102,7 @@ impl Default for MockCalls { } impl MockCalls { - fn act(&self, name: ArgName, value: Box) -> anyhow::Result<()> { + fn act(&self, name: ArgName, value: Box) -> Result<()> { if self.args.get(&name).unwrap().borrow().ret_err_times > 0 { self.args.get(&name).unwrap().borrow_mut().ret_err_times -= 1; if let Some(e) = &self.args.get(&name).unwrap().borrow().ret_err { @@ -138,39 +138,39 @@ impl Syscall for TestHelperSyscall { self } - fn pivot_rootfs(&self, _path: &Path) -> anyhow::Result<()> { + fn pivot_rootfs(&self, _path: &Path) -> Result<()> { unimplemented!() } - fn set_ns(&self, rawfd: i32, nstype: CloneFlags) -> anyhow::Result<()> { + fn set_ns(&self, rawfd: i32, nstype: CloneFlags) -> Result<()> { self.mocks .act(ArgName::Namespace, Box::new((rawfd, nstype))) } - fn set_id(&self, _uid: Uid, _gid: Gid) -> anyhow::Result<()> { + fn set_id(&self, _uid: Uid, _gid: Gid) -> Result<()> { unimplemented!() } - fn unshare(&self, flags: CloneFlags) -> anyhow::Result<()> { + fn unshare(&self, flags: CloneFlags) -> Result<()> { self.mocks.act(ArgName::Unshare, Box::new(flags)) } - fn set_capability(&self, cset: CapSet, value: &CapsHashSet) -> anyhow::Result<()> { + fn set_capability(&self, cset: CapSet, value: &CapsHashSet) -> Result<()> { self.mocks .act(ArgName::Capability, Box::new((cset, value.clone()))) } - fn set_hostname(&self, hostname: &str) -> anyhow::Result<()> { + fn set_hostname(&self, hostname: &str) -> Result<()> { self.mocks .act(ArgName::Hostname, Box::new(hostname.to_owned())) } - fn set_domainname(&self, domainname: &str) -> anyhow::Result<()> { + fn set_domainname(&self, domainname: &str) -> Result<()> { self.mocks .act(ArgName::Domainname, Box::new(domainname.to_owned())) } - fn set_rlimit(&self, _rlimit: &LinuxRlimit) -> anyhow::Result<()> { + fn set_rlimit(&self, _rlimit: &LinuxRlimit) -> Result<()> { todo!() } @@ -178,7 +178,7 @@ impl Syscall for TestHelperSyscall { Some(OsString::from("youki").into()) } - fn chroot(&self, _: &Path) -> anyhow::Result<()> { + fn chroot(&self, _: &Path) -> Result<()> { todo!() } @@ -189,7 +189,7 @@ impl Syscall for TestHelperSyscall { fstype: Option<&str>, flags: MsFlags, data: Option<&str>, - ) -> anyhow::Result<()> { + ) -> Result<()> { self.mocks.act( ArgName::Mount, Box::new(MountArgs { @@ -202,14 +202,14 @@ impl Syscall for TestHelperSyscall { ) } - fn symlink(&self, original: &Path, link: &Path) -> anyhow::Result<()> { + fn symlink(&self, original: &Path, link: &Path) -> Result<()> { self.mocks.act( ArgName::Symlink, Box::new((original.to_path_buf(), link.to_path_buf())), ) } - fn mknod(&self, path: &Path, kind: SFlag, perm: Mode, dev: u64) -> anyhow::Result<()> { + fn mknod(&self, path: &Path, kind: SFlag, perm: Mode, dev: u64) -> Result<()> { self.mocks.act( ArgName::Mknod, Box::new(MknodArgs { @@ -220,7 +220,7 @@ impl Syscall for TestHelperSyscall { }), ) } - fn chown(&self, path: &Path, owner: Option, group: Option) -> anyhow::Result<()> { + fn chown(&self, path: &Path, owner: Option, group: Option) -> Result<()> { self.mocks.act( ArgName::Chown, Box::new(ChownArgs { @@ -231,11 +231,11 @@ impl Syscall for TestHelperSyscall { ) } - fn set_groups(&self, groups: &[Gid]) -> anyhow::Result<()> { + fn set_groups(&self, groups: &[Gid]) -> Result<()> { self.mocks.act(ArgName::Groups, Box::new(groups.to_vec())) } - fn close_range(&self, _: i32) -> anyhow::Result<()> { + fn close_range(&self, _: i32) -> Result<()> { todo!() } @@ -246,13 +246,13 @@ impl Syscall for TestHelperSyscall { _: u32, _: &linux::MountAttr, _: libc::size_t, - ) -> anyhow::Result<()> { + ) -> Result<()> { todo!() } } impl TestHelperSyscall { - pub fn set_ret_err(&self, name: ArgName, err: fn() -> anyhow::Result<()>) { + pub fn set_ret_err(&self, name: ArgName, err: fn() -> Result<()>) { self.mocks.fetch_mut(name).ret_err = Some(err); self.set_ret_err_times(name, 1); } From 19966613a7824513bebd7bddec2ffe5da6beeafe Mon Sep 17 00:00:00 2001 From: yihuaf Date: Wed, 3 May 2023 03:12:10 +0000 Subject: [PATCH 3/7] implemented namespace error Signed-off-by: yihuaf --- crates/libcontainer/src/namespaces.rs | 43 ++++++++++++++++++++------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/crates/libcontainer/src/namespaces.rs b/crates/libcontainer/src/namespaces.rs index 72417a9e6..f28e8bf22 100644 --- a/crates/libcontainer/src/namespaces.rs +++ b/crates/libcontainer/src/namespaces.rs @@ -7,12 +7,31 @@ //! UTS (hostname and domain information, processes will think they're running on servers with different names), //! Cgroup (Resource limits, execution priority etc.) -use crate::syscall::{syscall::create_syscall, Syscall}; -use anyhow::{Context, Result}; +use crate::syscall::{syscall::create_syscall, Syscall, SyscallError}; use nix::{fcntl, sched::CloneFlags, sys::stat, unistd}; use oci_spec::runtime::{LinuxNamespace, LinuxNamespaceType}; use std::collections; +type Result = std::result::Result; + +#[derive(Debug, thiserror::Error)] +enum UnshareError { + #[error("syscall failed")] + SyscallFailed(#[from] SyscallError), + #[error("nix syscall failed")] + NixSyscallFailed(#[from] nix::Error), +} + +#[derive(Debug, thiserror::Error)] +pub enum NamespaceError { + #[error("failed to set namespace")] + ApplyNamespace { + namespace: LinuxNamespaceType, + namespace_type: CloneFlags, + err: UnshareError, + }, +} + static ORDERED_NAMESPACES: &[CloneFlags] = &[ CloneFlags::CLONE_NEWUSER, CloneFlags::CLONE_NEWPID, @@ -67,23 +86,27 @@ impl Namespaces { for (ns_type, ns) in to_enter { self.unshare_or_setns(ns) - .with_context(|| format!("failed to enter {ns_type:?} namespace: {ns:?}"))?; + .map_err(|err| NamespaceError::ApplyNamespace { + namespace: ns.typ(), + namespace_type: *ns_type, + err, + })?; } Ok(()) } - pub fn unshare_or_setns(&self, namespace: &LinuxNamespace) -> Result<()> { + pub fn unshare_or_setns( + &self, + namespace: &LinuxNamespace, + ) -> std::result::Result<(), UnshareError> { log::debug!("unshare or setns: {:?}", namespace); if namespace.path().is_none() { self.command.unshare(get_clone_flag(namespace.typ()))?; } else { let ns_path = namespace.path().as_ref().unwrap(); - let fd = fcntl::open(ns_path, fcntl::OFlag::empty(), stat::Mode::empty()) - .with_context(|| format!("failed to open namespace fd: {ns_path:?}"))?; - self.command - .set_ns(fd, get_clone_flag(namespace.typ())) - .with_context(|| "failed to set namespace")?; - unistd::close(fd).with_context(|| "failed to close namespace fd")?; + let fd = fcntl::open(ns_path, fcntl::OFlag::empty(), stat::Mode::empty())?; + self.command.set_ns(fd, get_clone_flag(namespace.typ()))?; + unistd::close(fd)?; } Ok(()) From 294fc001ccddcd3aa63fb0d10f56ed33266bacf9 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Wed, 3 May 2023 16:58:27 +0000 Subject: [PATCH 4/7] Implemented rest of the thiserror for process, syscall, namespace and etc. Signed-off-by: yihuaf --- crates/libcontainer/src/capabilities.rs | 10 +- crates/libcontainer/src/lib.rs | 85 +++---- crates/libcontainer/src/namespaces.rs | 78 +++--- crates/libcontainer/src/process/channel.rs | 240 ++++++++++++------ .../src/process/container_init_process.rs | 104 +++++--- .../process/container_intermediate_process.rs | 74 ++++-- crates/libcontainer/src/process/fork.rs | 14 +- crates/libcontainer/src/process/message.rs | 17 +- crates/libcontainer/src/process/mod.rs | 43 ++++ crates/libcontainer/src/rootfs/mount.rs | 4 +- crates/libcontainer/src/syscall/linux.rs | 6 +- crates/libcontainer/src/syscall/mod.rs | 8 +- crates/libcontainer/src/syscall/syscall.rs | 4 +- crates/libcontainer/src/syscall/test.rs | 2 +- 14 files changed, 448 insertions(+), 241 deletions(-) diff --git a/crates/libcontainer/src/capabilities.rs b/crates/libcontainer/src/capabilities.rs index a109c5a10..6174428af 100644 --- a/crates/libcontainer/src/capabilities.rs +++ b/crates/libcontainer/src/capabilities.rs @@ -1,9 +1,8 @@ //! Handles Management of Capabilities -use crate::syscall::Syscall; +use crate::syscall::{Syscall, SyscallError}; use caps::Capability as CapsCapability; use caps::*; -use anyhow::Result; use oci_spec::runtime::{Capabilities, Capability as SpecCapability, LinuxCapabilities}; /// Converts a list of capability types to capabilities has set @@ -124,14 +123,17 @@ impl CapabilityExt for SpecCapability { /// reset capabilities of process calling this to effective capabilities /// effective capability set is set of capabilities used by kernel to perform checks /// see for more information -pub fn reset_effective(syscall: &S) -> Result<()> { +pub fn reset_effective(syscall: &S) -> Result<(), SyscallError> { log::debug!("reset all caps"); syscall.set_capability(CapSet::Effective, &caps::all())?; Ok(()) } /// Drop any extra granted capabilities, and reset to defaults which are in oci specification -pub fn drop_privileges(cs: &LinuxCapabilities, syscall: &S) -> Result<()> { +pub fn drop_privileges( + cs: &LinuxCapabilities, + syscall: &S, +) -> Result<(), SyscallError> { log::debug!("dropping bounding capabilities to {:?}", cs.bounding()); if let Some(bounding) = cs.bounding() { syscall.set_capability(CapSet::Bounding, &to_set(bounding))?; diff --git a/crates/libcontainer/src/lib.rs b/crates/libcontainer/src/lib.rs index 4bd7245cd..a460fac83 100644 --- a/crates/libcontainer/src/lib.rs +++ b/crates/libcontainer/src/lib.rs @@ -15,54 +15,43 @@ pub mod syscall; pub mod tty; pub mod utils; pub mod workload; -use std::{any::Any, result::Result as StdResult}; -use thiserror::Error as ThisError; -pub type Result = StdResult; +// pub type Result = StdResult; -#[derive(ThisError, Debug)] -pub enum LibcontainerError { - #[error("unknown fatal error {0}")] - UnknownLegacy(#[from] anyhow::Error), - #[error("unknown fatal error {0}")] - UnknownWithMsg(String), - #[error("unknown fatal error")] - Unknown, - #[error("io error: {0}")] - Io(#[from] std::io::Error), - #[error("io error: {0}")] - UnixIo(#[from] nix::errno::Errno), - #[error("failed to clone process using clone3")] - CloneFailed{ - errno: nix::errno::Errno, - child_name: String, - }, - #[error("failed to add task {pid} to cgroup manager")] - CgroupAdd { - pid: nix::unistd::Pid, - err: anyhow::Error, - }, - #[error("failed to apply resource limits to cgroup")] - CgroupApply(anyhow::Error), - #[error("failed to get proc state")] - Procfs(#[from] procfs::ProcError), -} +// #[derive(ThisError, Debug)] +// pub enum LibcontainerError { +// #[error("unknown fatal error {0}")] +// UnknownLegacy(#[from] anyhow::Error), +// #[error("unknown fatal error {0}")] +// UnknownWithMsg(String), +// #[error("unknown fatal error")] +// Unknown, +// #[error("io error: {0}")] +// Io(#[from] std::io::Error), +// #[error("io error: {0}")] +// UnixIo(#[from] nix::errno::Errno), +// #[error("failed to clone process using clone3")] +// CloneFailed{ +// errno: nix::errno::Errno, +// child_name: String, +// }, +// } -impl From> for LibcontainerError { - fn from(e: Box) -> Self { - if e.downcast_ref::().is_none() { - match e.downcast_ref::<&'static str>() { - Some(s) => LibcontainerError::UnknownWithMsg(s.to_string()), - None => match e.downcast_ref::() { - Some(s) => LibcontainerError::UnknownWithMsg(s.into()), - None => LibcontainerError::Unknown, - }, - } - } else { - match e.downcast::() { - Ok(ae) => *ae, - Err(_) => LibcontainerError::Unknown, - } - } - } -} +// impl From> for LibcontainerError { +// fn from(e: Box) -> Self { +// if e.downcast_ref::().is_none() { +// match e.downcast_ref::<&'static str>() { +// Some(s) => LibcontainerError::UnknownWithMsg(s.to_string()), +// None => match e.downcast_ref::() { +// Some(s) => LibcontainerError::UnknownWithMsg(s.into()), +// None => LibcontainerError::Unknown, +// }, +// } +// } else { +// match e.downcast::() { +// Ok(ae) => *ae, +// Err(_) => LibcontainerError::Unknown, +// } +// } +// } +// } diff --git a/crates/libcontainer/src/namespaces.rs b/crates/libcontainer/src/namespaces.rs index f28e8bf22..16a92a3b8 100644 --- a/crates/libcontainer/src/namespaces.rs +++ b/crates/libcontainer/src/namespaces.rs @@ -14,21 +14,27 @@ use std::collections; type Result = std::result::Result; -#[derive(Debug, thiserror::Error)] -enum UnshareError { - #[error("syscall failed")] - SyscallFailed(#[from] SyscallError), - #[error("nix syscall failed")] - NixSyscallFailed(#[from] nix::Error), -} +// #[derive(Debug, thiserror::Error)] +// enum UnshareError { +// #[error("syscall failed")] +// SyscallFailed(#[from] SyscallError), +// #[error("nix syscall failed")] +// NixSyscallFailed(#[from] nix::Error), +// } #[derive(Debug, thiserror::Error)] pub enum NamespaceError { #[error("failed to set namespace")] - ApplyNamespace { - namespace: LinuxNamespaceType, - namespace_type: CloneFlags, - err: UnshareError, + ApplyNamespaceSyscallFailed { + namespace: Box, + #[source] + err: SyscallError, + }, + #[error("failed to set namespace")] + ApplyNamespaceUnixSyscallFailed { + namespace: Box, + #[source] + err: nix::Error, }, } @@ -84,29 +90,43 @@ impl Namespaces { .filter_map(|c| self.namespace_map.get_key_value(c)) .collect(); - for (ns_type, ns) in to_enter { - self.unshare_or_setns(ns) - .map_err(|err| NamespaceError::ApplyNamespace { - namespace: ns.typ(), - namespace_type: *ns_type, - err, - })?; + for (_, ns) in to_enter { + self.unshare_or_setns(ns)?; } Ok(()) } - pub fn unshare_or_setns( - &self, - namespace: &LinuxNamespace, - ) -> std::result::Result<(), UnshareError> { + pub fn unshare_or_setns(&self, namespace: &LinuxNamespace) -> Result<()> { log::debug!("unshare or setns: {:?}", namespace); - if namespace.path().is_none() { - self.command.unshare(get_clone_flag(namespace.typ()))?; - } else { - let ns_path = namespace.path().as_ref().unwrap(); - let fd = fcntl::open(ns_path, fcntl::OFlag::empty(), stat::Mode::empty())?; - self.command.set_ns(fd, get_clone_flag(namespace.typ()))?; - unistd::close(fd)?; + match namespace.path() { + Some(path) => { + let fd = fcntl::open(path, fcntl::OFlag::empty(), stat::Mode::empty()).map_err( + |err| NamespaceError::ApplyNamespaceUnixSyscallFailed { + namespace: Box::new(namespace.to_owned()), + err, + }, + )?; + self.command + .set_ns(fd, get_clone_flag(namespace.typ())) + .map_err(|err| NamespaceError::ApplyNamespaceSyscallFailed { + namespace: Box::new(namespace.to_owned()), + err, + })?; + unistd::close(fd).map_err(|err| { + NamespaceError::ApplyNamespaceUnixSyscallFailed { + namespace: Box::new(namespace.to_owned()), + err, + } + })?; + } + None => { + self.command + .unshare(get_clone_flag(namespace.typ())) + .map_err(|err| NamespaceError::ApplyNamespaceSyscallFailed { + namespace: Box::new(namespace.to_owned()), + err, + })?; + } } Ok(()) diff --git a/crates/libcontainer/src/process/channel.rs b/crates/libcontainer/src/process/channel.rs index d049e6d7d..c4db0ccf2 100644 --- a/crates/libcontainer/src/process/channel.rs +++ b/crates/libcontainer/src/process/channel.rs @@ -1,5 +1,4 @@ use crate::process::message::Message; -use anyhow::{bail, Context, Result}; use nix::{ sys::socket::{self, UnixAddr}, unistd::{self, Pid}, @@ -11,6 +10,37 @@ use std::{ os::unix::prelude::{AsRawFd, RawFd}, }; +#[derive(Debug, thiserror::Error)] +pub enum ChannelError { + #[error("received unexpected message: {received:?}, expected: {expected:?}")] + UnexpectedMessage { + expected: Message, + received: Message, + }, + #[error("failed to receive. {msg:?}. {source:?}")] + ReceiveError { + msg: String, + #[source] + source: BaseChannelError, + }, + #[error(transparent)] + BaseChannelError(#[from] BaseChannelError), + #[error("missing fds from seccomp request")] + MissingSeccompFds, + #[error("exec process failed with error {0}")] + ExecError(String), +} + +#[derive(Debug, thiserror::Error)] +pub enum BaseChannelError { + #[error("failed unix syscalls")] + UnixError(#[from] nix::Error), + #[error("failed serde serialization")] + SerdeError(#[from] serde_json::Error), + #[error("channel connection broken")] + BrokenChannel, +} + /// Channel Design /// /// Each of the main, intermediate, and init process will have a uni-directional @@ -22,7 +52,7 @@ use std::{ /// processes will share the main_sender and use it to send message to the main /// process. -pub fn main_channel() -> Result<(MainSender, MainReceiver)> { +pub fn main_channel() -> Result<(MainSender, MainReceiver), ChannelError> { let (sender, receiver) = channel::()?; Ok((MainSender { sender }, MainReceiver { receiver })) } @@ -34,21 +64,21 @@ pub struct MainSender { impl MainSender { // requests the Main to write the id mappings for the intermediate process // this needs to be done from the parent see https://man7.org/linux/man-pages/man7/user_namespaces.7.html - pub fn identifier_mapping_request(&mut self) -> Result<()> { + pub fn identifier_mapping_request(&mut self) -> Result<(), ChannelError> { log::debug!("send identifier mapping request"); self.sender.send(Message::WriteMapping)?; Ok(()) } - pub fn seccomp_notify_request(&mut self, fd: RawFd) -> Result<()> { + pub fn seccomp_notify_request(&mut self, fd: RawFd) -> Result<(), ChannelError> { self.sender .send_fds(Message::SeccompNotify, &[fd.as_raw_fd()])?; Ok(()) } - pub fn intermediate_ready(&mut self, pid: Pid) -> Result<()> { + pub fn intermediate_ready(&mut self, pid: Pid) -> Result<(), ChannelError> { // Send over the IntermediateReady follow by the pid. log::debug!("sending init pid ({:?})", pid); self.sender.send(Message::IntermediateReady(pid.as_raw()))?; @@ -56,19 +86,21 @@ impl MainSender { Ok(()) } - pub fn init_ready(&mut self) -> Result<()> { + pub fn init_ready(&mut self) -> Result<(), ChannelError> { self.sender.send(Message::InitReady)?; Ok(()) } - pub fn exec_failed(&mut self, err: String) -> Result<()> { + pub fn exec_failed(&mut self, err: String) -> Result<(), ChannelError> { self.sender.send(Message::ExecFailed(err))?; Ok(()) } - pub fn close(&self) -> Result<()> { - self.sender.close() + pub fn close(&self) -> Result<(), ChannelError> { + self.sender.close()?; + + Ok(()) } } @@ -79,79 +111,98 @@ pub struct MainReceiver { impl MainReceiver { /// Waits for associated intermediate process to send ready message /// and return the pid of init process which is forked by intermediate process - pub fn wait_for_intermediate_ready(&mut self) -> Result { + pub fn wait_for_intermediate_ready(&mut self) -> Result { let msg = self .receiver .recv() - .context("failed to receive a message from the intermediate process")?; + .map_err(|err| ChannelError::ReceiveError { + msg: "waiting for intermediate process".to_string(), + source: err, + })?; match msg { Message::IntermediateReady(pid) => Ok(Pid::from_raw(pid)), - Message::ExecFailed(err) => bail!("exec process failed with error {}", err), - _ => bail!( - "receive unexpected message {:?} waiting for intermediate ready", - msg - ), + Message::ExecFailed(err) => Err(ChannelError::ExecError(err)), + msg => Err(ChannelError::UnexpectedMessage { + expected: Message::IntermediateReady(0), + received: msg, + }), } } - pub fn wait_for_mapping_request(&mut self) -> Result<()> { + pub fn wait_for_mapping_request(&mut self) -> Result<(), ChannelError> { let msg = self .receiver .recv() - .context("failed to wait for mapping request")?; + .map_err(|err| ChannelError::ReceiveError { + msg: "waiting for mapping request".to_string(), + source: err, + })?; match msg { Message::WriteMapping => Ok(()), - msg => bail!( - "receive unexpected message {:?} waiting for mapping request", - msg - ), + msg => Err(ChannelError::UnexpectedMessage { + expected: Message::WriteMapping, + received: msg, + }), } } - pub fn wait_for_seccomp_request(&mut self) -> Result { - let (msg, fds) = self - .receiver - .recv_with_fds::<[RawFd; 1]>() - .context("failed to wait for seccomp request")?; + pub fn wait_for_seccomp_request(&mut self) -> Result { + let (msg, fds) = self.receiver.recv_with_fds::<[RawFd; 1]>().map_err(|err| { + ChannelError::ReceiveError { + msg: "waiting for seccomp request".to_string(), + source: err, + } + })?; match msg { Message::SeccompNotify => { let fd = match fds { - Some(fds) => fds[0], - None => bail!("expecting fds from seccomp request"), - }; + Some(fds) => { + if fds.is_empty() { + Err(ChannelError::MissingSeccompFds) + } else { + Ok(fds[0]) + } + } + None => Err(ChannelError::MissingSeccompFds), + }?; Ok(fd) } - msg => bail!( - "receive unexpected message {:?} waiting for seccomp request", - msg - ), + msg => Err(ChannelError::UnexpectedMessage { + expected: Message::SeccompNotify, + received: msg, + }), } } /// Waits for associated init process to send ready message /// and return the pid of init process which is forked by init process - pub fn wait_for_init_ready(&mut self) -> Result<()> { + pub fn wait_for_init_ready(&mut self) -> Result<(), ChannelError> { let msg = self .receiver .recv() - .context("failed to wait for init ready")?; + .map_err(|err| ChannelError::ReceiveError { + msg: "waiting for init ready".to_string(), + source: err, + })?; match msg { Message::InitReady => Ok(()), - msg => bail!( - "receive unexpected message {:?} waiting for init ready", - msg - ), + msg => Err(ChannelError::UnexpectedMessage { + expected: Message::InitReady, + received: msg, + }), } } - pub fn close(&self) -> Result<()> { - self.receiver.close() + pub fn close(&self) -> Result<(), ChannelError> { + self.receiver.close()?; + + Ok(()) } } -pub fn intermediate_channel() -> Result<(IntermediateSender, IntermediateReceiver)> { +pub fn intermediate_channel() -> Result<(IntermediateSender, IntermediateReceiver), ChannelError> { let (sender, receiver) = channel::()?; Ok(( IntermediateSender { sender }, @@ -164,15 +215,17 @@ pub struct IntermediateSender { } impl IntermediateSender { - pub fn mapping_written(&mut self) -> Result<()> { + pub fn mapping_written(&mut self) -> Result<(), ChannelError> { log::debug!("identifier mapping written"); self.sender.send(Message::MappingWritten)?; Ok(()) } - pub fn close(&self) -> Result<()> { - self.sender.close() + pub fn close(&self) -> Result<(), ChannelError> { + self.sender.close()?; + + Ok(()) } } @@ -182,27 +235,32 @@ pub struct IntermediateReceiver { impl IntermediateReceiver { // wait until the parent process has finished writing the id mappings - pub fn wait_for_mapping_ack(&mut self) -> Result<()> { + pub fn wait_for_mapping_ack(&mut self) -> Result<(), ChannelError> { log::debug!("waiting for mapping ack"); let msg = self .receiver .recv() - .context("failed to wait for init ready")?; + .map_err(|err| ChannelError::ReceiveError { + msg: "waiting for mapping ack".to_string(), + source: err, + })?; match msg { Message::MappingWritten => Ok(()), - msg => bail!( - "receive unexpected message {:?} waiting for init ready", - msg - ), + msg => Err(ChannelError::UnexpectedMessage { + expected: Message::MappingWritten, + received: msg, + }), } } - pub fn close(&self) -> Result<()> { - self.receiver.close() + pub fn close(&self) -> Result<(), ChannelError> { + self.receiver.close()?; + + Ok(()) } } -pub fn init_channel() -> Result<(InitSender, InitReceiver)> { +pub fn init_channel() -> Result<(InitSender, InitReceiver), ChannelError> { let (sender, receiver) = channel::()?; Ok((InitSender { sender }, InitReceiver { receiver })) } @@ -212,14 +270,16 @@ pub struct InitSender { } impl InitSender { - pub fn seccomp_notify_done(&mut self) -> Result<()> { + pub fn seccomp_notify_done(&mut self) -> Result<(), ChannelError> { self.sender.send(Message::SeccompNotifyDone)?; Ok(()) } - pub fn close(&self) -> Result<()> { - self.sender.close() + pub fn close(&self) -> Result<(), ChannelError> { + self.sender.close()?; + + Ok(()) } } @@ -228,23 +288,28 @@ pub struct InitReceiver { } impl InitReceiver { - pub fn wait_for_seccomp_request_done(&mut self) -> Result<()> { + pub fn wait_for_seccomp_request_done(&mut self) -> Result<(), ChannelError> { let msg = self .receiver .recv() - .context("failed to wait for seccomp request")?; + .map_err(|err| ChannelError::ReceiveError { + msg: "waiting for seccomp request".to_string(), + source: err, + })?; match msg { Message::SeccompNotifyDone => Ok(()), - msg => bail!( - "receive unexpected message {:?} waiting for seccomp done request", - msg - ), + msg => Err(ChannelError::UnexpectedMessage { + expected: Message::SeccompNotifyDone, + received: msg, + }), } } - pub fn close(&self) -> Result<()> { - self.receiver.close() + pub fn close(&self) -> Result<(), ChannelError> { + self.receiver.close()?; + + Ok(()) } } @@ -262,7 +327,11 @@ impl Sender where T: Serialize, { - fn send_iovec(&mut self, iov: &[IoSlice], fds: Option<&[RawFd]>) -> Result { + fn send_iovec( + &mut self, + iov: &[IoSlice], + fds: Option<&[RawFd]>, + ) -> Result { let cmsgs = if let Some(fds) = fds { vec![socket::ControlMessage::ScmRights(fds)] } else { @@ -272,7 +341,11 @@ where .map_err(|e| e.into()) } - fn send_slice_with_len(&mut self, data: &[u8], fds: Option<&[RawFd]>) -> Result { + fn send_slice_with_len( + &mut self, + data: &[u8], + fds: Option<&[RawFd]>, + ) -> Result { let len = data.len() as u64; // Here we prefix the length of the data onto the serialized data. let iov = [ @@ -287,21 +360,21 @@ where self.send_iovec(&iov[..], fds) } - pub fn send(&mut self, object: T) -> Result<()> { + pub fn send(&mut self, object: T) -> Result<(), BaseChannelError> { let payload = serde_json::to_vec(&object)?; self.send_slice_with_len(&payload, None)?; Ok(()) } - pub fn send_fds(&mut self, object: T, fds: &[RawFd]) -> Result<()> { + pub fn send_fds(&mut self, object: T, fds: &[RawFd]) -> Result<(), BaseChannelError> { let payload = serde_json::to_vec(&object)?; self.send_slice_with_len(&payload, Some(fds))?; Ok(()) } - pub fn close(&self) -> Result<()> { + pub fn close(&self) -> Result<(), BaseChannelError> { Ok(unistd::close(self.sender)?) } } @@ -310,7 +383,7 @@ impl Receiver where T: serde::de::DeserializeOwned, { - fn peek_size_iovec(&mut self) -> Result { + fn peek_size_iovec(&mut self) -> Result { let mut len: u64 = 0; let mut iov = [IoSliceMut::new(unsafe { std::slice::from_raw_parts_mut( @@ -321,12 +394,15 @@ where let _ = socket::recvmsg::(self.receiver, &mut iov, None, socket::MsgFlags::MSG_PEEK)?; match len { - 0 => bail!("channel connection broken"), + 0 => Err(BaseChannelError::BrokenChannel), _ => Ok(len), } } - fn recv_into_iovec(&mut self, iov: &mut [IoSliceMut]) -> Result<(usize, Option)> + fn recv_into_iovec( + &mut self, + iov: &mut [IoSliceMut], + ) -> Result<(usize, Option), BaseChannelError> where F: Default + AsMut<[RawFd]>, { @@ -361,7 +437,7 @@ where Ok((msg.bytes, fds)) } - fn recv_into_buf_with_len(&mut self) -> Result<(Vec, Option)> + fn recv_into_buf_with_len(&mut self) -> Result<(Vec, Option), BaseChannelError> where F: Default + AsMut<[RawFd]>, { @@ -382,13 +458,13 @@ where }; match bytes { - 0 => bail!("channel connection broken"), + 0 => Err(BaseChannelError::BrokenChannel), _ => Ok((buf, fds)), } } // Recv the next message of type T. - pub fn recv(&mut self) -> Result { + pub fn recv(&mut self) -> Result { let (buf, _) = self.recv_into_buf_with_len::<[RawFd; 0]>()?; Ok(serde_json::from_slice(&buf[..])?) } @@ -396,7 +472,7 @@ where // Works similar to `recv`, but will look for fds sent by SCM_RIGHTS // message. We use F as as `[RawFd; n]`, where `n` is the number of // descriptors you want to receive. - pub fn recv_with_fds(&mut self) -> Result<(T, Option)> + pub fn recv_with_fds(&mut self) -> Result<(T, Option), BaseChannelError> where F: Default + AsMut<[RawFd]>, { @@ -404,12 +480,12 @@ where Ok((serde_json::from_slice(&buf[..])?, fds)) } - pub fn close(&self) -> Result<()> { + pub fn close(&self) -> Result<(), BaseChannelError> { Ok(unistd::close(self.receiver)?) } } -pub fn channel() -> Result<(Sender, Receiver)> +pub fn channel() -> Result<(Sender, Receiver), BaseChannelError> where T: for<'de> Deserialize<'de> + Serialize, { @@ -426,7 +502,7 @@ where } // Use socketpair as the underlying pipe. -fn unix_channel() -> Result<(RawFd, RawFd)> { +fn unix_channel() -> Result<(RawFd, RawFd), BaseChannelError> { Ok(socket::socketpair( socket::AddressFamily::Unix, socket::SockType::SeqPacket, @@ -438,7 +514,7 @@ fn unix_channel() -> Result<(RawFd, RawFd)> { #[cfg(test)] mod tests { use super::*; - use anyhow::Context; + use anyhow::{Context, Result}; use nix::sys::wait; use nix::unistd; use serial_test::serial; diff --git a/crates/libcontainer/src/process/container_init_process.rs b/crates/libcontainer/src/process/container_init_process.rs index b31cc4701..dd6b210fb 100644 --- a/crates/libcontainer/src/process/container_init_process.rs +++ b/crates/libcontainer/src/process/container_init_process.rs @@ -1,6 +1,6 @@ use super::args::{ContainerArgs, ContainerType}; use crate::apparmor; -use crate::syscall::Syscall; +use crate::syscall::{Syscall, SyscallError}; use crate::{ capabilities, hooks, namespaces::Namespaces, process::channel, rootfs::RootFS, rootless::Rootless, tty, utils, @@ -53,13 +53,15 @@ fn readonly_path(path: &Path, syscall: &dyn Syscall) -> Result<()> { MsFlags::MS_BIND | MsFlags::MS_REC, None, ) { - if let Some(errno) = err.downcast_ref() { - // ignore error if path is not exist. - if matches!(errno, nix::errno::Errno::ENOENT) { - return Ok(()); + match err { + SyscallError::MountFailed { errno, .. } => { + // ignore error if path is not exist. + if matches!(errno, nix::errno::Errno::ENOENT) { + return Ok(()); + } } + _ => bail!(err), } - bail!(err) } syscall.mount( @@ -82,33 +84,39 @@ fn readonly_path(path: &Path, syscall: &dyn Syscall) -> Result<()> { // For files, bind mounts /dev/null over the top of the specified path. // For directories, mounts read-only tmpfs over the top of the specified path. fn masked_path(path: &Path, mount_label: &Option, syscall: &dyn Syscall) -> Result<()> { - if let Err(e) = syscall.mount( + if let Err(err) = syscall.mount( Some(Path::new("/dev/null")), path, None, MsFlags::MS_BIND, None, ) { - if let Some(errno) = e.downcast_ref() { - if matches!(errno, nix::errno::Errno::ENOENT) { - log::warn!("masked path {:?} not exist", path); - } else if matches!(errno, nix::errno::Errno::ENOTDIR) { - let label = match mount_label { - Some(l) => format!("context=\"{l}\""), - None => "".to_string(), - }; - syscall.mount( - Some(Path::new("tmpfs")), - path, - Some("tmpfs"), - MsFlags::MS_RDONLY, - Some(label.as_str()), - )?; - } - } else { - bail!(e) + match err { + SyscallError::MountFailed { errno, .. } => match errno { + nix::errno::Errno::ENOENT => { + log::warn!("masked path {:?} not exist", path); + } + nix::errno::Errno::ENOTDIR => { + let label = match mount_label { + Some(l) => format!("context=\"{l}\""), + None => "".to_string(), + }; + syscall.mount( + Some(Path::new("tmpfs")), + path, + Some("tmpfs"), + MsFlags::MS_RDONLY, + Some(label.as_str()), + )?; + } + _ => { + bail!(err) + } + }, + _ => bail!(err), } - }; + } + Ok(()) } @@ -731,7 +739,16 @@ mod tests { .as_any() .downcast_ref::() .unwrap(); - mocks.set_ret_err(ArgName::Mount, || bail!(nix::errno::Errno::ENOENT)); + mocks.set_ret_err(ArgName::Mount, || { + Err(SyscallError::MountFailed { + mount_source: None, + mount_target: PathBuf::new(), + fstype: None, + flags: MsFlags::empty(), + data: None, + errno: nix::errno::Errno::ENOENT, + }) + }); assert!(masked_path(Path::new("/proc/self"), &None, syscall.as_ref()).is_ok()); let got = mocks.get_mount_args(); @@ -745,7 +762,16 @@ mod tests { .as_any() .downcast_ref::() .unwrap(); - mocks.set_ret_err(ArgName::Mount, || bail!(nix::errno::Errno::ENOTDIR)); + mocks.set_ret_err(ArgName::Mount, || { + Err(SyscallError::MountFailed { + mount_source: None, + mount_target: PathBuf::new(), + fstype: None, + flags: MsFlags::empty(), + data: None, + errno: nix::errno::Errno::ENOTDIR, + }) + }); assert!(masked_path(Path::new("/proc/self"), &None, syscall.as_ref()).is_ok()); @@ -768,7 +794,16 @@ mod tests { .as_any() .downcast_ref::() .unwrap(); - mocks.set_ret_err(ArgName::Mount, || bail!(nix::errno::Errno::ENOTDIR)); + mocks.set_ret_err(ArgName::Mount, || { + Err(SyscallError::MountFailed { + mount_source: None, + mount_target: PathBuf::new(), + fstype: None, + flags: MsFlags::empty(), + data: None, + errno: nix::errno::Errno::ENOTDIR, + }) + }); assert!(masked_path( Path::new("/proc/self"), @@ -796,7 +831,16 @@ mod tests { .as_any() .downcast_ref::() .unwrap(); - mocks.set_ret_err(ArgName::Mount, || bail!("unknown error")); + mocks.set_ret_err(ArgName::Mount, || { + Err(SyscallError::MountFailed { + mount_source: None, + mount_target: PathBuf::new(), + fstype: None, + flags: MsFlags::empty(), + data: None, + errno: nix::errno::Errno::UnknownErrno, + }) + }); assert!(masked_path(Path::new("/proc/self"), &None, syscall.as_ref()).is_err()); let got = mocks.get_mount_args(); diff --git a/crates/libcontainer/src/process/container_intermediate_process.rs b/crates/libcontainer/src/process/container_intermediate_process.rs index 284a4accd..1e72ffbc3 100644 --- a/crates/libcontainer/src/process/container_intermediate_process.rs +++ b/crates/libcontainer/src/process/container_intermediate_process.rs @@ -1,6 +1,5 @@ -use crate::{LibcontainerError, Result}; +use crate::process::{ProcessError, Result}; use crate::{namespaces::Namespaces, process::channel, process::fork}; -use anyhow::{Context}; use libcgroups::common::CgroupManager; use nix::unistd::{close, write}; use nix::unistd::{Gid, Pid, Uid}; @@ -21,7 +20,7 @@ pub fn container_intermediate_process( let (init_sender, init_receiver) = init_chan; let command = &args.syscall; let spec = &args.spec; - let linux = spec.linux().as_ref().context("no linux in spec")?; + let linux = spec.linux().as_ref().ok_or(ProcessError::NoLinuxSpec)?; let namespaces = Namespaces::from(linux.namespaces().as_ref()); // this needs to be done before we create the init process, so that the init @@ -38,24 +37,31 @@ pub fn container_intermediate_process( &args.cgroup_manager, linux.resources().as_ref(), matches!(args.container_type, ContainerType::InitContainer), - ) - .context("failed to apply cgroups")?; + )?; // if new user is specified in specification, this will be true and new // namespace will be created, check // https://man7.org/linux/man-pages/man7/user_namespaces.7.html for more // information if let Some(user_namespace) = namespaces.get(LinuxNamespaceType::User) { - namespaces - .unshare_or_setns(user_namespace) - .with_context(|| format!("failed to enter user namespace: {user_namespace:?}"))?; + namespaces.unshare_or_setns(user_namespace)?; if user_namespace.path().is_none() { log::debug!("creating new user namespace"); // child needs to be dumpable, otherwise the non root parent is not // allowed to write the uid/gid maps prctl::set_dumpable(true).unwrap(); - main_sender.identifier_mapping_request()?; - inter_receiver.wait_for_mapping_ack()?; + main_sender + .identifier_mapping_request() + .map_err(|err| ProcessError::ChannelError { + msg: "failed to send id mapping request".into(), + source: err, + })?; + inter_receiver + .wait_for_mapping_ack() + .map_err(|err| ProcessError::ChannelError { + msg: "failed to hear back mapping ack".into(), + source: err, + })?; prctl::set_dumpable(false).unwrap(); } @@ -65,24 +71,20 @@ pub fn container_intermediate_process( // configuring the container process will require root, even though the // root in the user namespace likely is mapped to an non-privileged user // on the parent user namespace. - command.set_id(Uid::from_raw(0), Gid::from_raw(0)).context( - "failed to configure uid and gid root in the beginning of a new user namespace", - )?; + command.set_id(Uid::from_raw(0), Gid::from_raw(0))?; } // set limits and namespaces to the process - let proc = spec.process().as_ref().context("no process in spec")?; + let proc = spec.process().as_ref().ok_or(ProcessError::NoProcessSpec)?; if let Some(rlimits) = proc.rlimits() { for rlimit in rlimits { - command.set_rlimit(rlimit).context("failed to set rlimit")?; + command.set_rlimit(rlimit)?; } } // Pid namespace requires an extra fork to enter, so we enter pid namespace now. if let Some(pid_namespace) = namespaces.get(LinuxNamespaceType::Pid) { - namespaces - .unshare_or_setns(pid_namespace) - .with_context(|| format!("failed to enter pid namespace: {pid_namespace:?}"))?; + namespaces.unshare_or_setns(pid_namespace)?; } // We have to record the pid of the init process. The init process will be @@ -99,10 +101,16 @@ pub fn container_intermediate_process( // the socket. init_sender .close() - .context("failed to close receiver in init process")?; + .map_err(|err| ProcessError::ChannelError { + msg: "failed to close receiver in init process".into(), + source: err, + })?; inter_sender .close() - .context("failed to close sender in the intermediate process")?; + .map_err(|err| ProcessError::ChannelError { + msg: "failed to close sender in the intermediate process".into(), + source: err, + })?; match container_init_process(args, main_sender, init_receiver) { Ok(_) => Ok(0), Err(e) => { @@ -111,7 +119,7 @@ pub fn container_intermediate_process( write(exec_notify_fd, buf.as_bytes())?; close(exec_notify_fd)?; } - Err(LibcontainerError::UnknownLegacy(e)) + Err(ProcessError::InitProcessFailed) } } })?; @@ -123,18 +131,30 @@ pub fn container_intermediate_process( main_sender .intermediate_ready(pid) - .context("failed to send child ready from intermediate process")?; + .map_err(|err| ProcessError::ChannelError { + msg: "failed to wait on intermediate channel".into(), + source: err, + })?; // Close unused senders here so we don't have lingering socket around. main_sender .close() - .context("failed to close unused main sender")?; + .map_err(|err| ProcessError::ChannelError { + msg: "failed to close unused main sender".into(), + source: err, + })?; inter_sender .close() - .context("failed to close sender in the intermediate process")?; + .map_err(|err| ProcessError::ChannelError { + msg: "failed to close sender in the intermediate process".into(), + source: err, + })?; init_sender .close() - .context("failed to close unused init sender")?; + .map_err(|err| ProcessError::ChannelError { + msg: "failed to close unused init sender".into(), + source: err, + })?; Ok(pid) } @@ -149,7 +169,7 @@ fn apply_cgroups< let pid = Pid::from_raw(Process::myself()?.pid()); cmanager .add_task(pid) - .map_err(|err| LibcontainerError::CgroupAdd { pid, err })?; + .map_err(|err| ProcessError::CgroupAdd { pid, err })?; if let Some(resources) = resources { if init { @@ -162,7 +182,7 @@ fn apply_cgroups< cmanager .apply(&controller_opt) - .map_err(|err| LibcontainerError::CgroupApply(err))?; + .map_err(ProcessError::CgroupApply)?; } } diff --git a/crates/libcontainer/src/process/fork.rs b/crates/libcontainer/src/process/fork.rs index 74ebfce28..ff39fda39 100644 --- a/crates/libcontainer/src/process/fork.rs +++ b/crates/libcontainer/src/process/fork.rs @@ -1,4 +1,4 @@ -use crate::{LibcontainerError, Result}; +use crate::process::{ProcessError, Result}; use libc::SIGCHLD; use nix::unistd::Pid; use prctl; @@ -35,12 +35,10 @@ fn container_clone Result>( // code returned by the callback. If there was any error when trying to run // callback, exit with -1 match unsafe { - clone_cmd - .call() - .map_err(|err| LibcontainerError::CloneFailed { - errno: nix::errno::from_i32(err.0), - child_name: child_name.to_string(), - })? + clone_cmd.call().map_err(|err| ProcessError::CloneFailed { + errno: nix::errno::from_i32(err.0), + child_name: child_name.to_string(), + })? } { 0 => { prctl::set_name(child_name).expect("failed to set name"); @@ -96,7 +94,7 @@ mod test { #[test] fn test_container_err_fork() -> Result<()> { - let pid = container_fork("test:child", || Err(LibcontainerError::Unknown))?; + let pid = container_fork("test:child", || Err(ProcessError::Unknown))?; match waitpid(pid, None).expect("wait pid failed.") { WaitStatus::Exited(p, status) => { assert_eq!(pid, p); diff --git a/crates/libcontainer/src/process/message.rs b/crates/libcontainer/src/process/message.rs index 8cdbaf8a8..8de9de7c8 100644 --- a/crates/libcontainer/src/process/message.rs +++ b/crates/libcontainer/src/process/message.rs @@ -1,6 +1,7 @@ -/// Used as a wrapper for messages to be sent between child and parent processes +use core::fmt; use serde::{Deserialize, Serialize}; +/// Used as a wrapper for messages to be sent between child and parent processes #[derive(Debug, Serialize, Deserialize)] pub enum Message { IntermediateReady(i32), @@ -11,3 +12,17 @@ pub enum Message { SeccompNotifyDone, ExecFailed(String), } + +impl fmt::Display for Message { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Message::IntermediateReady(pid) => write!(f, "IntermediateReady({})", pid), + Message::InitReady => write!(f, "InitReady"), + Message::WriteMapping => write!(f, "WriteMapping"), + Message::MappingWritten => write!(f, "MappingWritten"), + Message::SeccompNotify => write!(f, "SeccompNotify"), + Message::SeccompNotifyDone => write!(f, "SeccompNotifyDone"), + Message::ExecFailed(s) => write!(f, "ExecFailed({})", s), + } + } +} diff --git a/crates/libcontainer/src/process/mod.rs b/crates/libcontainer/src/process/mod.rs index 9251ecaab..b9f39d059 100644 --- a/crates/libcontainer/src/process/mod.rs +++ b/crates/libcontainer/src/process/mod.rs @@ -1,6 +1,8 @@ //! Provides a thin wrapper around fork syscall, //! with enums and functions specific to youki implemented +use crate::syscall::SyscallError; + pub mod args; pub mod channel; pub mod container_init_process; @@ -9,3 +11,44 @@ pub mod container_main_process; pub mod fork; pub mod intel_rdt; pub mod message; + +type Result = std::result::Result; + +#[derive(Debug, thiserror::Error)] +pub enum ProcessError { + #[error("unknown fatal error")] + Unknown, + #[error("failed to clone process using clone3")] + CloneFailed { + errno: nix::errno::Errno, + child_name: String, + }, + #[error("failed init process")] + InitProcessFailed, + #[error("failed intermediate process")] + IntermediateProcessFailed, + #[error("io error: {0}")] + UnixIo(#[from] nix::errno::Errno), + #[error("failed to add task {pid} to cgroup manager")] + CgroupAdd { + pid: nix::unistd::Pid, + err: anyhow::Error, + }, + #[error("failed to apply resource limits to cgroup")] + CgroupApply(anyhow::Error), + #[error("failed to get proc state")] + Procfs(#[from] procfs::ProcError), + #[error("missing linux in spec")] + NoLinuxSpec, + #[error("missing process in spec")] + NoProcessSpec, + #[error("channel error")] + ChannelError { + msg: String, + source: channel::ChannelError, + }, + #[error("syscall failed")] + SyscallFailed(#[from] SyscallError), + #[error("failed to enter namespace")] + NamespaceError(#[from] crate::namespaces::NamespaceError), +} diff --git a/crates/libcontainer/src/rootfs/mount.rs b/crates/libcontainer/src/rootfs/mount.rs index 3221142b8..418baee89 100644 --- a/crates/libcontainer/src/rootfs/mount.rs +++ b/crates/libcontainer/src/rootfs/mount.rs @@ -2,7 +2,7 @@ use super::symlink::Symlink; use super::utils::{find_parent_mount, parse_mount, MountOptionConfig}; use crate::{ - syscall::{linux, syscall::create_syscall, Syscall}, + syscall::{linux, syscall::create_syscall, Syscall, SyscallError}, utils, utils::PathBufExt, }; @@ -419,7 +419,7 @@ impl Mount { self.syscall .mount(Some(&*src), dest, typ, mount_option_config.flags, Some(&*d)) { - if let Some(errno) = err.downcast_ref() { + if let SyscallError::MountFailed { errno, .. } = err { if !matches!(errno, Errno::EINVAL) { bail!("mount of {:?} failed. {}", m.destination(), errno); } diff --git a/crates/libcontainer/src/syscall/linux.rs b/crates/libcontainer/src/syscall/linux.rs index c3adf44a7..c9f07ed2e 100644 --- a/crates/libcontainer/src/syscall/linux.rs +++ b/crates/libcontainer/src/syscall/linux.rs @@ -282,7 +282,7 @@ impl Syscall for LinuxSyscall { ) .map_err(|errno| SyscallError::PivotRootFailed { errno, - msg: format!("failed to make root directory rslave"), + msg: "failed to make root directory rslave".to_string(), path: format!("{:?}", path), })?; @@ -292,13 +292,13 @@ impl Syscall for LinuxSyscall { // see https://man7.org/linux/man-pages/man2/umount2.2.html for more information umount2("/", MntFlags::MNT_DETACH).map_err(|errno| SyscallError::PivotRootFailed { errno, - msg: format!("failed to unmount old root directory"), + msg: "failed to unmount old root directory".to_string(), path: format!("{:?}", path), })?; // Change directory to the new root fchdir(newroot).map_err(|errno| SyscallError::PivotRootFailed { errno, - msg: format!("failed to change directory to new root"), + msg: "failed to change directory to new root".to_string(), path: format!("{:?}", path), })?; diff --git a/crates/libcontainer/src/syscall/mod.rs b/crates/libcontainer/src/syscall/mod.rs index 6b2f57cc6..238fdac4f 100644 --- a/crates/libcontainer/src/syscall/mod.rs +++ b/crates/libcontainer/src/syscall/mod.rs @@ -18,12 +18,12 @@ pub enum SyscallError { value: bool, }, #[error("set hostname to {hostname} returned {errno}")] - SetHostnameFailed{ + SetHostnameFailed { errno: nix::errno::Errno, hostname: String, }, #[error("set domainname to {domainname} returned {errno}")] - SetDomainnameFailed{ + SetDomainnameFailed { errno: nix::errno::Errno, domainname: String, }, @@ -32,7 +32,7 @@ pub enum SyscallError { #[error("failed to get open fds: {0}")] GetOpenFdsFailed(std::io::Error), #[error("failed to pivot root")] - PivotRootFailed{ + PivotRootFailed { path: String, msg: String, errno: nix::errno::Errno, @@ -40,7 +40,7 @@ pub enum SyscallError { #[error("failed to setns: {0}")] SetNamespaceFailed(nix::errno::Errno), #[error("failed to set real gid to {gid}: {errno}")] - SetRealGidFailed{ + SetRealGidFailed { errno: nix::errno::Errno, gid: nix::unistd::Gid, }, diff --git a/crates/libcontainer/src/syscall/syscall.rs b/crates/libcontainer/src/syscall/syscall.rs index 20c807650..eb6f09cd0 100644 --- a/crates/libcontainer/src/syscall/syscall.rs +++ b/crates/libcontainer/src/syscall/syscall.rs @@ -1,7 +1,6 @@ //! An interface trait so that rest of Youki can call //! necessary functions without having to worry about their //! implementation details -use std::{any::Any, ffi::OsStr, path::Path, sync::Arc}; use bitflags::bitflags; use caps::{CapSet, CapsHashSet}; use libc; @@ -11,13 +10,14 @@ use nix::{ sys::stat::{Mode, SFlag}, unistd::{Gid, Uid}, }; +use std::{any::Any, ffi::OsStr, path::Path, sync::Arc}; use oci_spec::runtime::LinuxRlimit; use crate::syscall::{ linux::{LinuxSyscall, MountAttr}, - Result, test::TestHelperSyscall, + Result, }; /// This specifies various kernel/other functionalities required for diff --git a/crates/libcontainer/src/syscall/test.rs b/crates/libcontainer/src/syscall/test.rs index c6c63f584..696efc208 100644 --- a/crates/libcontainer/src/syscall/test.rs +++ b/crates/libcontainer/src/syscall/test.rs @@ -17,7 +17,7 @@ use nix::{ use oci_spec::runtime::LinuxRlimit; -use super::{linux, Syscall, Result}; +use super::{linux, Result, Syscall}; #[derive(Clone, PartialEq, Eq, Debug)] pub struct MountArgs { From f8db959cda8462923ae0842c3696e443c5655b94 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Wed, 3 May 2023 18:16:35 +0000 Subject: [PATCH 5/7] Fix libcgroup error Signed-off-by: yihuaf --- .../src/process/container_intermediate_process.rs | 9 +++++++-- crates/libcontainer/src/process/mod.rs | 7 ++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/libcontainer/src/process/container_intermediate_process.rs b/crates/libcontainer/src/process/container_intermediate_process.rs index 1e72ffbc3..aabdba447 100644 --- a/crates/libcontainer/src/process/container_intermediate_process.rs +++ b/crates/libcontainer/src/process/container_intermediate_process.rs @@ -169,7 +169,10 @@ fn apply_cgroups< let pid = Pid::from_raw(Process::myself()?.pid()); cmanager .add_task(pid) - .map_err(|err| ProcessError::CgroupAdd { pid, err })?; + .map_err(|err| ProcessError::CgroupAdd { + pid, + msg: err.to_string(), + })?; if let Some(resources) = resources { if init { @@ -182,7 +185,9 @@ fn apply_cgroups< cmanager .apply(&controller_opt) - .map_err(ProcessError::CgroupApply)?; + .map_err(|err| ProcessError::CgroupApply { + msg: err.to_string(), + })?; } } diff --git a/crates/libcontainer/src/process/mod.rs b/crates/libcontainer/src/process/mod.rs index b9f39d059..5962b6c01 100644 --- a/crates/libcontainer/src/process/mod.rs +++ b/crates/libcontainer/src/process/mod.rs @@ -30,12 +30,9 @@ pub enum ProcessError { #[error("io error: {0}")] UnixIo(#[from] nix::errno::Errno), #[error("failed to add task {pid} to cgroup manager")] - CgroupAdd { - pid: nix::unistd::Pid, - err: anyhow::Error, - }, + CgroupAdd { pid: nix::unistd::Pid, msg: String }, #[error("failed to apply resource limits to cgroup")] - CgroupApply(anyhow::Error), + CgroupApply { msg: String }, #[error("failed to get proc state")] Procfs(#[from] procfs::ProcError), #[error("missing linux in spec")] From 4d470bab2f4d8b6bc9aa126f4abd6e79a72f1e54 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Wed, 3 May 2023 18:18:40 +0000 Subject: [PATCH 6/7] remove dead code Signed-off-by: yihuaf --- crates/libcontainer/src/lib.rs | 40 ---------------------------------- 1 file changed, 40 deletions(-) diff --git a/crates/libcontainer/src/lib.rs b/crates/libcontainer/src/lib.rs index a460fac83..28a2402e3 100644 --- a/crates/libcontainer/src/lib.rs +++ b/crates/libcontainer/src/lib.rs @@ -15,43 +15,3 @@ pub mod syscall; pub mod tty; pub mod utils; pub mod workload; - -// pub type Result = StdResult; - -// #[derive(ThisError, Debug)] -// pub enum LibcontainerError { -// #[error("unknown fatal error {0}")] -// UnknownLegacy(#[from] anyhow::Error), -// #[error("unknown fatal error {0}")] -// UnknownWithMsg(String), -// #[error("unknown fatal error")] -// Unknown, -// #[error("io error: {0}")] -// Io(#[from] std::io::Error), -// #[error("io error: {0}")] -// UnixIo(#[from] nix::errno::Errno), -// #[error("failed to clone process using clone3")] -// CloneFailed{ -// errno: nix::errno::Errno, -// child_name: String, -// }, -// } - -// impl From> for LibcontainerError { -// fn from(e: Box) -> Self { -// if e.downcast_ref::().is_none() { -// match e.downcast_ref::<&'static str>() { -// Some(s) => LibcontainerError::UnknownWithMsg(s.to_string()), -// None => match e.downcast_ref::() { -// Some(s) => LibcontainerError::UnknownWithMsg(s.into()), -// None => LibcontainerError::Unknown, -// }, -// } -// } else { -// match e.downcast::() { -// Ok(ae) => *ae, -// Err(_) => LibcontainerError::Unknown, -// } -// } -// } -// } From 17537e910e18c30559897b311bbfa0f4a0f8fa21 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Wed, 3 May 2023 18:19:45 +0000 Subject: [PATCH 7/7] fix dead code Signed-off-by: yihuaf --- crates/libcontainer/src/namespaces.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/libcontainer/src/namespaces.rs b/crates/libcontainer/src/namespaces.rs index 16a92a3b8..3d5495de0 100644 --- a/crates/libcontainer/src/namespaces.rs +++ b/crates/libcontainer/src/namespaces.rs @@ -14,14 +14,6 @@ use std::collections; type Result = std::result::Result; -// #[derive(Debug, thiserror::Error)] -// enum UnshareError { -// #[error("syscall failed")] -// SyscallFailed(#[from] SyscallError), -// #[error("nix syscall failed")] -// NixSyscallFailed(#[from] nix::Error), -// } - #[derive(Debug, thiserror::Error)] pub enum NamespaceError { #[error("failed to set namespace")]