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.

Sign up for GitHub

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

Jump to bottom

update nix to 0.27.1 #2369

Merged
merged 5 commits into from
Oct 9, 2023
Merged

update nix to 0.27.1 #2369

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
59937cf
update nix to 0.27.1
anti-entropy123 Oct 7, 2023
705b17a
add comments
anti-entropy123 Oct 9, 2023
d07a538
resolve conflicts
anti-entropy123 Oct 9, 2023
326aea1
fix lint error
anti-entropy123 Oct 9, 2023
951e5c6
remove `let socket = socket;`
anti-entropy123 Oct 9, 2023
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
26 changes: 19 additions & 7 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions crates/libcgroups/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,21 @@ keywords = ["youki", "container", "cgroups"]
default = ["v1", "v2", "systemd"]
v1 = []
v2 = []
systemd = ["v2"]
cgroupsv2_devices = ["rbpf", "libbpf-sys", "errno", "libc"]
systemd = ["v2", "nix/socket", "nix/uio"]
cgroupsv2_devices = ["rbpf", "libbpf-sys", "errno", "libc", "nix/dir"]

[dependencies]
nix = "0.26.2"
nix = { version = "0.27.1", features = ["signal", "user", "fs"] }
procfs = "0.15.1"
oci-spec = { version = "~0.6.2", features = ["runtime"] }
fixedbitset = "0.4.2"
serde = { version = "1.0", features = ["derive"] }
rbpf = {version = "0.2.0", optional = true }
rbpf = { version = "0.2.0", optional = true }
libbpf-sys = { version = "1.2.1", optional = true }
errno = { version = "0.3.4", optional = true }
libc = { version = "0.2.148", optional = true }
thiserror = "1.0.49"
tracing = { version = "0.1.37", features = ["attributes"]}
tracing = { version = "0.1.37", features = ["attributes"] }

[dev-dependencies]
anyhow = "1.0"
Expand Down
14 changes: 8 additions & 6 deletions crates/libcgroups/src/systemd/dbus_native/dbus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use super::utils::{DbusError, Result, SystemdClientError};
use nix::sys::socket;
use std::collections::HashMap;
use std::io::{IoSlice, IoSliceMut};
use std::mem::ManuallyDrop;
use std::os::fd::AsRawFd;
use std::path::PathBuf;
use std::sync::atomic::{AtomicU32, Ordering};

