syscalls: switch to rustix for most of our syscalls

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]>
openSUSE · Oct 17, 2024 · 5499641 · 5499641
1 parent 3fc2b5d
commit 5499641
Show file tree

Hide file tree

Showing 13 changed files with 451 additions and 668 deletions.
diff --git a/Cargo.toml b/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 = [

diff --git a/src/flags.rs b/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 {

diff --git a/src/procfs.rs b/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(())

diff --git a/src/resolvers/opath/impl.rs b/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,

diff --git a/src/resolvers/procfs.rs b/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, "")

diff --git a/src/root.rs b/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,