Skip to content

Commit

Permalink
syscalls: switch to rustix for most of our syscalls
Browse files Browse the repository at this point in the history
Using libc leads to several issues when dealing with multiarch that make
things quite frustrating, and rustix does provide nicer APIs (for the
most part -- some are a little wonky). One major annoyance is that
building for musl leads to annoying build failures because musl uses
differently-sized or differently-signed types, which we don't want to
care about because we just want to use the actual kernel APIs. Also,
musl is missing wrappers for things like statx(2) which we need to use.

We still need syscall wrappers to provide nice error information, but we
can remove most of the internal-only bitflags and unsafe blocks. The
only syscall wrapper we don't switch to rustix is openat2 because
rustix's API is not designed to be extensible and so we can stick with
libc for now.

In the future we might want to consider migrating away from libc
entirely (to linux_raw_sys) to reduce the code bloat of having two
different syscall wrappers.

Signed-off-by: Aleksa Sarai <[email protected]>
  • Loading branch information
cyphar committed Oct 17, 2024
1 parent 3fc2b5d commit 43e1cdb
Show file tree
Hide file tree
Showing 13 changed files with 451 additions and 669 deletions.
4 changes: 1 addition & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ once_cell = "^1"
# MSRV(1.65): Update to >=0.4.1 which uses let_else. 0.4.0 was broken.
open-enum = { version = "=0.3.0", optional = true }
rand = { version = "^0.8", optional = true }
rustix = { version = "^0.38", features = ["fs"] }
rustix = { version = "^0.38", features = ["fs", "process", "thread", "mount"] }
thiserror = "^1"

[dev-dependencies]
Expand All @@ -65,8 +65,6 @@ errno = "^0.3"
tempfile = "^3"
paste = "^1"
pretty_assertions = "^1"
# Enable the "process" feature for getcwd() in our tests.
rustix = { version = "^0.38", features = ["process"] }