Expand Down Expand Up @@ -121,17 +123,17 @@ impl DbusConnection {
/// Open a new dbus connection to given address
/// authenticating as user with given uid
pub fn new(addr: &str, uid: u32, system: bool) -> Result<Self> {
let socket = socket::socket(
let socket = ManuallyDrop::new(socket::socket(
socket::AddressFamily::Unix,
socket::SockType::Stream,
socket::SockFlag::empty(),
None,
)?;
)?);

let addr = socket::UnixAddr::new(addr)?;
socket::connect(socket, &addr)?;
socket::connect(socket.as_raw_fd(), &addr)?;
let mut dbus = Self {
socket,
socket: socket.as_raw_fd(),
msg_ctr: AtomicU32::new(0),
id: None,
system,
Expand Down Expand Up @@ -237,11 +239,11 @@ impl DbusConnection {
let mut ret = Vec::with_capacity(512);
loop {
let mut reply: [u8; REPLY_BUF_SIZE] = [0_u8; REPLY_BUF_SIZE];
let reply_buffer = IoSliceMut::new(&mut reply[0..]);
let mut reply_buffer = [IoSliceMut::new(&mut reply[0..])];

let reply_rcvd = socket::recvmsg::<()>(
self.socket,
&mut [reply_buffer],
&mut reply_buffer,
None,
socket::MsgFlags::empty(),
)?;
Expand Down
20 changes: 16 additions & 4 deletions crates/libcontainer/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,35 @@ cgroupsv2_devices = ["libcgroups/cgroupsv2_devices"]
[dependencies]
bitflags = "2.4.0"
caps = "0.5.5"
chrono = { version = "0.4", default-features = false, features = ["clock", "serde"] }
chrono = { version = "0.4", default-features = false, features = [
"clock",
"serde",
] }
fastrand = "^2.0.1"
futures = { version = "0.3", features = ["thread-pool"] }
libc = "0.2.148"
nix = "0.26.2"
nix = { version = "0.27.1", features = [
"socket",
"sched",
"mount",
"mman",
"resource",
"dir",
"term",
"hostname",
] }
oci-spec = { version = "~0.6.2", features = ["runtime"] }
once_cell = "1.18.0"
procfs = "0.15.1"
prctl = "1.0.0"
libcgroups = { version = "0.2.0", path = "../libcgroups", default-features = false }
libseccomp = { version = "0.3.0", optional=true }
libseccomp = { version = "0.3.0", optional = true }
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
rust-criu = "0.4.0"
regex = "1.9.6"
thiserror = "1.0.49"
tracing = { version = "0.1.37", features = ["attributes"]}
tracing = { version = "0.1.37", features = ["attributes"] }
safe-path = "0.1.0"

[dev-dependencies]
Expand Down
10 changes: 7 additions & 3 deletions crates/libcontainer/src/channel.rs
View file
Open in desktop
anti-entropy123 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize};
use std::{
io::{IoSlice, IoSliceMut},
marker::PhantomData,
os::unix::prelude::RawFd,
os::{fd::AsRawFd, unix::prelude::RawFd},
};

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -210,10 +210,14 @@ where

// Use socketpair as the underlying pipe.
fn unix_channel() -> Result<(RawFd, RawFd), ChannelError> {
Ok(socket::socketpair(
let (f1, f2) = socket::socketpair(
socket::AddressFamily::Unix,
socket::SockType::SeqPacket,
None,
socket::SockFlag::SOCK_CLOEXEC,
)?)
)?;
let f1 = std::mem::ManuallyDrop::new(f1);
let f2 = std::mem::ManuallyDrop::new(f2);
Comment on lines +221 to +222
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits: Could you leave a comment on why we need to ManuallyDrop?


Ok((f1.as_raw_fd(), f2.as_raw_fd()))
}
8 changes: 5 additions & 3 deletions crates/libcontainer/src/process/fork.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{ffi::c_int, num::NonZeroUsize};
use std::{ffi::c_int, fs::File, num::NonZeroUsize};

use libc::SIGCHLD;
use nix::{
Expand Down Expand Up @@ -164,12 +164,14 @@ fn clone(cb: CloneCb, flags: u64, exit_signal: Option<u64>) -> Result<Pid, Clone
// do not use MAP_GROWSDOWN since it is not well supported.
// Ref: https://man7.org/linux/man-pages/man2/mmap.2.html
let child_stack = unsafe {
mman::mmap(
// Since nix = "0.27.1", `mmap()` requires a generic type `F: AsFd`.
// `::<File>` doesn't have any meaning because we won't use it.
mman::mmap::<File>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask you to write a comment that File doesn't have any meaning because we won't use it.

Copy link
Contributor Author

@anti-entropy123 anti-entropy123 Sep 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's related to the change in the function signature of nix::sys::mman::mmap(). The current signature of the mmap() function is:

pub unsafe fn mmap<F: AsFd>(
    addr: Option<NonZeroUsize>, 
    length: NonZeroUsize, 
    prot: ProtFlags, 
    flags: MapFlags, 
    f: Option<F>, 
    offset: off_t) -> Result<*mut c_void>

If you remove ::<File>, it will result in a compilation error:

type annotations needed; cannot infer type of the type parameter F declared on the function mmap

Is there any other way to avoid the compilation error?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood it, but it's unclear to the reader of this code. May I ask you to write a comment about it?

None,
NonZeroUsize::new(default_stack_size).ok_or(CloneError::ZeroStackSize)?,
mman::ProtFlags::PROT_READ | mman::ProtFlags::PROT_WRITE,
mman::MapFlags::MAP_PRIVATE | mman::MapFlags::MAP_ANONYMOUS | mman::MapFlags::MAP_STACK,
-1,
None,
0,
)
.map_err(CloneError::StackAllocation)?
Expand Down
24 changes: 14 additions & 10 deletions crates/libcontainer/src/process/seccomp_listener.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use nix::{
unistd,
};
use oci_spec::runtime;
use std::{io::IoSlice, path::Path};
use std::{io::IoSlice, os::fd::AsRawFd, path::Path};

use super::channel;

Expand Down Expand Up @@ -76,7 +76,7 @@ fn sync_seccomp_send_msg(listener_path: &Path, msg: &[u8], fd: i32) -> Result<()
);
SeccompListenerError::UnixOther(err)
})?;
socket::connect(socket, &unix_addr).map_err(|err| {
socket::connect(socket.as_raw_fd(), &unix_addr).map_err(|err| {
tracing::error!(
?err,
?listener_path,
Expand All @@ -91,15 +91,19 @@ fn sync_seccomp_send_msg(listener_path: &Path, msg: &[u8], fd: i32) -> Result<()
let iov = [IoSlice::new(msg)];
let fds = [fd];
let cmsgs = socket::ControlMessage::ScmRights(&fds);
socket::sendmsg::<UnixAddr>(socket, &iov, &[cmsgs], socket::MsgFlags::empty(), None).map_err(
|err| {
tracing::error!(?err, "failed to write container state to seccomp listener");
SeccompListenerError::UnixOther(err)
},
)?;
socket::sendmsg::<UnixAddr>(
socket.as_raw_fd(),
&iov,
&[cmsgs],
socket::MsgFlags::empty(),
None,
)
.map_err(|err| {
tracing::error!(?err, "failed to write container state to seccomp listener");
SeccompListenerError::UnixOther(err)
})?;
// The spec requires the listener socket to be closed immediately after sending.
let _ = unistd::close(socket);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we can remove close()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think OwnedFd will automatically call libc::close() when it goes out of function body.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec requires the listener socket to be closed immediately after sending.

We need to close it as soon as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L106 is the return statement, so sendmsg() will be immediately followed by the drop of the socket.
However, considering that the spec emphasizes this point, I believe it's necessary to explicitly close the socket. I plan to call drop(socket) on L105. Do you think that's okay?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's necessary to explicitly close the socket. I plan to call drop(socket) on L105. Do you think that's okay?

I prefer it because there is a possibility that we might change the code around there.


drop(socket);
Ok(())
}

Expand Down
4 changes: 3 additions & 1 deletion crates/libcontainer/src/syscall/linux.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use nix::{
use oci_spec::runtime::LinuxRlimit;
use std::ffi::{CStr, CString, OsStr};
use std::fs;
use std::os::fd::BorrowedFd;
use std::os::unix::ffi::OsStrExt;
use std::os::unix::fs::symlink;
use std::os::unix::io::RawFd;
Expand Down Expand Up @@ -305,7 +306,8 @@ impl Syscall for LinuxSyscall {

/// Set namespace for process
fn set_ns(&self, rawfd: i32, nstype: CloneFlags) -> Result<()> {
nix::sched::setns(rawfd, nstype)?;
let fd = unsafe { BorrowedFd::borrow_raw(rawfd) };
nix::sched::setns(fd, nstype)?;
Ok(())
}

Expand Down
33 changes: 20 additions & 13 deletions crates/libcontainer/src/tty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,17 @@ pub fn setup_console_socket(
console_socket_path: console_socket_path.to_path_buf().into(),
})?;

let mut csocketfd = socket::socket(
socket::AddressFamily::Unix,
socket::SockType::Stream,
socket::SockFlag::empty(),
None,
)
.map_err(|err| TTYError::CreateConsoleSocketFd { source: err })?;
csocketfd = match socket::connect(
csocketfd,
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(
csocketfd.as_raw_fd(),
&socket::UnixAddr::new(socket_name).map_err(|err| TTYError::InvalidSocketName {
source: err,
socket_name: socket_name.to_string(),
Expand All @@ -101,7 +103,7 @@ pub fn setup_console_socket(
source: errno,
socket_name: socket_name.to_string(),
})?,
Ok(()) => csocketfd,
Ok(()) => csocketfd.as_raw_fd(),
};
Ok(csocketfd)
}
Expand All @@ -113,7 +115,12 @@ pub fn setup_console(console_fd: &RawFd) -> Result<()> {
.map_err(|err| TTYError::CreatePseudoTerminal { source: err })?;
let pty_name: &[u8] = b"/dev/ptmx";
let iov = [IoSlice::new(pty_name)];
let fds = [openpty_result.master];

let [master, slave] = [openpty_result.master, openpty_result.slave];
let master = std::mem::ManuallyDrop::new(master);
let slave = std::mem::ManuallyDrop::new(slave);

let fds = [master.as_raw_fd()];
let cmsg = socket::ControlMessage::ScmRights(&fds);
socket::sendmsg::<UnixAddr>(
console_fd.as_raw_fd(),
Expand All @@ -124,10 +131,10 @@ pub fn setup_console(console_fd: &RawFd) -> Result<()> {
)
.map_err(|err| TTYError::SendPtyMaster { source: err })?;

if unsafe { libc::ioctl(openpty_result.slave, libc::TIOCSCTTY) } < 0 {
if unsafe { libc::ioctl(slave.as_raw_fd(), libc::TIOCSCTTY) } < 0 {
tracing::warn!("could not TIOCSCTTY");
};
let slave = openpty_result.slave;
let slave = slave.as_raw_fd();
connect_stdio(&slave, &slave, &slave)?;
close(console_fd.as_raw_fd()).map_err(|err| TTYError::CloseConsoleSocket { source: err })?;

Expand Down
3 changes: 1 addition & 2 deletions crates/libcontainer/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::collections::HashMap;
use std::fs::{self, DirBuilder, File};
use std::os::linux::fs::MetadataExt;
use std::os::unix::fs::DirBuilderExt;
use std::os::unix::prelude::AsRawFd;
use std::path::{Component, Path, PathBuf};

use nix::sys::stat::Mode;
Expand Down Expand Up @@ -249,7 +248,7 @@ pub fn ensure_procfs(path: &Path) -> Result<(), EnsureProcfsError> {
tracing::error!(?err, ?path, "failed to open procfs file");
err
})?;
let fstat_info = statfs::fstatfs(&procfs_fd.as_raw_fd()).map_err(|err| {
let fstat_info = statfs::fstatfs(&procfs_fd).map_err(|err| {
tracing::error!(?err, ?path, "failed to fstatfs the procfs");
err
})?;
Expand Down
2 changes: 1 addition & 1 deletion crates/youki/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ chrono = { version = "0.4", default-features = false, features = ["clock", "serd
libcgroups = { version = "0.2.0", path = "../libcgroups", default-features = false }
libcontainer = { version = "0.2.0", path = "../libcontainer", default-features = false }
liboci-cli = { version = "0.2.0", path = "../liboci-cli" }
nix = "0.26.2"
nix = "0.27.1"
once_cell = "1.18.0"
pentacle = "1.0.0"
procfs = "0.15.1"
Expand Down
Loading