From 8a6a8c6e960c0d879d2dd0baf29d81d272f59e70 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Thu, 18 May 2023 21:24:17 -0700 Subject: [PATCH 1/2] simplified the syscall error Signed-off-by: yihuaf --- .../src/process/container_init_process.rs | 26 +- crates/libcontainer/src/rootfs/mount.rs | 2 +- crates/libcontainer/src/syscall/linux.rs | 250 +++++++++++------- crates/libcontainer/src/syscall/mod.rs | 72 +---- crates/libcontainer/src/syscall/syscall.rs | 8 - crates/libcontainer/src/utils.rs | 33 +-- 6 files changed, 178 insertions(+), 213 deletions(-) diff --git a/crates/libcontainer/src/process/container_init_process.rs b/crates/libcontainer/src/process/container_init_process.rs index 296875513..9a6c3598a 100644 --- a/crates/libcontainer/src/process/container_init_process.rs +++ b/crates/libcontainer/src/process/container_init_process.rs @@ -170,7 +170,7 @@ fn readonly_path(path: &Path, syscall: &dyn Syscall) -> Result<()> { MsFlags::MS_BIND | MsFlags::MS_REC, None, ) { - if let SyscallError::Mount { source: errno } = err { + if let SyscallError::Nix(errno) = err { // ignore error if path is not exist. if matches!(errno, nix::errno::Errno::ENOENT) { return Ok(()); @@ -214,15 +214,11 @@ fn masked_path(path: &Path, mount_label: &Option, syscall: &dyn Syscall) None, ) { match err { - SyscallError::Mount { - source: nix::errno::Errno::ENOENT, - } => { + SyscallError::Nix(nix::errno::Errno::ENOENT) => { // ignore error if path is not exist. tracing::warn!("masked path {:?} not exist", path); } - SyscallError::Mount { - source: nix::errno::Errno::ENOTDIR, - } => { + SyscallError::Nix(nix::errno::Errno::ENOTDIR) => { let label = match mount_label { Some(l) => format!("context=\"{l}\""), None => "".to_string(), @@ -968,9 +964,7 @@ mod tests { .downcast_ref::() .unwrap(); mocks.set_ret_err(ArgName::Mount, || { - Err(SyscallError::Mount { - source: nix::errno::Errno::ENOENT, - }) + Err(SyscallError::Nix(nix::errno::Errno::ENOENT)) }); assert!(masked_path(Path::new("/proc/self"), &None, syscall.as_ref()).is_ok()); @@ -986,9 +980,7 @@ mod tests { .downcast_ref::() .unwrap(); mocks.set_ret_err(ArgName::Mount, || { - Err(SyscallError::Mount { - source: nix::errno::Errno::ENOTDIR, - }) + Err(SyscallError::Nix(nix::errno::Errno::ENOTDIR)) }); assert!(masked_path(Path::new("/proc/self"), &None, syscall.as_ref()).is_ok()); @@ -1013,9 +1005,7 @@ mod tests { .downcast_ref::() .unwrap(); mocks.set_ret_err(ArgName::Mount, || { - Err(SyscallError::Mount { - source: nix::errno::Errno::ENOTDIR, - }) + Err(SyscallError::Nix(nix::errno::Errno::ENOTDIR)) }); assert!(masked_path( @@ -1045,9 +1035,7 @@ mod tests { .downcast_ref::() .unwrap(); mocks.set_ret_err(ArgName::Mount, || { - Err(SyscallError::Mount { - source: nix::errno::Errno::UnknownErrno, - }) + Err(SyscallError::Nix(nix::errno::Errno::UnknownErrno)) }); assert!(masked_path(Path::new("/proc/self"), &None, syscall.as_ref()).is_err()); diff --git a/crates/libcontainer/src/rootfs/mount.rs b/crates/libcontainer/src/rootfs/mount.rs index fee813553..647a71c69 100644 --- a/crates/libcontainer/src/rootfs/mount.rs +++ b/crates/libcontainer/src/rootfs/mount.rs @@ -522,7 +522,7 @@ impl Mount { self.syscall .mount(Some(&*src), dest, typ, mount_option_config.flags, Some(&*d)) { - if let SyscallError::Mount { source: errno, .. } = err { + if let SyscallError::Nix(errno) = err { if !matches!(errno, Errno::EINVAL) { tracing::error!("mount of {:?} failed. {}", m.destination(), errno); return Err(err.into()); diff --git a/crates/libcontainer/src/syscall/linux.rs b/crates/libcontainer/src/syscall/linux.rs index 5e7293bf4..c425a63e9 100644 --- a/crates/libcontainer/src/syscall/linux.rs +++ b/crates/libcontainer/src/syscall/linux.rs @@ -3,7 +3,6 @@ use caps::{CapSet, CapsHashSet}; use libc::{c_char, setdomainname, uid_t}; use nix::fcntl; use nix::{ - errno::Errno, fcntl::{open, OFlag}, mount::{mount, umount2, MntFlags, MsFlags}, sched::{unshare, CloneFlags}, @@ -20,10 +19,8 @@ use std::os::unix::io::RawFd; use std::str::FromStr; use std::sync::Arc; use std::{any::Any, mem, path::Path, ptr}; -use syscalls::{syscall, Sysno, Sysno::close_range}; use super::{Result, Syscall, SyscallError}; -use crate::syscall::syscall::CloseRange; use crate::{capabilities, utils}; // Flags used in mount_setattr(2). @@ -212,11 +209,19 @@ impl LinuxSyscall { // Get a list of open fds for the calling process. fn get_open_fds() -> Result> { const PROCFS_FD_PATH: &str = "/proc/self/fd"; - utils::ensure_procfs(Path::new(PROCFS_FD_PATH)) - .map_err(|_| SyscallError::NotProcfs(PROCFS_FD_PATH.to_string()))?; + utils::ensure_procfs(Path::new(PROCFS_FD_PATH)).map_err(|err| { + tracing::error!(?err, "failed to ensure /proc is mounted"); + match err { + utils::EnsureProcfsError::Nix(err) => SyscallError::Nix(err), + utils::EnsureProcfsError::IO(err) => SyscallError::IO(err), + } + })?; let fds: Vec = fs::read_dir(PROCFS_FD_PATH) - .map_err(SyscallError::GetOpenFds)? + .map_err(|err| { + tracing::error!(?err, "failed to read /proc/self/fd"); + err + })? .filter_map(|entry| match entry { Ok(entry) => Some(entry.path()), Err(_) => None, @@ -250,8 +255,8 @@ impl Syscall for LinuxSyscall { // open the path as directory and read only let newroot = open(path, OFlag::O_DIRECTORY | OFlag::O_RDONLY, Mode::empty()).map_err(|errno| { - tracing::error!("failed to open {:?}", path); - SyscallError::PivotRoot { source: errno } + tracing::error!(?errno, ?path, "failed to open the new root for pivot root"); + errno })?; // make the given path as the root directory for the container @@ -263,8 +268,8 @@ impl Syscall for LinuxSyscall { // 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).map_err(|errno| { - tracing::error!("failed to pivot root to {:?}", path); - SyscallError::PivotRoot { source: errno } + tracing::error!(?errno, ?path, "failed to pivot root to"); + errno })?; // Make the original root directory rslave to avoid propagating unmount event to the host mount namespace. @@ -277,8 +282,8 @@ impl Syscall for LinuxSyscall { None::<&str>, ) .map_err(|errno| { - tracing::error!("failed to make original root directory rslave"); - SyscallError::PivotRoot { source: errno } + tracing::error!(?errno, "failed to make original root directory rslave"); + errno })?; // Unmount the original root directory which was stacked on top of new root directory @@ -286,13 +291,13 @@ impl Syscall for LinuxSyscall { // 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).map_err(|errno| { - tracing::error!("failed to unmount old root directory"); - SyscallError::PivotRoot { source: errno } + tracing::error!(?errno, "failed to unmount old root directory"); + errno })?; // Change directory to the new root fchdir(newroot).map_err(|errno| { - tracing::error!("failed to change directory to new root"); - SyscallError::PivotRoot { source: errno } + tracing::error!(?errno, ?newroot, "failed to change directory to new root"); + errno })?; Ok(()) @@ -300,23 +305,33 @@ impl Syscall for LinuxSyscall { /// Set namespace for process fn set_ns(&self, rawfd: i32, nstype: CloneFlags) -> Result<()> { - nix::sched::setns(rawfd, nstype).map_err(SyscallError::SetNamespace)?; + nix::sched::setns(rawfd, nstype)?; Ok(()) } /// set uid and gid for process fn set_id(&self, uid: Uid, gid: Gid) -> Result<()> { prctl::set_keep_capabilities(true).map_err(|errno| { - SyscallError::PrctlSetKeepCapabilites { - errno: nix::errno::from_i32(errno), - value: true, - } + tracing::error!(?errno, "failed to set keep capabilities to true"); + nix::errno::from_i32(errno) })?; // args : real *id, effective *id, saved set *id respectively - unistd::setresgid(gid, gid, gid) - .map_err(|errno| SyscallError::SetRealGid { errno, gid })?; - unistd::setresuid(uid, uid, uid) - .map_err(|errno| SyscallError::SetRealUid { errno, uid })?; + unistd::setresgid(gid, gid, gid).map_err(|err| { + tracing::error!( + ?err, + ?gid, + "failed to set real, effective and saved set gid" + ); + err + })?; + unistd::setresuid(uid, uid, uid).map_err(|err| { + tracing::error!( + ?err, + ?uid, + "failed to set real, effective and saved set uid" + ); + err + })?; // if not the root user, reset capabilities to effective capabilities, // which are used by kernel to perform checks @@ -325,10 +340,8 @@ impl Syscall for LinuxSyscall { capabilities::reset_effective(self)?; } prctl::set_keep_capabilities(false).map_err(|errno| { - SyscallError::PrctlSetKeepCapabilites { - errno: nix::errno::from_i32(errno), - value: false, - } + tracing::error!(?errno, "failed to set keep capabilities to false"); + nix::errno::from_i32(errno) })?; Ok(()) } @@ -336,7 +349,12 @@ impl Syscall for LinuxSyscall { /// 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).map_err(SyscallError::Unshare) + unshare(flags).map_err(|err| { + tracing::error!(?err, ?flags, "failed to unshare"); + err + })?; + + Ok(()) } /// Set capabilities for container process @@ -364,10 +382,11 @@ impl Syscall for LinuxSyscall { /// Sets hostname for process fn set_hostname(&self, hostname: &str) -> Result<()> { - sethostname(hostname).map_err(|errno| SyscallError::SetHostname { - errno, - hostname: hostname.to_string(), - }) + sethostname(hostname).map_err(|err| { + tracing::error!(?hostname, "failed to set hostname"); + err + })?; + Ok(()) } /// Sets domainname for process (see @@ -375,19 +394,20 @@ impl Syscall for LinuxSyscall { fn set_domainname(&self, domainname: &str) -> Result<()> { let ptr = domainname.as_bytes().as_ptr() as *const c_char; let len = domainname.len(); - let res = unsafe { setdomainname(ptr, len) }; - match res { + match unsafe { setdomainname(ptr, len) } { 0 => Ok(()), - -1 => Err(SyscallError::SetDomainname { - errno: nix::errno::Errno::last(), - domainname: domainname.to_string(), - }), - - _ => Err(SyscallError::SetDomainname { - errno: nix::errno::Errno::UnknownErrno, - domainname: domainname.to_string(), - }), - } + -1 => { + tracing::error!(?domainname, "failed to set domainname"); + Err(nix::Error::last()) + } + + _ => { + tracing::error!(?domainname, "failed to set domainname for unknown reason"); + Err(nix::Error::UnknownErrno) + } + }?; + + Ok(()) } /// Sets resource limit for process @@ -403,12 +423,18 @@ impl Syscall for LinuxSyscall { #[cfg(target_env = "musl")] let res = unsafe { libc::setrlimit(rlimit.typ() as i32, rlim) }; - Errno::result(res) - .map(drop) - .map_err(|errno| SyscallError::SetRlimit { - errno, - rlimit: rlimit.typ(), - })?; + match res { + 0 => Ok(()), + -1 => { + tracing::error!(?res, rlimit = ?rlimit.typ(), "failed to set rlimit"); + Err(SyscallError::Nix(nix::Error::last())) + } + _ => { + tracing::error!(?res, rlimit = ?rlimit.typ(), "failed to set rlimit for unknown reason"); + Err(SyscallError::Nix(nix::Error::UnknownErrno)) + } + }?; + Ok(()) } @@ -447,7 +473,10 @@ impl Syscall for LinuxSyscall { } fn chroot(&self, path: &Path) -> Result<()> { - unistd::chroot(path).map_err(|errno| SyscallError::Chroot { source: errno })?; + unistd::chroot(path).map_err(|err| { + tracing::error!(?err, ?path, "failed to chroot"); + err + })?; Ok(()) } @@ -460,15 +489,23 @@ impl Syscall for LinuxSyscall { flags: MsFlags, data: Option<&str>, ) -> Result<()> { - mount(source, target, fstype, flags, data) - .map_err(|errno| SyscallError::Mount { source: errno }) + mount(source, target, fstype, flags, data).map_err(|err| { + tracing::error!( + "failed to mount {source:?} to {target:?} with fstype {fstype:?}, flags {flags:?}, data {data:?}: {err}", + ); + err + })?; + + Ok(()) } fn symlink(&self, original: &Path, link: &Path) -> Result<()> { symlink(original, link).map_err(|err| { tracing::error!("failed to create symlink from {original:?} to {link:?}: {err}"); - SyscallError::Symlink { source: err } - }) + err + })?; + + Ok(()) } fn mknod(&self, path: &Path, kind: SFlag, perm: Mode, dev: u64) -> Result<()> { @@ -476,42 +513,58 @@ impl Syscall for LinuxSyscall { tracing::error!( "failed to mknod {path:?}, kind {kind:?}, perm {perm:?}, dev {dev:?}: {errno}" ); - SyscallError::Mknod { source: errno } - }) + errno + })?; + + Ok(()) } fn chown(&self, path: &Path, owner: Option, group: Option) -> Result<()> { chown(path, owner, group).map_err(|errno| { tracing::error!("failed to chown {path:?} to {owner:?}:{group:?}: {errno}"); - SyscallError::Chown { source: errno } - }) + errno + })?; + + Ok(()) } fn set_groups(&self, groups: &[Gid]) -> Result<()> { - setgroups(groups).map_err(|errno| SyscallError::SetGroups { source: errno }) + setgroups(groups)?; + + Ok(()) } + #[tracing::instrument(skip(self))] fn close_range(&self, preserve_fds: i32) -> Result<()> { - let result = unsafe { - syscall!( - close_range, - 3 + preserve_fds as usize, - usize::MAX, - CloseRange::CLOEXEC.bits() + match unsafe { + libc::syscall( + libc::SYS_close_range, + 3 + preserve_fds, + libc::c_int::MAX, + libc::CLOSE_RANGE_CLOEXEC, ) - }; - match result { - Ok(_) => Ok(()), - Err(e) if e == syscalls::Errno::ENOSYS || e == syscalls::Errno::EINVAL => { - // close_range was introduced in kernel 5.9 and CLOSEEXEC was introduced in - // kernel 5.11. If the kernel is older we emulate close_range in userspace. - Self::emulate_close_range(preserve_fds) + } { + 0 => Ok(()), + -1 => { + match nix::errno::Errno::last() { + nix::errno::Errno::ENOSYS | nix::errno::Errno::EINVAL => { + // close_range was introduced in kernel 5.9 and CLOSEEXEC was introduced in + // kernel 5.11. If the kernel is older we emulate close_range in userspace. + Self::emulate_close_range(preserve_fds) + } + e => { + tracing::error!(errno = ?e, "failed to close_range"); + Err(SyscallError::Nix(e)) + } + } } - Err(e) => Err(SyscallError::CloseRange { - preserve_fds, - errno: e, - }), - } + _ => { + tracing::error!("failed to close_range unknown failure"); + Err(SyscallError::Nix(nix::errno::Errno::UnknownErrno)) + } + }?; + + Ok(()) } fn mount_setattr( @@ -525,28 +578,31 @@ impl Syscall for LinuxSyscall { 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 - // https://docs.rs/libc/0.2.139/libc/constant.SYS_mount_setattr.html - // https://docs.rs/libc/0.2.139/libc/fn.syscall.html - syscall!( - Sysno::mount_setattr, + .map(CString::new) + .ok_or_else(|| { + tracing::error!(path = ?pathname, "failed to convert path to string"); + nix::Error::EINVAL + })? + .map_err(|err| { + tracing::error!(path = ?pathname, ?err, "failed to convert path to string"); + nix::Error::EINVAL + })?; + + match unsafe { + libc::syscall( + libc::SYS_mount_setattr, dirfd, path_c_string.as_ptr(), flags, mount_attr as *const MountAttr, - size + size, ) - }; - - match result { - Ok(_) => Ok(()), - Err(e) => Err(SyscallError::MountSetattr { source: e }), - } + } { + 0 => Ok(()), + -1 => Err(nix::Error::last()), + _ => Err(nix::Error::UnknownErrno), + }?; + Ok(()) } } diff --git a/crates/libcontainer/src/syscall/mod.rs b/crates/libcontainer/src/syscall/mod.rs index e83f27b23..d27bf411a 100644 --- a/crates/libcontainer/src/syscall/mod.rs +++ b/crates/libcontainer/src/syscall/mod.rs @@ -12,76 +12,12 @@ pub use syscall::Syscall; pub enum SyscallError { #[error("unexpected mount attr option: {0}")] UnexpectedMountAttrOption(String), - #[error("set keep capabilities to {value}")] - PrctlSetKeepCapabilites { - #[source] - errno: nix::errno::Errno, - value: bool, - }, - #[error("set hostname to {hostname}")] - SetHostname { - #[source] - errno: nix::errno::Errno, - hostname: String, - }, - #[error("set domainname to {domainname}")] - SetDomainname { - #[source] - errno: nix::errno::Errno, - domainname: String, - }, - #[error("{0} is not an actual procfs")] - NotProcfs(String), - #[error("failed to get open fds")] - GetOpenFds(#[source] std::io::Error), - #[error("failed to pivot root")] - PivotRoot { source: nix::errno::Errno }, - #[error("failed to setns: {0}")] - SetNamespace(nix::errno::Errno), - #[error("failed to set real gid to {gid}")] - SetRealGid { - #[source] - errno: nix::errno::Errno, - gid: nix::unistd::Gid, - }, - #[error("failed to set real uid to {uid}: {errno}")] - SetRealUid { - #[source] - errno: nix::errno::Errno, - uid: nix::unistd::Uid, - }, - #[error("failed to unshare: {0}")] - Unshare(nix::errno::Errno), + #[error(transparent)] + Nix(#[from] nix::Error), + #[error(transparent)] + IO(#[from] std::io::Error), #[error("failed to set capabilities: {0}")] SetCaps(#[from] caps::errors::CapsError), - #[error("failed to set rlimit {rlimit:?}")] - SetRlimit { - #[source] - errno: nix::errno::Errno, - rlimit: oci_spec::runtime::LinuxRlimitType, - }, - #[error("failed to chroot: {source}")] - Chroot { source: nix::errno::Errno }, - #[error("mount failed")] - Mount { source: nix::errno::Errno }, - #[error("symlink failed")] - Symlink { source: std::io::Error }, - #[error("mknod failed")] - Mknod { source: nix::errno::Errno }, - #[error("chown failed")] - Chown { source: nix::errno::Errno }, - #[error("setgroups failed")] - SetGroups { source: nix::errno::Errno }, - #[error("close range failed")] - CloseRange { - preserve_fds: i32, - #[source] - errno: syscalls::Errno, - }, - #[error("invalid filename: {0:?}")] - InvalidFilename(std::path::PathBuf), - #[error("mount_setattr failed")] - MountSetattr { source: syscalls::Errno }, } type Result = std::result::Result; diff --git a/crates/libcontainer/src/syscall/syscall.rs b/crates/libcontainer/src/syscall/syscall.rs index eb6f09cd0..587963839 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 bitflags::bitflags; use caps::{CapSet, CapsHashSet}; use libc; use nix::{ @@ -64,10 +63,3 @@ pub fn create_syscall() -> Box { Box::new(LinuxSyscall) } } - -bitflags! { -pub struct CloseRange : u32 { - const NONE = 0b00000000; - const UNSHARE = 0b00000010; - const CLOEXEC = 0b00000100; -}} diff --git a/crates/libcontainer/src/utils.rs b/crates/libcontainer/src/utils.rs index 0c43bd12b..71c747bca 100644 --- a/crates/libcontainer/src/utils.rs +++ b/crates/libcontainer/src/utils.rs @@ -285,34 +285,27 @@ pub fn create_dir_all_with_mode>( #[derive(Debug, thiserror::Error)] pub enum EnsureProcfsError { - #[error("failed to open {path:?}")] - OpenProcfs { - source: std::io::Error, - path: PathBuf, - }, - #[error("failed to get statfs for {path:?}")] - StatfsProcfs { source: nix::Error, path: PathBuf }, - #[error("{path:?} is not on the procfs")] - NotProcfs { path: PathBuf }, + #[error(transparent)] + Nix(#[from] nix::Error), + #[error(transparent)] + IO(#[from] std::io::Error), } // Make sure a given path is on procfs. This is to avoid the security risk that // /proc path is mounted over. Ref: CVE-2019-16884 pub fn ensure_procfs(path: &Path) -> Result<(), EnsureProcfsError> { - let procfs_fd = fs::File::open(path).map_err(|err| EnsureProcfsError::OpenProcfs { - source: err, - path: path.to_path_buf(), + let procfs_fd = fs::File::open(path).map_err(|err| { + tracing::error!(?err, ?path, "failed to open procfs file"); + err + })?; + let fstat_info = statfs::fstatfs(&procfs_fd.as_raw_fd()).map_err(|err| { + tracing::error!(?err, ?path, "failed to fstatfs the procfs"); + err })?; - let fstat_info = - statfs::fstatfs(&procfs_fd.as_raw_fd()).map_err(|err| EnsureProcfsError::StatfsProcfs { - source: err, - path: path.to_path_buf(), - })?; if fstat_info.filesystem_type() != statfs::PROC_SUPER_MAGIC { - return Err(EnsureProcfsError::NotProcfs { - path: path.to_path_buf(), - }); + tracing::error!(?path, "given path is not on the procfs"); + Err(nix::Error::EINVAL)?; } Ok(()) From c45e5f20635b15f8bfbc6cef07cdbff4c336ff78 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Thu, 18 May 2023 22:53:54 -0700 Subject: [PATCH 2/2] removed unified syscall error Signed-off-by: yihuaf --- crates/libcontainer/src/error.rs | 13 --------- crates/libcontainer/src/namespaces.rs | 37 ++++++++++++------------ crates/libcontainer/src/rootfs/device.rs | 12 ++++---- crates/libcontainer/src/tty.rs | 6 ++-- 4 files changed, 25 insertions(+), 43 deletions(-) diff --git a/crates/libcontainer/src/error.rs b/crates/libcontainer/src/error.rs index de7050170..d51c50200 100644 --- a/crates/libcontainer/src/error.rs +++ b/crates/libcontainer/src/error.rs @@ -1,16 +1,3 @@ -/// UnifiedSyscallError aims to simplify error handling of syscalls in -/// libcontainer. In many occasions, we mix nix::Error, std::io::Error and our -/// own syscall wrappers, which makes error handling complicated. -#[derive(Debug, thiserror::Error)] -pub enum UnifiedSyscallError { - #[error(transparent)] - Io(#[from] std::io::Error), - #[error(transparent)] - Nix(#[from] nix::Error), - #[error(transparent)] - Syscall(#[from] crate::syscall::SyscallError), -} - #[derive(Debug, thiserror::Error)] pub enum MissingSpecError { #[error("missing process in spec")] diff --git a/crates/libcontainer/src/namespaces.rs b/crates/libcontainer/src/namespaces.rs index ef303a772..642451e66 100644 --- a/crates/libcontainer/src/namespaces.rs +++ b/crates/libcontainer/src/namespaces.rs @@ -7,7 +7,6 @@ //! UTS (hostname and domain information, processes will think they're running on servers with different names), //! Cgroup (Resource limits, execution priority etc.) -use crate::error::UnifiedSyscallError; use crate::syscall::{syscall::create_syscall, Syscall}; use nix::{fcntl, sched::CloneFlags, sys::stat, unistd}; use oci_spec::runtime::{LinuxNamespace, LinuxNamespaceType}; @@ -17,12 +16,12 @@ type Result = std::result::Result; #[derive(Debug, thiserror::Error)] pub enum NamespaceError { - #[error("failed to set namespace")] - ApplyNamespaceSyscallFailed { - namespace: Box, - #[source] - err: UnifiedSyscallError, - }, + #[error(transparent)] + Nix(#[from] nix::Error), + #[error(transparent)] + IO(#[from] std::io::Error), + #[error(transparent)] + Syscall(#[from] crate::syscall::SyscallError), } static ORDERED_NAMESPACES: &[CloneFlags] = &[ @@ -88,28 +87,28 @@ impl Namespaces { match namespace.path() { Some(path) => { let fd = fcntl::open(path, fcntl::OFlag::empty(), stat::Mode::empty()).map_err( - |err| NamespaceError::ApplyNamespaceSyscallFailed { - namespace: Box::new(namespace.to_owned()), - err: err.into(), + |err| { + tracing::error!(?err, ?namespace, "failed to open namespace file"); + err }, )?; self.command .set_ns(fd, get_clone_flag(namespace.typ())) - .map_err(|err| NamespaceError::ApplyNamespaceSyscallFailed { - namespace: Box::new(namespace.to_owned()), - err: err.into(), + .map_err(|err| { + tracing::error!(?err, ?namespace, "failed to set namespace"); + err })?; - unistd::close(fd).map_err(|err| NamespaceError::ApplyNamespaceSyscallFailed { - namespace: Box::new(namespace.to_owned()), - err: err.into(), + unistd::close(fd).map_err(|err| { + tracing::error!(?err, ?namespace, "failed to close namespace file"); + err })?; } None => { self.command .unshare(get_clone_flag(namespace.typ())) - .map_err(|err| NamespaceError::ApplyNamespaceSyscallFailed { - namespace: Box::new(namespace.to_owned()), - err: err.into(), + .map_err(|err| { + tracing::error!(?err, ?namespace, "failed to unshare namespace"); + err })?; } } diff --git a/crates/libcontainer/src/rootfs/device.rs b/crates/libcontainer/src/rootfs/device.rs index cdce4408b..dcd107116 100644 --- a/crates/libcontainer/src/rootfs/device.rs +++ b/crates/libcontainer/src/rootfs/device.rs @@ -14,13 +14,11 @@ use std::path::{Path, PathBuf}; pub enum DeviceError { #[error("{0:?} is not a valid device path")] InvalidDevicePath(std::path::PathBuf), - #[error("failed to bind dev")] - BindDev { - source: crate::error::UnifiedSyscallError, - }, #[error("failed syscall to create device")] Syscall(#[from] crate::syscall::SyscallError), #[error(transparent)] + Nix(#[from] nix::Error), + #[error(transparent)] Other(Box), #[error("{0}")] Custom(String), @@ -91,9 +89,9 @@ impl Device { ) .map_err(|err| { tracing::error!("failed to open bind dev {:?}: {}", full_container_path, err); - DeviceError::BindDev { source: err.into() } + err })?; - close(fd).map_err(|err| DeviceError::BindDev { source: err.into() })?; + close(fd)?; self.syscall .mount( Some(dev.path()), @@ -108,7 +106,7 @@ impl Device { full_container_path, err ); - DeviceError::BindDev { source: err.into() } + err })?; Ok(()) diff --git a/crates/libcontainer/src/tty.rs b/crates/libcontainer/src/tty.rs index 8588077b2..e43ecae58 100644 --- a/crates/libcontainer/src/tty.rs +++ b/crates/libcontainer/src/tty.rs @@ -10,8 +10,6 @@ use std::os::unix::io::AsRawFd; use std::os::unix::prelude::RawFd; use std::path::{Path, PathBuf}; -use crate::error::UnifiedSyscallError; - #[derive(Debug)] pub enum StdIO { Stdin = 0, @@ -60,7 +58,7 @@ pub enum TTYError { source: nix::Error, }, #[error("failed to create console socket fd")] - CreateConsoleSocketFd { source: UnifiedSyscallError }, + CreateConsoleSocketFd { source: nix::Error }, #[error("could not create pseudo terminal")] CreatePseudoTerminal { source: nix::Error }, #[error("failed to send pty master")] @@ -90,7 +88,7 @@ pub fn setup_console_socket( socket::SockFlag::empty(), None, ) - .map_err(|err| TTYError::CreateConsoleSocketFd { source: err.into() })?; + .map_err(|err| TTYError::CreateConsoleSocketFd { source: err })?; csocketfd = match socket::connect( csocketfd, &socket::UnixAddr::new(socket_name).map_err(|err| TTYError::InvalidSocketName {