Skip to content

Commit

Permalink
refactor: I/O safety for sys/stat.rs (nix-rust#2439)
Browse files Browse the repository at this point in the history
* refactor: I/O safety for sys/stat.rs

* fix test on FreeBSD & correct changelog PR number
SteveLauC authored Jun 9, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent e0e6997 commit 8a1e9df
Showing 6 changed files with 103 additions and 123 deletions.
1 change: 1 addition & 0 deletions changelog/2439.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Module `sys/stat` now adopts I/O safety.
27 changes: 19 additions & 8 deletions src/fcntl.rs
Original file line number Diff line number Diff line change
@@ -66,9 +66,8 @@ pub use self::posix_fadvise::{posix_fadvise, PosixFadviseAdvice};
/// use nix::errno::Errno;
/// use nix::fcntl::AT_FDCWD;
/// use nix::sys::stat::fstat;
/// use std::os::fd::AsRawFd;
///
/// let never = fstat(AT_FDCWD.as_raw_fd()).unwrap();
/// let never = fstat(AT_FDCWD).unwrap();
/// ```
//
// SAFETY:
@@ -585,13 +584,20 @@ unsafe fn inner_readlink<P: ?Sized + NixPath>(
Some(_) => unreachable!("redox does not have readlinkat(2)"),
#[cfg(any(linux_android, target_os = "freebsd", target_os = "hurd"))]
Some(dirfd) => {
// SAFETY:
//
// If this call of `borrow_raw()` is safe or not depends on the
// usage of `unsafe fn inner_readlink()`.
let dirfd = unsafe {
std::os::fd::BorrowedFd::borrow_raw(dirfd)
};
let flags = if path.is_empty() {
AtFlags::AT_EMPTY_PATH
} else {
AtFlags::empty()
};
super::sys::stat::fstatat(
Some(dirfd),
dirfd,
path,
flags | AtFlags::AT_SYMLINK_NOFOLLOW,
)
@@ -602,11 +608,16 @@ unsafe fn inner_readlink<P: ?Sized + NixPath>(
target_os = "freebsd",
target_os = "hurd"
)))]
Some(dirfd) => super::sys::stat::fstatat(
Some(dirfd),
path,
AtFlags::AT_SYMLINK_NOFOLLOW,
),
Some(dirfd) => {
// SAFETY:
//
// If this call of `borrow_raw()` is safe or not depends on the
// usage of `unsafe fn inner_readlink()`.
let dirfd = unsafe {
std::os::fd::BorrowedFd::borrow_raw(dirfd)
};
super::sys::stat::fstatat(dirfd, path, AtFlags::AT_SYMLINK_NOFOLLOW)
},
None => super::sys::stat::lstat(path),
}
.map(|x| x.st_size)
73 changes: 47 additions & 26 deletions src/sys/stat.rs
Original file line number Diff line number Diff line change
@@ -6,11 +6,10 @@ pub use libc::stat as FileStat;
pub use libc::{dev_t, mode_t};

#[cfg(not(target_os = "redox"))]
use crate::fcntl::{at_rawfd, AtFlags};
use crate::fcntl::AtFlags;
use crate::sys::time::{TimeSpec, TimeVal};
use crate::{errno::Errno, NixPath, Result};
use std::mem;
use std::os::unix::io::RawFd;

