From d460f9c4cc61771ff47fd52107a8f0f1c8fef055 Mon Sep 17 00:00:00 2001 From: Abel Feng Date: Wed, 30 Oct 2024 11:29:30 +0800 Subject: [PATCH] libcontainer: use OwnedFd as console_socket in ContainerBuilder Signed-off-by: Abel Feng --- .../src/container/builder_impl.rs | 6 +- .../src/container/tenant_builder.rs | 5 +- .../src/process/container_init_process.rs | 2 +- crates/libcontainer/src/tty.rs | 69 +++++++++---------- 4 files changed, 39 insertions(+), 43 deletions(-) diff --git a/crates/libcontainer/src/container/builder_impl.rs b/crates/libcontainer/src/container/builder_impl.rs index 0a9e43f52..6c0fc4f56 100644 --- a/crates/libcontainer/src/container/builder_impl.rs +++ b/crates/libcontainer/src/container/builder_impl.rs @@ -1,6 +1,6 @@ use std::fs; use std::io::Write; -use std::os::unix::prelude::RawFd; +use std::os::fd::{AsRawFd, OwnedFd}; use std::path::PathBuf; use std::rc::Rc; @@ -36,7 +36,7 @@ pub(super) struct ContainerBuilderImpl { /// container process to the higher level runtime pub pid_file: Option, /// Socket to communicate the file descriptor of the ptty - pub console_socket: Option, + pub console_socket: Option, /// Options for new user namespace pub user_ns_config: Option, /// Path to the Unix Domain Socket to communicate container start @@ -148,7 +148,7 @@ impl ContainerBuilderImpl { syscall: self.syscall, spec: Rc::clone(&self.spec), rootfs: self.rootfs.to_owned(), - console_socket: self.console_socket, + console_socket: self.console_socket.as_ref().map(|c| c.as_raw_fd()), notify_listener, preserve_fds: self.preserve_fds, container: self.container.to_owned(), diff --git a/crates/libcontainer/src/container/tenant_builder.rs b/crates/libcontainer/src/container/tenant_builder.rs index e54a22cca..029f73956 100644 --- a/crates/libcontainer/src/container/tenant_builder.rs +++ b/crates/libcontainer/src/container/tenant_builder.rs @@ -3,8 +3,7 @@ use std::convert::TryFrom; use std::ffi::{OsStr, OsString}; use std::fs; use std::io::BufReader; -use std::os::fd::AsRawFd; -use std::os::unix::prelude::RawFd; +use std::os::fd::{AsRawFd, OwnedFd}; use std::path::{Path, PathBuf}; use std::rc::Rc; use std::str::FromStr; @@ -500,7 +499,7 @@ impl TenantContainerBuilder { Ok(socket_path) } - fn setup_tty_socket(&self, container_dir: &Path) -> Result, LibcontainerError> { + fn setup_tty_socket(&self, container_dir: &Path) -> Result, LibcontainerError> { let tty_name = Self::generate_name(container_dir, TENANT_TTY); let csocketfd = if let Some(console_socket) = &self.base.console_socket { Some(tty::setup_console_socket( diff --git a/crates/libcontainer/src/process/container_init_process.rs b/crates/libcontainer/src/process/container_init_process.rs index 8d30b74b4..3adf4194a 100644 --- a/crates/libcontainer/src/process/container_init_process.rs +++ b/crates/libcontainer/src/process/container_init_process.rs @@ -370,7 +370,7 @@ pub fn container_init_process( // set up tty if specified if let Some(csocketfd) = args.console_socket { - tty::setup_console(&csocketfd).map_err(|err| { + tty::setup_console(csocketfd).map_err(|err| { tracing::error!(?err, "failed to set up tty"); InitProcessError::Tty(err) })?; diff --git a/crates/libcontainer/src/tty.rs b/crates/libcontainer/src/tty.rs index b9a753127..112471f41 100644 --- a/crates/libcontainer/src/tty.rs +++ b/crates/libcontainer/src/tty.rs @@ -2,12 +2,12 @@ use std::env; use std::io::IoSlice; +use std::os::fd::OwnedFd; use std::os::unix::fs::symlink; use std::os::unix::io::AsRawFd; use std::os::unix::prelude::RawFd; use std::path::{Path, PathBuf}; -use nix::errno::Errno; use nix::sys::socket::{self, UnixAddr}; use nix::unistd::{close, dup2}; @@ -75,12 +75,21 @@ pub fn setup_console_socket( container_dir: &Path, console_socket_path: &Path, socket_name: &str, -) -> Result { +) -> Result { + struct CurrentDirGuard { + path: PathBuf, + } + impl Drop for CurrentDirGuard { + fn drop(&mut self) { + let _ = env::set_current_dir(&self.path); + } + } // Move into the container directory to avoid sun family conflicts with long socket path names. // ref: https://github.com/containers/youki/issues/2910 let prev_dir = env::current_dir().unwrap(); let _ = env::set_current_dir(container_dir); + let _guard = CurrentDirGuard { path: prev_dir }; let linked = PathBuf::from(socket_name); @@ -89,36 +98,29 @@ pub fn setup_console_socket( linked: linked.to_path_buf().into(), console_socket_path: console_socket_path.to_path_buf().into(), })?; - // Using ManuallyDrop to keep the socket open. - let csocketfd = std::mem::ManuallyDrop::new( - socket::socket( - socket::AddressFamily::Unix, - socket::SockType::Stream, - socket::SockFlag::empty(), - None, - ) - .map_err(|err| TTYError::CreateConsoleSocketFd { source: err })?, - ); - let csocketfd = match socket::connect( + let csocketfd = socket::socket( + socket::AddressFamily::Unix, + socket::SockType::Stream, + socket::SockFlag::empty(), + None, + ) + .map_err(|err| TTYError::CreateConsoleSocketFd { source: err })?; + socket::connect( csocketfd.as_raw_fd(), &socket::UnixAddr::new(linked.as_path()).map_err(|err| TTYError::InvalidSocketName { source: err, socket_name: socket_name.to_string(), })?, - ) { - Err(Errno::ENOENT) => -1, - Err(errno) => Err(TTYError::CreateConsoleSocket { - source: errno, - socket_name: socket_name.to_string(), - })?, - Ok(()) => csocketfd.as_raw_fd(), - }; + ) + .map_err(|e| TTYError::CreateConsoleSocket { + source: e, + socket_name: socket_name.to_string(), + })?; - let _ = env::set_current_dir(prev_dir); Ok(csocketfd) } -pub fn setup_console(console_fd: &RawFd) -> Result<()> { +pub fn setup_console(console_fd: RawFd) -> Result<()> { // You can also access pty master, but it is better to use the API. // ref. https://github.com/containerd/containerd/blob/261c107ffc4ff681bc73988f64e3f60c32233b37/vendor/github.com/containerd/go-runc/console.go#L139-L154 let openpty_result = nix::pty::openpty(None, None) @@ -133,21 +135,15 @@ pub fn setup_console(console_fd: &RawFd) -> Result<()> { let fds = [master.as_raw_fd()]; let cmsg = socket::ControlMessage::ScmRights(&fds); - socket::sendmsg::( - console_fd.as_raw_fd(), - &iov, - &[cmsg], - socket::MsgFlags::empty(), - None, - ) - .map_err(|err| TTYError::SendPtyMaster { source: err })?; + socket::sendmsg::(console_fd, &iov, &[cmsg], socket::MsgFlags::empty(), None) + .map_err(|err| TTYError::SendPtyMaster { source: err })?; if unsafe { libc::ioctl(slave.as_raw_fd(), libc::TIOCSCTTY) } < 0 { tracing::warn!("could not TIOCSCTTY"); }; let slave = slave.as_raw_fd(); connect_stdio(&slave, &slave, &slave)?; - close(console_fd.as_raw_fd()).map_err(|err| TTYError::CloseConsoleSocket { source: err })?; + close(console_fd).map_err(|err| TTYError::CloseConsoleSocket { source: err })?; Ok(()) } @@ -174,6 +170,7 @@ fn connect_stdio(stdin: &RawFd, stdout: &RawFd, stderr: &RawFd) -> Result<()> { #[cfg(test)] mod tests { use std::fs::File; + use std::os::fd::IntoRawFd; use std::os::unix::net::UnixListener; use anyhow::{Ok, Result}; @@ -200,8 +197,8 @@ mod tests { fn test_setup_console_socket_empty() -> Result<()> { let testdir = tempfile::tempdir()?; let socket_path = Path::join(testdir.path(), "test-socket"); - let fd = setup_console_socket(testdir.path(), &socket_path, CONSOLE_SOCKET)?; - assert_eq!(fd.as_raw_fd(), -1); + let fd = setup_console_socket(testdir.path(), &socket_path, CONSOLE_SOCKET); + assert!(fd.is_err()); Ok(()) } @@ -232,8 +229,8 @@ mod tests { let lis = UnixListener::bind(&socket_path); assert!(lis.is_ok()); - let fd = setup_console_socket(testdir.path(), &socket_path, CONSOLE_SOCKET); - let status = setup_console(&fd.unwrap()); + let fd = setup_console_socket(testdir.path(), &socket_path, CONSOLE_SOCKET)?; + let status = setup_console(fd.into_raw_fd()); // restore the original std* before doing final assert dup2(old_stdin, StdIO::Stdin.into())?;