Skip to content

Commit

Permalink
Guard against future statx fields. (#379)
Browse files Browse the repository at this point in the history
* Guard against future `statx` fields.

Future versions of Linux may recognize new `statx` mask flags, which
enable writing to new `struct statx` fields. If a rustix user constructs
a flags value with `from_bits_unchecked`, they could potentially create
a flags value that causes `statx` on such a future Linux version to write
fields outside the buffer, evoking undefined behavior.

This is very unlikely to be a practical problem today, because:

 - Scanning all known public users of rustix, I don't see any constructing
   `StatxFlags` values this way.

 - It's unlikely that any rustix user would ever have a need to construct
   `StatxFlags` values this way.

 - No existing version of Linux increases the size of `struct statx`
   beyond what rustix currently knows about.

 - `struct statx` contains several spare fields, including a
   `__u64 __spare3[12]` specifically for expansion, so even if a future
   Linux version defines new fields, it has lots of space available before
   it needs to start increasing the size of `struct statx`.
  • Loading branch information
sunfishcode committed Jul 13, 2022
1 parent 7fa7a98 commit 2c6db2e
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 1 deletion.
22 changes: 22 additions & 0 deletions src/backend/libc/fs/syscalls.rs
Expand Up @@ -1456,6 +1456,28 @@ pub(crate) fn statx(
flags: AtFlags,
mask: StatxFlags,
) -> 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(not(any(target_os = "android", target_env = "musl")))]
const STATX__RESERVED: u32 = libc::STATX__RESERVED as u32;
#[cfg(any(target_os = "android", 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(
Expand Down
20 changes: 19 additions & 1 deletion src/backend/linux_raw/fs/syscalls.rs
Expand Up @@ -38,7 +38,7 @@ use linux_raw_sys::general::{
__kernel_fsid_t, __kernel_timespec, open_how, statx, AT_EACCESS, AT_FDCWD, AT_REMOVEDIR,
AT_SYMLINK_NOFOLLOW, F_ADD_SEALS, F_DUPFD, F_DUPFD_CLOEXEC, F_GETFD, F_GETFL, F_GETLEASE,
F_GETOWN, F_GETPIPE_SZ, F_GETSIG, F_GET_SEALS, F_SETFD, F_SETFL, F_SETPIPE_SZ, SEEK_CUR,
SEEK_END, SEEK_SET,
SEEK_END, SEEK_SET, STATX__RESERVED,
};
#[cfg(target_pointer_width = "32")]
use {
Expand Down Expand Up @@ -722,6 +722,24 @@ pub(crate) fn statx(
flags: AtFlags,
mask: StatxFlags,
) -> 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!(
Expand Down
2 changes: 2 additions & 0 deletions tests/fs/main.rs
Expand Up @@ -37,5 +37,7 @@ mod readdir;
mod renameat;
#[cfg(not(any(target_os = "illumos", target_os = "redox", target_os = "wasi")))]
mod statfs;
#[cfg(any(target_os = "android", target_os = "linux"))]
mod statx;
mod utimensat;
mod y2038;
37 changes: 37 additions & 0 deletions tests/fs/statx.rs
@@ -0,0 +1,37 @@
#[test]
fn test_statx_unknown_flags() {
use rustix::fs::{AtFlags, StatxFlags};

let f = std::fs::File::open(".").unwrap();

// It's ok (though still unwise) to construct flags values that have
// unknown bits. Exclude `STATX__RESERVED` here as that evokes an explicit
// failure; that's tested separately below.
let too_many_flags =
unsafe { StatxFlags::from_bits_unchecked(!0 & !linux_raw_sys::general::STATX__RESERVED) };

// It's also ok to pass such flags to `statx`.
let result = rustix::fs::statx(&f, "Cargo.toml", AtFlags::empty(), too_many_flags).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);
}

#[test]
fn test_statx_reserved() {
use rustix::fs::{AtFlags, StatxFlags};

let f = std::fs::File::open(".").unwrap();

// It's ok (though still unwise) to construct a `STATX__RESERVED` flag
// value but `statx` should reliably fail with `INVAL`.
let reserved =
unsafe { StatxFlags::from_bits_unchecked(linux_raw_sys::general::STATX__RESERVED) };
match rustix::fs::statx(&f, "Cargo.toml", AtFlags::empty(), reserved) {
Ok(_) => panic!("statx succeeded with `STATX__RESERVED`"),
Err(err) => assert_eq!(err, rustix::io::Errno::INVAL),
}
}

0 comments on commit 2c6db2e

Please sign in to comment.