libc_bitflags!(
/// "File type" flags for `mknod` and related functions.
@@ -168,17 +167,18 @@ pub fn mknod<P: ?Sized + NixPath>(

/// Create a special or ordinary file, relative to a given directory.
#[cfg(not(any(apple_targets, target_os = "redox", target_os = "haiku")))]
pub fn mknodat<P: ?Sized + NixPath>(
dirfd: Option<RawFd>,
pub fn mknodat<Fd: std::os::fd::AsFd, P: ?Sized + NixPath>(
dirfd: Fd,
path: &P,
kind: SFlag,
perm: Mode,
dev: dev_t,
) -> Result<()> {
let dirfd = at_rawfd(dirfd);
use std::os::fd::AsRawFd;

let res = path.with_nix_path(|cstr| unsafe {
libc::mknodat(
dirfd,
dirfd.as_fd().as_raw_fd(),
cstr.as_ptr(),
kind.bits() | perm.bits() as mode_t,
dev,
@@ -233,26 +233,29 @@ pub fn lstat<P: ?Sized + NixPath>(path: &P) -> Result<FileStat> {
Ok(unsafe { dst.assume_init() })
}

pub fn fstat(fd: RawFd) -> Result<FileStat> {
pub fn fstat<Fd: std::os::fd::AsFd>(fd: Fd) -> Result<FileStat> {
use std::os::fd::AsRawFd;

let mut dst = mem::MaybeUninit::uninit();
let res = unsafe { libc::fstat(fd, dst.as_mut_ptr()) };
let res = unsafe { libc::fstat(fd.as_fd().as_raw_fd(), dst.as_mut_ptr()) };

Errno::result(res)?;

Ok(unsafe { dst.assume_init() })
}

#[cfg(not(target_os = "redox"))]
pub fn fstatat<P: ?Sized + NixPath>(
dirfd: Option<RawFd>,
pub fn fstatat<Fd: std::os::fd::AsFd, P: ?Sized + NixPath>(
dirfd: Fd,
pathname: &P,
f: AtFlags,
) -> Result<FileStat> {
let dirfd = at_rawfd(dirfd);
use std::os::fd::AsRawFd;

let mut dst = mem::MaybeUninit::uninit();
let res = pathname.with_nix_path(|cstr| unsafe {
libc::fstatat(
dirfd,
dirfd.as_fd().as_raw_fd(),
cstr.as_ptr(),
dst.as_mut_ptr(),
f.bits() as libc::c_int,
@@ -269,8 +272,11 @@ pub fn fstatat<P: ?Sized + NixPath>(
/// # References
///
/// [fchmod(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/fchmod.html).
pub fn fchmod(fd: RawFd, mode: Mode) -> Result<()> {
let res = unsafe { libc::fchmod(fd, mode.bits() as mode_t) };
pub fn fchmod<Fd: std::os::fd::AsFd>(fd: Fd, mode: Mode) -> Result<()> {
use std::os::fd::AsRawFd;

let res =
unsafe { libc::fchmod(fd.as_fd().as_raw_fd(), mode.bits() as mode_t) };

Errno::result(res).map(drop)
}
@@ -299,19 +305,21 @@ pub enum FchmodatFlags {
///
/// [fchmodat(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/fchmodat.html).
#[cfg(not(target_os = "redox"))]
pub fn fchmodat<P: ?Sized + NixPath>(
dirfd: Option<RawFd>,
pub fn fchmodat<Fd: std::os::fd::AsFd, P: ?Sized + NixPath>(
dirfd: Fd,
path: &P,
mode: Mode,
flag: FchmodatFlags,
) -> Result<()> {
use std::os::fd::AsRawFd;

let atflag = match flag {
FchmodatFlags::FollowSymlink => AtFlags::empty(),
FchmodatFlags::NoFollowSymlink => AtFlags::AT_SYMLINK_NOFOLLOW,
};
let res = path.with_nix_path(|cstr| unsafe {
libc::fchmodat(
at_rawfd(dirfd),
dirfd.as_fd().as_raw_fd(),
cstr.as_ptr(),
mode.bits() as mode_t,
atflag.bits() as libc::c_int,
@@ -383,9 +391,15 @@ pub fn lutimes<P: ?Sized + NixPath>(
///
/// [futimens(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html).
#[inline]
pub fn futimens(fd: RawFd, atime: &TimeSpec, mtime: &TimeSpec) -> Result<()> {
pub fn futimens<Fd: std::os::fd::AsFd>(
fd: Fd,
atime: &TimeSpec,
mtime: &TimeSpec,
) -> Result<()> {
use std::os::fd::AsRawFd;

let times: [libc::timespec; 2] = [*atime.as_ref(), *mtime.as_ref()];
let res = unsafe { libc::futimens(fd, &times[0]) };
let res = unsafe { libc::futimens(fd.as_fd().as_raw_fd(), &times[0]) };

Errno::result(res).map(drop)
}
@@ -418,21 +432,23 @@ pub enum UtimensatFlags {
///
/// [utimensat(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/utimens.html).
#[cfg(not(target_os = "redox"))]
pub fn utimensat<P: ?Sized + NixPath>(
dirfd: Option<RawFd>,
pub fn utimensat<Fd: std::os::fd::AsFd, P: ?Sized + NixPath>(
dirfd: Fd,
path: &P,
atime: &TimeSpec,
mtime: &TimeSpec,
flag: UtimensatFlags,
) -> Result<()> {
use std::os::fd::AsRawFd;

let atflag = match flag {
UtimensatFlags::FollowSymlink => AtFlags::empty(),
UtimensatFlags::NoFollowSymlink => AtFlags::AT_SYMLINK_NOFOLLOW,
};
let times: [libc::timespec; 2] = [*atime.as_ref(), *mtime.as_ref()];
let res = path.with_nix_path(|cstr| unsafe {
libc::utimensat(
at_rawfd(dirfd),
dirfd.as_fd().as_raw_fd(),
cstr.as_ptr(),
&times[0],
atflag.bits() as libc::c_int,
@@ -443,14 +459,19 @@ pub fn utimensat<P: ?Sized + NixPath>(
}

#[cfg(not(target_os = "redox"))]
pub fn mkdirat<P: ?Sized + NixPath>(
fd: Option<RawFd>,
pub fn mkdirat<Fd: std::os::fd::AsFd, P: ?Sized + NixPath>(
fd: Fd,
path: &P,
mode: Mode,
) -> Result<()> {
let fd = at_rawfd(fd);
use std::os::fd::AsRawFd;

let res = path.with_nix_path(|cstr| unsafe {
libc::mkdirat(fd, cstr.as_ptr(), mode.bits() as mode_t)
libc::mkdirat(
fd.as_fd().as_raw_fd(),
cstr.as_ptr(),
mode.bits() as mode_t,
)
})?;

Errno::result(res).map(drop)
102 changes: 29 additions & 73 deletions test/sys/test_stat.rs
Original file line number Diff line number Diff line change
@@ -5,7 +5,6 @@ use std::fs::File;
use std::os::unix::fs::symlink;
#[cfg(not(any(target_os = "redox", target_os = "haiku")))]
use std::os::unix::fs::PermissionsExt;
use std::os::unix::prelude::AsRawFd;
#[cfg(not(target_os = "redox"))]
use std::path::Path;
#[cfg(not(any(target_os = "redox", target_os = "haiku")))]
@@ -102,27 +101,21 @@ fn test_stat_and_fstat() {
let stat_result = stat(&filename);
assert_stat_results(stat_result);

let fstat_result = fstat(file.as_raw_fd());
let fstat_result = fstat(&file);
assert_stat_results(fstat_result);
}

#[test]
#[cfg(not(any(target_os = "netbsd", target_os = "redox")))]
fn test_fstatat() {
use std::os::fd::AsRawFd;

let tempdir = tempfile::tempdir().unwrap();
let filename = tempdir.path().join("foo.txt");
File::create(&filename).unwrap();
let dirfd =
fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty())
.unwrap();

let result = stat::fstatat(
Some(dirfd.as_raw_fd()),
&filename,
fcntl::AtFlags::empty(),
);
let result = stat::fstatat(&dirfd, &filename, fcntl::AtFlags::empty());
assert_stat_results(result);
}

@@ -147,7 +140,7 @@ fn test_stat_fstat_lstat() {
let lstat_result = lstat(&linkname);
assert_lstat_results(lstat_result);

let fstat_result = fstat(link.as_raw_fd());
let fstat_result = fstat(&link);
assert_stat_results(fstat_result);
}

@@ -160,14 +153,14 @@ fn test_fchmod() {
let mut mode1 = Mode::empty();
mode1.insert(Mode::S_IRUSR);
mode1.insert(Mode::S_IWUSR);
fchmod(file.as_raw_fd(), mode1).unwrap();
fchmod(&file, mode1).unwrap();

let file_stat1 = stat(&filename).unwrap();
assert_eq!(file_stat1.st_mode as mode_t & 0o7777, mode1.bits());

let mut mode2 = Mode::empty();
mode2.insert(Mode::S_IROTH);
fchmod(file.as_raw_fd(), mode2).unwrap();
fchmod(&file, mode2).unwrap();

let file_stat2 = stat(&filename).unwrap();
assert_eq!(file_stat2.st_mode as mode_t & 0o7777, mode2.bits());
@@ -176,8 +169,6 @@ fn test_fchmod() {
#[test]
#[cfg(not(target_os = "redox"))]
fn test_fchmodat() {
use std::os::fd::AsRawFd;

let _dr = crate::DirRestore::new();
let tempdir = tempfile::tempdir().unwrap();
let filename = "foo.txt";
@@ -191,13 +182,7 @@ fn test_fchmodat() {
let mut mode1 = Mode::empty();
mode1.insert(Mode::S_IRUSR);
mode1.insert(Mode::S_IWUSR);
fchmodat(
Some(dirfd.as_raw_fd()),
filename,
mode1,
FchmodatFlags::FollowSymlink,
)
.unwrap();
fchmodat(&dirfd, filename, mode1, FchmodatFlags::FollowSymlink).unwrap();

let file_stat1 = stat(&fullpath).unwrap();
assert_eq!(file_stat1.st_mode as mode_t & 0o7777, mode1.bits());
@@ -206,7 +191,13 @@ fn test_fchmodat() {

let mut mode2 = Mode::empty();
mode2.insert(Mode::S_IROTH);
fchmodat(None, filename, mode2, FchmodatFlags::FollowSymlink).unwrap();
fchmodat(
fcntl::AT_FDCWD,
filename,
mode2,
FchmodatFlags::FollowSymlink,
)
.unwrap();

let file_stat2 = stat(&fullpath).unwrap();
assert_eq!(file_stat2.st_mode as mode_t & 0o7777, mode2.bits());
@@ -279,29 +270,20 @@ fn test_lutimes() {
#[test]
#[cfg(not(any(target_os = "redox", target_os = "haiku")))]
fn test_futimens() {
use std::os::fd::AsRawFd;

let tempdir = tempfile::tempdir().unwrap();
let fullpath = tempdir.path().join("file");
drop(File::create(&fullpath).unwrap());

let fd = fcntl::open(&fullpath, fcntl::OFlag::empty(), stat::Mode::empty())
.unwrap();

futimens(
fd.as_raw_fd(),
&TimeSpec::seconds(10),
&TimeSpec::seconds(20),
)
.unwrap();
futimens(&fd, &TimeSpec::seconds(10), &TimeSpec::seconds(20)).unwrap();
assert_times_eq(10, 20, &fs::metadata(&fullpath).unwrap());
}

#[test]
#[cfg(not(any(target_os = "redox", target_os = "haiku")))]
fn test_utimensat() {
use std::os::fd::AsRawFd;

let _dr = crate::DirRestore::new();
let tempdir = tempfile::tempdir().unwrap();
let filename = "foo.txt";
@@ -313,7 +295,7 @@ fn test_utimensat() {
.unwrap();

utimensat(
Some(dirfd.as_raw_fd()),
&dirfd,
filename,
&TimeSpec::seconds(12345),
&TimeSpec::seconds(678),
@@ -325,7 +307,7 @@ fn test_utimensat() {
chdir(tempdir.path()).unwrap();

utimensat(
None,
fcntl::AT_FDCWD,
filename,
&TimeSpec::seconds(500),
&TimeSpec::seconds(800),
@@ -343,25 +325,21 @@ fn test_mkdirat_success_path() {
let dirfd =
fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty())
.unwrap();
mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU)
.expect("mkdirat failed");
mkdirat(&dirfd, filename, Mode::S_IRWXU).expect("mkdirat failed");
assert!(Path::exists(&tempdir.path().join(filename)));
}

#[test]
#[cfg(not(any(target_os = "redox", target_os = "haiku")))]
fn test_mkdirat_success_mode() {
use std::os::fd::AsRawFd;

let expected_bits =
stat::SFlag::S_IFDIR.bits() | stat::Mode::S_IRWXU.bits();
let tempdir = tempfile::tempdir().unwrap();
let filename = "example_subdir";
let dirfd =
fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty())
.unwrap();
mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU)
.expect("mkdirat failed");
mkdirat(&dirfd, filename, Mode::S_IRWXU).expect("mkdirat failed");
let permissions = fs::metadata(tempdir.path().join(filename))
.unwrap()
.permissions();
@@ -372,8 +350,6 @@ fn test_mkdirat_success_mode() {
#[test]
#[cfg(not(target_os = "redox"))]
fn test_mkdirat_fail() {
use std::os::fd::AsRawFd;

let tempdir = tempfile::tempdir().unwrap();
let not_dir_filename = "example_not_dir";
let filename = "example_subdir_dir";
@@ -383,8 +359,7 @@ fn test_mkdirat_fail() {
stat::Mode::empty(),
)
.unwrap();
let result =
mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU).unwrap_err();
let result = mkdirat(dirfd, filename, Mode::S_IRWXU).unwrap_err();
assert_eq!(result, Errno::ENOTDIR);
}

@@ -424,30 +399,17 @@ fn test_mknodat() {
let tempdir = tempfile::tempdir().unwrap();
let target_dir =
Dir::open(tempdir.path(), OFlag::O_DIRECTORY, Mode::S_IRWXU).unwrap();
mknodat(
Some(target_dir.as_raw_fd()),
file_name,
SFlag::S_IFREG,
Mode::S_IRWXU,
0,
)
.unwrap();
let mode = fstatat(
Some(target_dir.as_raw_fd()),
file_name,
AtFlags::AT_SYMLINK_NOFOLLOW,
)
.unwrap()
.st_mode as mode_t;
mknodat(&target_dir, file_name, SFlag::S_IFREG, Mode::S_IRWXU, 0).unwrap();
let mode = fstatat(&target_dir, file_name, AtFlags::AT_SYMLINK_NOFOLLOW)
.unwrap()
.st_mode as mode_t;
assert_eq!(mode & libc::S_IFREG, libc::S_IFREG);
assert_eq!(mode & libc::S_IRWXU, libc::S_IRWXU);
}

#[test]
#[cfg(not(any(target_os = "redox", target_os = "haiku")))]
fn test_futimens_unchanged() {
use std::os::fd::AsRawFd;

let tempdir = tempfile::tempdir().unwrap();
let fullpath = tempdir.path().join("file");
drop(File::create(&fullpath).unwrap());
@@ -463,8 +425,7 @@ fn test_futimens_unchanged() {
.modified()
.unwrap();

futimens(fd.as_raw_fd(), &TimeSpec::UTIME_OMIT, &TimeSpec::UTIME_OMIT)
.unwrap();
futimens(&fd, &TimeSpec::UTIME_OMIT, &TimeSpec::UTIME_OMIT).unwrap();

let new_atime = fs::metadata(fullpath.as_path())
.unwrap()
@@ -481,8 +442,6 @@ fn test_futimens_unchanged() {
#[test]
#[cfg(not(any(target_os = "redox", target_os = "haiku")))]
fn test_utimensat_unchanged() {
use std::os::fd::AsRawFd;

let _dr = crate::DirRestore::new();
let tempdir = tempfile::tempdir().unwrap();
let filename = "foo.txt";
@@ -501,7 +460,7 @@ fn test_utimensat_unchanged() {
.modified()
.unwrap();
utimensat(
Some(dirfd.as_raw_fd()),
&dirfd,
filename,
&TimeSpec::UTIME_OMIT,
&TimeSpec::UTIME_OMIT,
@@ -529,23 +488,20 @@ fn test_chflags() {
sys::stat::{fstat, FileFlag},
unistd::chflags,
};
use std::os::unix::io::AsRawFd;
use tempfile::NamedTempFile;

let f = NamedTempFile::new().unwrap();

let initial = FileFlag::from_bits_truncate(
fstat(f.as_raw_fd()).unwrap().st_flags.into(),
);
let initial =
FileFlag::from_bits_truncate(fstat(&f).unwrap().st_flags.into());
// UF_OFFLINE is preserved by all FreeBSD file systems, but not interpreted
// in any way, so it's handy for testing.
let commanded = initial ^ FileFlag::UF_OFFLINE;

chflags(f.path(), commanded).unwrap();

let changed = FileFlag::from_bits_truncate(
fstat(f.as_raw_fd()).unwrap().st_flags.into(),
);
let changed =
FileFlag::from_bits_truncate(fstat(&f).unwrap().st_flags.into());

assert_eq!(commanded, changed);
}
8 changes: 2 additions & 6 deletions test/test_fcntl.rs
Original file line number Diff line number Diff line change
@@ -405,7 +405,6 @@ mod linux_android {
fn test_ofd_write_lock() {
use nix::sys::stat::fstat;
use std::mem;
use std::os::fd::AsRawFd;

let tmp = NamedTempFile::new().unwrap();

@@ -416,8 +415,7 @@ mod linux_android {
// skip the test.
skip!("/proc/locks does not work on overlayfs");
}
let inode =
fstat(tmp.as_raw_fd()).expect("fstat failed").st_ino as usize;
let inode = fstat(&tmp).expect("fstat failed").st_ino as usize;

let mut flock: libc::flock = unsafe {
mem::zeroed() // required for Linux/mips
@@ -445,7 +443,6 @@ mod linux_android {
fn test_ofd_read_lock() {
use nix::sys::stat::fstat;
use std::mem;
use std::os::fd::AsRawFd;

let tmp = NamedTempFile::new().unwrap();

@@ -456,8 +453,7 @@ mod linux_android {
// skip the test.
skip!("/proc/locks does not work on overlayfs");
}
let inode =
fstat(tmp.as_raw_fd()).expect("fstat failed").st_ino as usize;
let inode = fstat(&tmp).expect("fstat failed").st_ino as usize;

let mut flock: libc::flock = unsafe {
mem::zeroed() // required for Linux/mips
15 changes: 5 additions & 10 deletions test/test_unistd.rs
Original file line number Diff line number Diff line change
@@ -192,12 +192,8 @@ fn test_mkfifoat() {

mkfifoat(Some(dirfd.as_raw_fd()), mkfifoat_name, Mode::S_IRUSR).unwrap();

let stats = stat::fstatat(
Some(dirfd.as_raw_fd()),
mkfifoat_name,
fcntl::AtFlags::empty(),
)
.unwrap();
let stats =
stat::fstatat(&dirfd, mkfifoat_name, fcntl::AtFlags::empty()).unwrap();
let typ = stat::SFlag::from_bits_truncate(stats.st_mode);
assert_eq!(typ, SFlag::S_IFIFO);
}
@@ -231,8 +227,7 @@ fn test_mkfifoat_directory() {
let tempdir = tempdir().unwrap();
let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap();
let mkfifoat_dir = "mkfifoat_dir";
stat::mkdirat(Some(dirfd.as_raw_fd()), mkfifoat_dir, Mode::S_IRUSR)
.unwrap();
stat::mkdirat(&dirfd, mkfifoat_dir, Mode::S_IRUSR).unwrap();

mkfifoat(Some(dirfd.as_raw_fd()), mkfifoat_dir, Mode::S_IRUSR)
.expect_err("assertion failed");
@@ -742,12 +737,12 @@ fn test_getresgid() {
fn test_pipe() {
let (fd0, fd1) = pipe().unwrap();
let m0 = stat::SFlag::from_bits_truncate(
stat::fstat(fd0.as_raw_fd()).unwrap().st_mode as mode_t,
stat::fstat(&fd0).unwrap().st_mode as mode_t,
);
// S_IFIFO means it's a pipe
assert_eq!(m0, SFlag::S_IFIFO);
let m1 = stat::SFlag::from_bits_truncate(
stat::fstat(fd1.as_raw_fd()).unwrap().st_mode as mode_t,
stat::fstat(&fd1).unwrap().st_mode as mode_t,
);
assert_eq!(m1, SFlag::S_IFIFO);
}

0 comments on commit 8a1e9df

Please sign in to comment.