Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libcontainer: use OwnedFd as console_socket in ContainerBuilder #2966

Merged
merged 2 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/libcontainer/src/container/builder_impl.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -36,7 +36,7 @@ pub(super) struct ContainerBuilderImpl {
/// container process to the higher level runtime
pub pid_file: Option<PathBuf>,
/// Socket to communicate the file descriptor of the ptty
pub console_socket: Option<RawFd>,
pub console_socket: Option<OwnedFd>,
/// Options for new user namespace
pub user_ns_config: Option<UserNamespaceConfig>,
/// Path to the Unix Domain Socket to communicate container start
Expand Down Expand Up @@ -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(),
Expand Down
5 changes: 2 additions & 3 deletions crates/libcontainer/src/container/tenant_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -500,7 +499,7 @@ impl TenantContainerBuilder {
Ok(socket_path)
}

fn setup_tty_socket(&self, container_dir: &Path) -> Result<Option<RawFd>, LibcontainerError> {
fn setup_tty_socket(&self, container_dir: &Path) -> Result<Option<OwnedFd>, 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(
Expand Down
2 changes: 1 addition & 1 deletion crates/libcontainer/src/process/container_init_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})?;
Expand Down
69 changes: 33 additions & 36 deletions crates/libcontainer/src/tty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -75,12 +75,21 @@ pub fn setup_console_socket(
container_dir: &Path,
console_socket_path: &Path,
socket_name: &str,
) -> Result<RawFd> {
) -> Result<OwnedFd> {
struct CurrentDirGuard {
path: PathBuf,
}
impl Drop for CurrentDirGuard {
fn drop(&mut self) {
let _ = env::set_current_dir(&self.path);
}
}
YJDoc2 marked this conversation as resolved.
Show resolved Hide resolved
// 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);

Expand All @@ -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,
YJDoc2 marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand All @@ -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::<UnixAddr>(
console_fd.as_raw_fd(),
&iov,
&[cmsg],
socket::MsgFlags::empty(),
None,
)
.map_err(|err| TTYError::SendPtyMaster { source: err })?;
socket::sendmsg::<UnixAddr>(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(())
}
Expand All @@ -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};
Expand All @@ -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(())
}

Expand Down Expand Up @@ -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())?;
Expand Down
Loading