[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = [
Expand Down
12 changes: 12 additions & 0 deletions src/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ bitflags! {
}
}

impl Into<rustix::fs::OFlags> for OpenFlags {
fn into(self) -> rustix::fs::OFlags {
rustix::fs::OFlags::from_bits_retain(self.bits() as u32)
}
}

impl OpenFlags {
/// Grab the access mode bits from the flags.
///
Expand Down Expand Up @@ -192,6 +198,12 @@ bitflags! {
}
}

impl Into<rustix::fs::RenameFlags> for RenameFlags {
fn into(self) -> rustix::fs::RenameFlags {
rustix::fs::RenameFlags::from_bits_retain(self.bits())
}
}

impl RenameFlags {
/// Is this set of RenameFlags supported by the running kernel?
pub fn is_supported(self) -> bool {
Expand Down
34 changes: 20 additions & 14 deletions src/procfs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,25 @@ use crate::{
error::{Error, ErrorExt, ErrorImpl, ErrorKind},
flags::{OpenFlags, ResolverFlags},
resolvers::procfs::ProcfsResolver,
syscalls::{self, FsmountFlags, FsopenFlags, OpenTreeFlags},
utils,
syscalls,
utils::{self, FdExt},
};

use std::{
fs::File,
io::Error as IOError,
os::unix::io::{AsFd, BorrowedFd, OwnedFd},
os::unix::{
fs::MetadataExt,
io::{AsFd, BorrowedFd, OwnedFd},
},
path::{Path, PathBuf},
};

use once_cell::sync::Lazy;
use rustix::fs::{self as rustix_fs, Access, AtFlags};
use rustix::{
fs::{self as rustix_fs, Access, AtFlags},
mount::{FsMountFlags, FsOpenFlags, MountAttrFlags, OpenTreeFlags},
};

/// A `procfs` handle to which is used globally by libpathrs.
// MSRV(1.80): Use LazyLock.
Expand Down Expand Up @@ -181,7 +187,7 @@ impl ProcfsHandle {
/// against racing attackers changing the mount table and is guaranteed to
/// have no overmounts because it is a brand-new procfs.
pub(crate) fn new_fsopen(subset: bool) -> Result<Self, Error> {
let sfd = syscalls::fsopen("proc", FsopenFlags::FSOPEN_CLOEXEC).map_err(|err| {
let sfd = syscalls::fsopen("proc", FsOpenFlags::FSOPEN_CLOEXEC).map_err(|err| {
ErrorImpl::RawOsError {
operation: "create procfs suberblock".into(),
source: err,
Expand All @@ -202,8 +208,10 @@ impl ProcfsHandle {

syscalls::fsmount(
&sfd,
FsmountFlags::FSMOUNT_CLOEXEC,
libc::MS_NODEV | libc::MS_NOEXEC | libc::MS_NOSUID,
FsMountFlags::FSMOUNT_CLOEXEC,
MountAttrFlags::MOUNT_ATTR_NODEV
| MountAttrFlags::MOUNT_ATTR_NOEXEC
| MountAttrFlags::MOUNT_ATTR_NOSUID,
)
.map_err(|err| {
ErrorImpl::RawOsError {
Expand Down Expand Up @@ -243,7 +251,7 @@ impl ProcfsHandle {
syscalls::openat(
syscalls::AT_FDCWD,
"/proc",
libc::O_PATH | libc::O_DIRECTORY,
OpenFlags::O_PATH | OpenFlags::O_DIRECTORY,
0,
)
.map_err(|err| {
Expand Down Expand Up @@ -386,7 +394,7 @@ impl ProcfsHandle {
// for the ProcfsHandle::{new_fsopen,new_open_tree} cases.
verify_same_mnt(parent_mnt_id, &parent, trailing)?;

syscalls::openat_follow(parent, trailing, oflags.bits(), 0)
syscalls::openat_follow(parent, trailing, oflags, 0)
.map(File::from)
.map_err(|err| {
ErrorImpl::RawOsError {
Expand Down Expand Up @@ -511,9 +519,7 @@ impl ProcfsHandle {
// And make sure it's the root of procfs. The root directory is
// guaranteed to have an inode number of PROC_ROOT_INO. If this check
// ever stops working, it's a kernel regression.
let ino = syscalls::fstatat(&inner, "")
.expect("fstat(/proc) should work")
.st_ino;
let ino = inner.metadata().expect("fstat(/proc) should work").ino();
if ino != Self::PROC_ROOT_INO {
Err(ErrorImpl::SafetyViolation {
description: format!(
Expand Down Expand Up @@ -553,14 +559,14 @@ pub(crate) fn verify_is_procfs<Fd: AsFd>(fd: Fd) -> Result<(), Error> {
source: err,
})?
.f_type;
if fs_type != libc::PROC_SUPER_MAGIC {
if fs_type != rustix_fs::PROC_SUPER_MAGIC {
Err(ErrorImpl::OsError {
operation: "verify lookup is still on a procfs mount".into(),
source: IOError::from_raw_os_error(libc::EXDEV),
})
.wrap(format!(
"fstype mismatch in restricted procfs resolver (f_type is 0x{fs_type:X}, not 0x{:X})",
libc::PROC_SUPER_MAGIC,
rustix_fs::PROC_SUPER_MAGIC,
))?
}
Ok(())
Expand Down
24 changes: 14 additions & 10 deletions src/resolvers/opath/impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

use crate::{
error::{Error, ErrorExt, ErrorImpl},
flags::ResolverFlags,
flags::{OpenFlags, ResolverFlags},
procfs::GLOBAL_PROCFS_HANDLE,
resolvers::{opath::SymlinkStack, PartialLookup, MAX_SYMLINK_TRAVERSALS},
syscalls,
Expand Down Expand Up @@ -262,15 +262,19 @@ fn do_resolve<Fd: AsFd, P: AsRef<Path>>(

// Get our next element.
// MSRV(1.69): Remove &*.
match syscalls::openat(&*current, &part, libc::O_PATH | libc::O_NOFOLLOW, 0).map_err(
|err| {
ErrorImpl::RawOsError {
operation: "open next component of resolution".into(),
source: err,
}
.into()
},
) {
match syscalls::openat(
&*current,
&part,
OpenFlags::O_PATH | OpenFlags::O_NOFOLLOW,
0,
)
.map_err(|err| {
ErrorImpl::RawOsError {
operation: "open next component of resolution".into(),
source: err,
}
.into()
}) {
Err(err) => {
return Ok(PartialLookup::Partial {
handle: current,
Expand Down
18 changes: 11 additions & 7 deletions src/resolvers/procfs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,16 @@ fn opath_resolve<F: AsFd, P: AsRef<Path>>(
}

// Get our next element.
let next = syscalls::openat(&current, &part, libc::O_PATH | libc::O_NOFOLLOW, 0).map_err(
|err| ErrorImpl::RawOsError {
operation: "open next component of resolution".into(),
source: err,
},
)?;
let next = syscalls::openat(
&current,
&part,
OpenFlags::O_PATH | OpenFlags::O_NOFOLLOW,
0,
)
.map_err(|err| ErrorImpl::RawOsError {
operation: "open next component of resolution".into(),
source: err,
})?;

// Check that the next component is on the same mountpoint.
// NOTE: If the root is the host /proc mount, this is only safe if there
Expand Down Expand Up @@ -256,7 +260,7 @@ fn opath_resolve<F: AsFd, P: AsRef<Path>>(
// continue walking).
&& oflags.intersection(OpenFlags::O_PATH | OpenFlags::O_NOFOLLOW | OpenFlags::O_DIRECTORY) != OpenFlags::O_PATH
{
match syscalls::openat(&current, &part, oflags.bits() | libc::O_NOFOLLOW, 0) {
match syscalls::openat(&current, &part, oflags | OpenFlags::O_NOFOLLOW, 0) {
Ok(final_reopen) => {
// Re-verify the next component is on the same mount.
procfs::verify_same_mnt(root_mnt_id, &final_reopen, "")
Expand Down
18 changes: 9 additions & 9 deletions src/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use std::{
path::{Path, PathBuf},
};

use libc::dev_t;
use rustix::fs::{self as rustix_fs, AtFlags};

/// An inode type to be created with [`Root::create`].
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -85,12 +85,12 @@ pub enum InodeType {
/// Character device, as in [`mknod(2)`] with `S_IFCHR`.
///
/// [`mknod(2)`]: http://man7.org/linux/man-pages/man2/mknod.2.html
CharacterDevice(Permissions, dev_t),
CharacterDevice(Permissions, rustix_fs::Dev),

/// Block device, as in [`mknod(2)`] with `S_IFBLK`.
///
/// [`mknod(2)`]: http://man7.org/linux/man-pages/man2/mknod.2.html
BlockDevice(Permissions, dev_t),
BlockDevice(Permissions, rustix_fs::Dev),
// XXX: Does this really make sense?
//// "Detached" unix socket, as in [`mknod(2)`] with `S_IFSOCK`.
////
Expand Down Expand Up @@ -158,7 +158,7 @@ impl Root {
let file = syscalls::openat(
syscalls::AT_FDCWD,
path,
libc::O_PATH | libc::O_DIRECTORY,
OpenFlags::O_PATH | OpenFlags::O_DIRECTORY,
0,
)
.map_err(|err| ErrorImpl::RawOsError {
Expand Down Expand Up @@ -757,7 +757,7 @@ impl RootRef<'_> {
name: "target".into(),
description: "hardlink target has trailing slash".into(),
})?;
syscalls::linkat(olddir, oldname, dir, name, 0)
syscalls::linkat(olddir, oldname, dir, name, AtFlags::empty())
}
InodeType::Fifo(perm) => {
let mode = perm.mode() & !libc::S_IFMT;
Expand Down Expand Up @@ -827,7 +827,7 @@ impl RootRef<'_> {
// O_NOFOLLOW. We might want to expose that here, though because it
// can't be done with the emulated backend that might be a bad idea.
flags.insert(OpenFlags::O_CREAT);
let fd = syscalls::openat(dir, name, flags.bits(), perm.mode()).map_err(|err| {
let fd = syscalls::openat(dir, name, flags, perm.mode()).map_err(|err| {
ErrorImpl::RawOsError {
operation: "pathrs create_file".into(),
source: err,
Expand Down Expand Up @@ -1003,8 +1003,8 @@ impl RootRef<'_> {
})?;

let flags = match inode_type {
RemoveInodeType::Regular => 0,
RemoveInodeType::Directory => libc::AT_REMOVEDIR,
RemoveInodeType::Regular => AtFlags::empty(),
RemoveInodeType::Directory => AtFlags::REMOVEDIR,
};
syscalls::unlinkat(dir, name, flags).map_err(|err| {
ErrorImpl::RawOsError {
Expand Down Expand Up @@ -1118,7 +1118,7 @@ impl RootRef<'_> {
description: "rename destination path has trailing slash".into(),
})?;

syscalls::renameat2(src_dir, src_name, dst_dir, dst_name, rflags.bits()).map_err(|err| {
syscalls::renameat2(src_dir, src_name, dst_dir, dst_name, rflags).map_err(|err| {
ErrorImpl::RawOsError {
operation: "pathrs rename".into(),
source: err,
Expand Down
Loading

0 comments on commit 43e1cdb

Please sign in to comment.