Skip to content

Commit ba9e1d6

Browse files
committed
refactor: use rustix safe wrappers in O_PATH fallback
Replace raw unsafe libc::openat/libc::chmod calls in chmod_at_via_opath with rustix::fs::openat and rustix::fs::chmod, eliminating all unsafe blocks from the O_PATH fallback path. Also includes formatting cleanup per review feedback.
1 parent a07e076 commit ba9e1d6

1 file changed

Lines changed: 18 additions & 29 deletions

File tree

src/uucore/src/lib/features/safe_traversal.rs

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
//
1111
// spell-checker:ignore CLOEXEC RDONLY TOCTOU closedir dirp fdopendir fstatat openat REMOVEDIR unlinkat smallfile
1212
// spell-checker:ignore RAII dirfd fchownat fchown FchmodatFlags fchmodat fchmod mkdirat CREAT WRONLY ELOOP ENOTDIR
13-
// spell-checker:ignore atimensec mtimensec ctimensec
13+
// spell-checker:ignore atimensec mtimensec ctimensec opath chmods
1414

1515
#[cfg(test)]
1616
use std::os::unix::ffi::OsStringExt;
@@ -389,32 +389,25 @@ impl DirFd {
389389
/// Opens the file with O_PATH|O_NOFOLLOW to get an fd without following
390390
/// symlinks, then chmods via /proc/self/fd/{fd}. This avoids the TOCTOU
391391
/// race because the fd pins the inode.
392+
///
392393
#[cfg(target_os = "linux")]
393-
fn chmod_at_via_opath(&self, name: &CStr, mode: u32) -> io::Result<()> {
394-
use std::os::unix::io::FromRawFd;
395-
396-
let fd = unsafe {
397-
libc::openat(
398-
self.fd.as_raw_fd(),
399-
name.as_ptr(),
400-
libc::O_PATH | libc::O_NOFOLLOW | libc::O_CLOEXEC,
401-
)
402-
};
403-
if fd < 0 {
404-
return Err(io::Error::last_os_error());
405-
}
406-
let fd = unsafe { OwnedFd::from_raw_fd(fd) };
394+
fn chmod_at_via_opath(&self, name: &std::ffi::CStr, mode: u32) -> io::Result<()> {
395+
use rustix::fs::{Mode, OFlags, chmod, openat};
396+
397+
let fd = openat(
398+
&self.fd,
399+
name,
400+
OFlags::PATH | OFlags::NOFOLLOW | OFlags::CLOEXEC,
401+
Mode::empty(),
402+
)
403+
.map_err(|e| io::Error::from_raw_os_error(e.raw_os_error()))?;
407404

408405
let proc_path = format!("/proc/self/fd/{}\0", fd.as_raw_fd());
409-
let proc_cstr = CStr::from_bytes_with_nul(proc_path.as_bytes())
406+
let proc_cstr = std::ffi::CStr::from_bytes_with_nul(proc_path.as_bytes())
410407
.map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "invalid proc path"))?;
411408

412-
let ret = unsafe { libc::chmod(proc_cstr.as_ptr(), mode as libc::mode_t) };
413-
if ret < 0 {
414-
Err(io::Error::last_os_error())
415-
} else {
416-
Ok(())
417-
}
409+
chmod(proc_cstr, Mode::from_bits_truncate(mode))
410+
.map_err(|e| io::Error::from_raw_os_error(e.raw_os_error()))
418411
}
419412

420413
/// Change mode of this directory
@@ -907,8 +900,8 @@ impl std::os::unix::fs::MetadataExt for Metadata {
907900
mod tests {
908901
use super::*;
909902
use std::fs;
910-
use std::os::unix::fs::symlink;
911903
use std::os::unix::fs::MetadataExt;
904+
use std::os::unix::fs::symlink;
912905
use std::os::unix::io::IntoRawFd;
913906
use tempfile::TempDir;
914907

@@ -1345,9 +1338,7 @@ mod tests {
13451338
// Create a sentinel file outside the traversal directory
13461339
let sentinel = temp_dir.path().join("sentinel");
13471340
fs::write(&sentinel, "victim").unwrap();
1348-
let sentinel_mode = fs::symlink_metadata(&sentinel)
1349-
.unwrap()
1350-
.mode();
1341+
let sentinel_mode = fs::symlink_metadata(&sentinel).unwrap().mode();
13511342

13521343
// Create a subdirectory with a symlink pointing to the sentinel
13531344
let subdir = temp_dir.path().join("subdir");
@@ -1364,9 +1355,7 @@ mod tests {
13641355
// which is acceptable — the important thing is the target is NOT modified.
13651356
if let Ok(()) = result {
13661357
// fchmodat2 succeeded: verify sentinel mode is unchanged
1367-
let new_sentinel_mode = fs::symlink_metadata(&sentinel)
1368-
.unwrap()
1369-
.mode();
1358+
let new_sentinel_mode = fs::symlink_metadata(&sentinel).unwrap().mode();
13701359
assert_eq!(
13711360
new_sentinel_mode, sentinel_mode,
13721361
"sentinel mode should not change when chmod'ing symlink with NoFollow"

0 commit comments

Comments
 (0)