Restore statx request-mask filtering

src/backend/libc/fs/syscalls.rs

@@ -49,6 +49,15 @@ use crate::fs::SealFlags;
     target_os = "wasi",
 )))]
 use crate::fs::StatFs;
+#[cfg(all(
+    linux_kernel,
+    any(
+        target_pointer_width = "32",
+        target_arch = "mips64",
+        target_arch = "mips64r6"
+    )
+))]
+use crate::fs::StatxFlags;
 #[cfg(not(any(target_os = "espidf", target_os = "vita")))]
 use crate::fs::Timestamps;
 #[cfg(not(any(
@@ -91,7 +100,7 @@ use {crate::fs::Advice, core::num::NonZeroU64};
 use {crate::fs::XattrFlags, core::mem::size_of, core::ptr::null_mut};
 #[cfg(linux_kernel)]
 use {
-    crate::fs::{ResolveFlags, Statx, StatxFlags, CWD},
+    crate::fs::{ResolveFlags, Statx, StatxRequestFlags, CWD},
     core::ptr::null,
 };
 
@@ -2056,30 +2065,8 @@ pub(crate) fn statx(
     dirfd: BorrowedFd<'_>,
     path: &CStr,
     flags: AtFlags,
-    mask: StatxFlags,
+    mask: StatxRequestFlags,
 ) -> io::Result<Statx> {
-    // If a future Linux kernel adds more fields to `struct statx` and users
-    // passing flags unknown to rustix in `StatxFlags`, we could end up
-    // writing outside of the buffer. To prevent this possibility, we mask off
-    // any flags that we don't know about.
-    //
-    // This includes `STATX__RESERVED`, which has a value that we know, but
-    // which could take on arbitrary new meaning in the future. Linux currently
-    // rejects this flag with `EINVAL`, so we do the same.
-    //
-    // This doesn't rely on `STATX_ALL` because [it's deprecated] and already
-    // doesn't represent all the known flags.
-    //
-    // [it's deprecated]: https://patchwork.kernel.org/project/linux-fsdevel/patch/20200505095915.11275-7-mszeredi@redhat.com/
-    #[cfg(any(not(linux_raw_dep), not(target_env = "musl")))]
-    const STATX__RESERVED: u32 = c::STATX__RESERVED as u32;
-    #[cfg(target_env = "musl")]
-    const STATX__RESERVED: u32 = linux_raw_sys::general::STATX__RESERVED;
-    if (mask.bits() & STATX__RESERVED) == STATX__RESERVED {
-        return Err(io::Errno::INVAL);
-    }
-    let mask = mask & StatxFlags::all();
-
     let mut statx_buf = MaybeUninit::<Statx>::uninit();
     unsafe {
         ret(sys::statx(

src/backend/linux_raw/fs/syscalls.rs

@@ -33,7 +33,7 @@ use crate::fs::CWD;
 use crate::fs::{
     inotify, Access, Advice, AtFlags, FallocateFlags, FileType, FlockOperation, Fsid, Gid,
     MemfdFlags, Mode, OFlags, RenameFlags, ResolveFlags, SealFlags, SeekFrom, Stat, StatFs,
-    StatVfs, StatVfsMountFlags, Statx, StatxFlags, Timestamps, Uid, XattrFlags,
+    StatVfs, StatVfsMountFlags, Statx, StatxFlags, StatxRequestFlags, Timestamps, Uid, XattrFlags,
 };
 use crate::io;
 use core::mem::MaybeUninit;
@@ -42,7 +42,7 @@ use core::num::NonZeroU64;
 use linux_raw_sys::general::stat as linux_stat64;
 use linux_raw_sys::general::{
     open_how, AT_EACCESS, AT_FDCWD, AT_REMOVEDIR, AT_SYMLINK_NOFOLLOW, F_ADD_SEALS, F_GETFL,
-    F_GET_SEALS, F_SETFL, SEEK_CUR, SEEK_DATA, SEEK_END, SEEK_HOLE, SEEK_SET, STATX__RESERVED,
+    F_GET_SEALS, F_SETFL, SEEK_CUR, SEEK_DATA, SEEK_END, SEEK_HOLE, SEEK_SET,
 };
 #[cfg(target_pointer_width = "32")]
 use {
@@ -819,34 +819,16 @@ pub(crate) fn statx(
     dirfd: BorrowedFd<'_>,
     path: &CStr,
     flags: AtFlags,
-    mask: StatxFlags,
+    mask: StatxRequestFlags,
 ) -> io::Result<Statx> {
-    // If a future Linux kernel adds more fields to `struct statx` and users
-    // passing flags unknown to rustix in `StatxFlags`, we could end up
-    // writing outside of the buffer. To prevent this possibility, we mask off
-    // any flags that we don't know about.
-    //
-    // This includes `STATX__RESERVED`, which has a value that we know, but
-    // which could take on arbitrary new meaning in the future. Linux currently
-    // rejects this flag with `EINVAL`, so we do the same.
-    //
-    // This doesn't rely on `STATX_ALL` because [it's deprecated] and already
-    // doesn't represent all the known flags.
-    //
-    // [it's deprecated]: https://patchwork.kernel.org/project/linux-fsdevel/patch/20200505095915.11275-7-mszeredi@redhat.com/
-    if (mask.bits() & STATX__RESERVED) == STATX__RESERVED {
-        return Err(io::Errno::INVAL);
-    }
-    let mask = mask & StatxFlags::all();
-
     unsafe {
         let mut statx_buf = MaybeUninit::<Statx>::uninit();
         ret(syscall!(
             __NR_statx,
             dirfd,
             path,
             flags,
-            mask,
+            StatxFlags::from_bits_retain(mask.bits()),
             &mut statx_buf
         ))?;
         Ok(statx_buf.assume_init())

src/fs/statx.rs

@@ -124,6 +124,41 @@ bitflags! {
     }
 }
 
+/// A `statx` request mask containing only flags known to this definition of
+/// [`Statx`].
+#[repr(transparent)]
+#[derive(Copy, Clone)]
+pub(crate) struct StatxRequestFlags(u32);
+
+impl StatxRequestFlags {
+    // Keep this list to the elementary request flags; `STATX_BASIC_STATS` and
+    // `STATX_ALL` are composite sets, and the latter is deprecated.
+    const KNOWN_REQUEST_BITS: u32 = StatxFlags::TYPE.bits()
+        | StatxFlags::MODE.bits()
+        | StatxFlags::NLINK.bits()
+        | StatxFlags::UID.bits()
+        | StatxFlags::GID.bits()
+        | StatxFlags::ATIME.bits()
+        | StatxFlags::MTIME.bits()
+        | StatxFlags::CTIME.bits()
+        | StatxFlags::INO.bits()
+        | StatxFlags::SIZE.bits()
+        | StatxFlags::BLOCKS.bits()
+        | StatxFlags::BTIME.bits()
+        | StatxFlags::MNT_ID.bits()
+        | StatxFlags::DIOALIGN.bits();
+
+    #[inline]
+    const fn sanitize(mask: StatxFlags) -> Self {
+        Self(mask.bits() & Self::KNOWN_REQUEST_BITS)
+    }
+
+    #[inline]
+    pub(crate) const fn bits(self) -> u32 {
+        self.0
+    }
+}
+
 bitflags! {
     /// `STATX_ATTR_*` flags for use with [`Statx`].
     #[repr(transparent)]
@@ -208,14 +243,32 @@ pub fn statx<P: path::Arg, Fd: AsFd>(
     flags: AtFlags,
     mask: StatxFlags,
 ) -> io::Result<Statx> {
-    path.into_with_c_str(|path| _statx(dirfd.as_fd(), path, flags, mask))
+    path.into_with_c_str(|path| {
+        // `STATX__RESERVED` could take on arbitrary new meaning in the future.
+        // Linux currently rejects it with `EINVAL`, so do the same before
+        // sanitizing the request.
+        #[cfg(all(not(linux_raw), not(target_env = "musl")))]
+        const STATX__RESERVED: u32 = c::STATX__RESERVED as u32;
+        #[cfg(any(linux_raw, target_env = "musl"))]
+        const STATX__RESERVED: u32 = linux_raw_sys::general::STATX__RESERVED;
+        if (mask.bits() & STATX__RESERVED) == STATX__RESERVED {
+            return Err(io::Errno::INVAL);
+        }
+
+        _statx(
+            dirfd.as_fd(),
+            path,
+            flags,
+            StatxRequestFlags::sanitize(mask),
+        )
+    })
 }
 
 #[cfg(not(feature = "linux_4_11"))]
 mod compat {
     use crate::fd::BorrowedFd;
     use crate::ffi::CStr;
-    use crate::fs::{AtFlags, Statx, StatxFlags};
+    use crate::fs::{AtFlags, Statx, StatxRequestFlags};
     use crate::{backend, io};
     use core::sync::atomic::{AtomicU8, Ordering};
 
@@ -233,7 +286,7 @@ mod compat {
         dirfd: BorrowedFd<'_>,
         path: &CStr,
         flags: AtFlags,
-        mask: StatxFlags,
+        mask: StatxRequestFlags,
     ) -> io::Result<Statx> {
         match STATX_STATE.load(Ordering::Relaxed) {
             0 => statx_init(dirfd, path, flags, mask),
@@ -247,7 +300,7 @@ mod compat {
         dirfd: BorrowedFd<'_>,
         path: &CStr,
         flags: AtFlags,
-        mask: StatxFlags,
+        mask: StatxRequestFlags,
     ) -> io::Result<Statx> {
         match backend::fs::syscalls::statx(dirfd, path, flags, mask) {
             Err(err) => statx_error(err),
@@ -275,3 +328,21 @@ mod compat {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{StatxFlags, StatxRequestFlags};
+
+    #[test]
+    fn request_mask_is_exactly_the_named_flags() {
+        let named = StatxFlags::all()
+            .iter_names()
+            .fold(StatxFlags::empty(), |flags, (_, flag)| flags | flag);
+
+        assert_ne!(named, StatxFlags::all());
+        assert_eq!(
+            StatxRequestFlags::sanitize(StatxFlags::all()).bits(),
+            named.bits()
+        );
+    }
+}

tests/fs/statx.rs

@@ -12,19 +12,16 @@ fn test_statx_unknown_flags() {
     #[cfg(linux_raw_dep)]
     const STATX__RESERVED: u32 = linux_raw_sys::general::STATX__RESERVED;
     let too_many_flags = StatxFlags::from_bits_retain(!STATX__RESERVED);
+    assert_eq!(too_many_flags.bits(), !STATX__RESERVED);
 
     // It's also ok to pass such flags to `statx`.
-    let result = match rustix::fs::statx(&f, "Cargo.toml", AtFlags::empty(), too_many_flags) {
+    match rustix::fs::statx(&f, "Cargo.toml", AtFlags::empty(), too_many_flags) {
         // If we don't have `statx` at all, skip the rest of this test.
-        Err(rustix::io::Errno::NOSYS) => return,
-        otherwise => otherwise.unwrap(),
-    };
-
-    // But, rustix should mask off bits it doesn't recognize, because these
-    // extra flags may tell future kernels to set extra fields beyond the
-    // extend of rustix's statx buffer. So make sure we didn't get extra
-    // fields.
-    assert_eq!(result.stx_mask & !StatxFlags::all().bits(), 0);
+        Err(rustix::io::Errno::NOSYS) => {}
+        otherwise => {
+            otherwise.unwrap();
+        }
+    }
 }
 
 #[test]