merge #92 into openSUSE/libpathrs:main

Aleksa Sarai (1):
  syscalls: switch to rustix for most of our syscalls

LGTMs: cyphar

CHANGELOG.md

@@ -15,6 +15,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - python bindings: add `Root.creat_raw` to create a new file and wrap it in a
  raw `WrappedFd` (os opposed to `Root.creat` which returns an `os.fdopen`).
 
+### Changed ###
+- syscalls: switch to rustix for most of our syscall wrappers to simplify how
+ much code we have for wrapper raw syscalls. This also lets us build on
+ musl-based targets because musl doesn't support some of the syscalls we need.
+
+ There are some outstanding issues with rustix that make this switch a little
+ uglier than necessary ([rustix#1186][], [rustix#1187][]), but this is a net
+ improvement overall.
+
+[rustix#1186]: https://github.com/bytecodealliance/rustix/issues/1186
+[rustix#1187]: https://github.com/bytecodealliance/rustix/issues/1187
+
 ## [0.1.3] - 2024-10-10 ##
 
 > 自動化って物は試しとすればいい物だ

Cargo.toml

@@ -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]
@@ -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 = [

src/flags.rs

@@ -123,6 +123,12 @@ bitflags! {
  }
 }
 
+impl From<OpenFlags> for rustix::fs::OFlags {
+ fn from(flags: OpenFlags) -> Self {
+ Self::from_bits_retain(flags.bits() as u32)
+ }
+}
+
 impl OpenFlags {
  /// Grab the access mode bits from the flags.
  ///
@@ -192,6 +198,12 @@ bitflags! {
  }
 }
 
+impl From<RenameFlags> for rustix::fs::RenameFlags {
+ fn from(flags: RenameFlags) -> Self {
+ Self::from_bits_retain(flags.bits())
+ }
+}
+
 impl RenameFlags {
  /// Is this set of RenameFlags supported by the running kernel?
  pub fn is_supported(self) -> bool {

src/procfs.rs

@@ -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.
@@ -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,
@@ -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 {
@@ -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| {
@@ -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 {
@@ -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!(
@@ -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(())

src/resolvers/opath/impl.rs

@@ -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,
@@ -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,

src/resolvers/procfs.rs

@@ -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
@@ -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, "")

src/root.rs

@@ -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)]
@@ -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`.
  ////
@@ -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 {
@@ -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;
@@ -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,
@@ -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 {
@@ -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,