Merge pull request #5014 from RalfJung/permissions support chmod and fchmod on unix hosts
diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 0766352..7489873 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs
@@ -362,6 +362,26 @@ fn emulate_foreign_item_inner( let result = this.stat(path, buf)?; this.write_scalar(result, dest)?; } + "chmod" => { + let [path, mode] = this.check_shim_sig( + shim_sig!(extern "C" fn(*const _, libc::mode_t) -> i32), + link_name, + abi, + args, + )?; + let result = this.chmod(path, mode)?; + this.write_scalar(result, dest)?; + } + "fchmod" => { + let [fd, mode] = this.check_shim_sig( + shim_sig!(extern "C" fn(i32, libc::mode_t) -> i32), + link_name, + abi, + args, + )?; + let result = this.fchmod(fd, mode)?; + this.write_scalar(result, dest)?; + } "rename" => { // FIXME: This does not have a direct test (#3179). let [oldpath, newpath] = this.check_shim_sig( @@ -536,7 +556,7 @@ fn emulate_foreign_item_inner( this.write_scalar(result, dest)?; } - // Unnamed sockets and pipes + // Sockets and pipes "socketpair" => { let [domain, type_, protocol, sv] = this.check_shim_sig( shim_sig!(extern "C" fn(i32, i32, i32, *mut _) -> i32),
diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index b1d3951..8df5461 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs
@@ -2,10 +2,7 @@ use std::borrow::Cow; use std::ffi::OsString; -use std::fs::{ - self, DirBuilder, File, FileType, OpenOptions, TryLockError, read_dir, remove_dir, remove_file, - rename, -}; +use std::fs::{self, DirBuilder, File, FileType, OpenOptions, TryLockError}; use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write}; use std::path::{self, Path, PathBuf}; use std::time::SystemTime; @@ -236,15 +233,10 @@ fn write_stat_buf( // which can be different between the libc used by std and the libc used by everyone else. let buf = this.deref_pointer(buf_op)?; - // `libc::S_IF*` constants are of type `mode_t`, which varies in width across targets - // (`u16` on macOS, `u32` on Linux). Read the scalar using `mode_t`'s size on the target. - let mode_t_size = this.libc_ty_layout("mode_t").size; - let mode: u32 = metadata.mode.to_uint(mode_t_size)?.try_into().unwrap(); - this.write_int_fields_named( &[ ("st_dev", metadata.dev.unwrap_or(0).into()), - ("st_mode", mode.into()), + ("st_mode", metadata.mode.into()), ("st_nlink", metadata.nlink.unwrap_or(0).into()), ("st_ino", metadata.ino.unwrap_or(0).into()), ("st_uid", metadata.uid.unwrap_or(0).into()), @@ -346,6 +338,17 @@ fn dir_entry_fields( }, }) } + + #[cfg(unix)] + fn host_permissions_from_mode(&self, mode: u32) -> InterpResult<'tcx, fs::Permissions> { + use std::os::unix::fs::PermissionsExt; + interp_ok(fs::Permissions::from_mode(mode)) + } + + #[cfg(not(unix))] + fn host_permissions_from_mode(&self, _mode: u32) -> InterpResult<'tcx, fs::Permissions> { + throw_unsup_format!("setting file permissions is only supported on Unix hosts") + } } impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} @@ -547,7 +550,7 @@ fn unlink(&mut self, path_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied); } - let result = remove_file(path).map(|_| 0); + let result = fs::remove_file(path).map(|_| 0); interp_ok(Scalar::from_i32(this.try_unwrap_io_result(result)?)) } @@ -766,15 +769,6 @@ fn linux_statx( mask |= this.eval_libc_u32("STATX_BLOCKS"); } - // `statx.stx_mode` is `__u16`. `libc::S_IF*` are of type `mode_t`, which varies in - // width across targets (`u16` on macOS, `u32` on Linux). Read using `mode_t`'s size. - let mode_t_size = this.libc_ty_layout("mode_t").size; - let mode: u16 = metadata - .mode - .to_uint(mode_t_size)? - .try_into() - .unwrap_or_else(|_| bug!("libc contains bad value for constant")); - // We need to set the corresponding bits of `mask` if the access, creation and modification // times were available. Otherwise we let them be zero. let (access_sec, access_nsec) = metadata @@ -805,12 +799,12 @@ fn linux_statx( this.write_int_fields_named( &[ ("stx_mask", mask.into()), + ("stx_mode", metadata.mode.into()), ("stx_blksize", metadata.blksize.unwrap_or(0).into()), ("stx_attributes", 0), ("stx_nlink", metadata.nlink.unwrap_or(0).into()), ("stx_uid", metadata.uid.unwrap_or(0).into()), ("stx_gid", metadata.gid.unwrap_or(0).into()), - ("stx_mode", mode.into()), ("stx_ino", metadata.ino.unwrap_or(0).into()), ("stx_size", metadata.size.into()), ("stx_blocks", metadata.blocks.unwrap_or(0).into()), @@ -858,6 +852,47 @@ fn linux_statx( interp_ok(Scalar::from_i32(0)) } + fn chmod(&mut self, path_op: &OpTy<'tcx>, mode_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let path_ptr = this.read_pointer(path_op)?; + let mode = this.read_scalar(mode_op)?.to_uint(this.libc_ty_layout("mode_t").size)?; + + if this.ptr_is_null(path_ptr)? { + return this.set_last_error_and_return_i32(LibcError("EFAULT")); + } + let path = this.read_path_from_c_str(path_ptr)?; + + let permissions = this.host_permissions_from_mode(mode.try_into().unwrap())?; + if let Err(err) = fs::set_permissions(path, permissions) { + return this.set_last_error_and_return_i32(IoError::HostError(err)); + } + + interp_ok(Scalar::from_i32(0)) + } + + fn fchmod(&mut self, fd_op: &OpTy<'tcx>, mode_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let fd_num = this.read_scalar(fd_op)?.to_i32()?; + let mode = this.read_scalar(mode_op)?.to_uint(this.libc_ty_layout("mode_t").size)?; + + let Some(fd) = this.machine.fds.get(fd_num) else { + return this.set_last_error_and_return_i32(LibcError("EBADF")); + }; + let Some(file) = fd.downcast::<FileHandle>() else { + // The docs don't talk about what happens for non-regular files... + throw_unsup_format!("`fchmod` is only supported on regular files") + }; + + let permissions = this.host_permissions_from_mode(mode.try_into().unwrap())?; + if let Err(err) = file.file.set_permissions(permissions) { + return this.set_last_error_and_return_i32(IoError::HostError(err)); + } + + interp_ok(Scalar::from_i32(0)) + } + fn rename( &mut self, oldpath_op: &OpTy<'tcx>, @@ -881,7 +916,7 @@ fn rename( return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied); } - let result = rename(oldpath, newpath).map(|_| 0); + let result = fs::rename(oldpath, newpath).map(|_| 0); interp_ok(Scalar::from_i32(this.try_unwrap_io_result(result)?)) } @@ -931,7 +966,7 @@ fn rmdir(&mut self, path_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied); } - let result = remove_dir(path).map(|_| 0i32); + let result = fs::remove_dir(path).map(|_| 0i32); interp_ok(Scalar::from_i32(this.try_unwrap_io_result(result)?)) } @@ -948,7 +983,7 @@ fn opendir(&mut self, name_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { return interp_ok(Scalar::null_ptr(this)); } - let result = read_dir(name); + let result = fs::read_dir(name); match result { Ok(dir_iter) => { @@ -1684,7 +1719,8 @@ fn file_type_to_mode_name(file_type: std::fs::FileType) -> &'static str { /// expose it. `statx` must only advertise the corresponding `STATX_*` bit when the field is `Some`; /// legacy `stat` writes zero for `None` to preserve the old fallback behavior. struct FileMetadata { - mode: Scalar, + /// This holds both the file type (dir, regular, symlink, ...) and permissions. + mode: u32, size: u64, created: Option<(u64, u32)>, accessed: Option<(u64, u32)>, @@ -1728,6 +1764,9 @@ fn synthetic<'tcx>( mode_name: &str, ) -> InterpResult<'tcx, Result<FileMetadata, IoError>> { let mode = ecx.eval_libc(mode_name); + let mode: u32 = mode.to_uint(ecx.libc_ty_layout("mode_t").size)?.try_into().unwrap(); + // We observed 0x777 on sockets and 0x600 on pipes... + let mode = mode | 0o666; interp_ok(Ok(FileMetadata { mode, size: 0, @@ -1757,6 +1796,7 @@ fn from_meta<'tcx>( let file_type = metadata.file_type(); let mode = ecx.eval_libc(file_type_to_mode_name(file_type)); + let mut mode = mode.to_uint(ecx.libc_ty_layout("mode_t").size)?.try_into().unwrap(); let size = metadata.len(); @@ -1769,6 +1809,8 @@ fn from_meta<'tcx>( cfg_select! { unix => { use std::os::unix::fs::MetadataExt; + use std::os::unix::fs::PermissionsExt; + let dev = metadata.dev(); let ino = metadata.ino(); let nlink = metadata.nlink(); @@ -1777,6 +1819,8 @@ fn from_meta<'tcx>( let blksize = metadata.blksize(); let blocks = metadata.blocks(); + mode |= metadata.permissions().mode(); + interp_ok(Ok(FileMetadata { mode, size, @@ -1792,20 +1836,25 @@ fn from_meta<'tcx>( blocks: Some(blocks), })) } - _ => interp_ok(Ok(FileMetadata { - mode, - size, - created, - accessed, - modified, - dev: None, - ino: None, - nlink: None, - uid: None, - gid: None, - blksize: None, - blocks: None, - })), + _ => { + // Emulate "everyone can read" or "everyone can read and write". + mode |= if metadata.permissions().readonly() { 0o111 } else { 0o333 }; + + interp_ok(Ok(FileMetadata { + mode, + size, + created, + accessed, + modified, + dev: None, + ino: None, + nlink: None, + uid: None, + gid: None, + blksize: None, + blocks: None, + })) + }, } } }
diff --git a/src/tools/miri/tests/pass-dep/libc/libc-fs-permissions.rs b/src/tools/miri/tests/pass-dep/libc/libc-fs-permissions.rs new file mode 100644 index 0000000..f03e1ad --- /dev/null +++ b/src/tools/miri/tests/pass-dep/libc/libc-fs-permissions.rs
@@ -0,0 +1,50 @@ +//@ignore-target: windows # no libc +//@ignore-host: windows # needs unix PermissionExt +//@compile-flags: -Zmiri-disable-isolation + +#![feature(io_error_more)] +#![feature(io_error_uncategorized)] + +use std::ffi::{CStr, CString}; +use std::mem::MaybeUninit; +use std::os::unix::ffi::OsStrExt; + +#[path = "../../utils/mod.rs"] +mod utils; + +#[path = "../../utils/libc.rs"] +mod libc_utils; +use libc_utils::{errno_check, errno_result}; + +fn main() { + test_chmod(); + test_fchmod(); +} + +#[track_caller] +fn getmod(path: &CStr) -> u32 { + let mut stat = MaybeUninit::<libc::stat>::uninit(); + unsafe { errno_check(libc::stat(path.as_ptr(), stat.as_mut_ptr())) }; + u32::from(unsafe { stat.assume_init_ref().st_mode & !libc::S_IFMT }) +} + +fn test_chmod() { + let path = utils::prepare_with_content("miri_test_libc_chmod.txt", b"abcdef"); + let c_path = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed"); + + unsafe { errno_check(libc::chmod(c_path.as_ptr(), 0o777)) }; + assert_eq!(getmod(&c_path), 0o777); + unsafe { errno_check(libc::chmod(c_path.as_ptr(), 0o610)) }; + assert_eq!(getmod(&c_path), 0o610); +} + +fn test_fchmod() { + let path = utils::prepare_with_content("miri_test_libc_chmod.txt", b"abcdef"); + let c_path = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed"); + + let fd = unsafe { errno_result(libc::open(c_path.as_ptr(), libc::O_RDONLY)).unwrap() }; + unsafe { errno_check(libc::fchmod(fd, 0o777)) }; + assert_eq!(getmod(&c_path), 0o777); + unsafe { errno_check(libc::fchmod(fd, 0o610)) }; + assert_eq!(getmod(&c_path), 0o610); +}
diff --git a/src/tools/miri/tests/pass-dep/libc/libc-fs.rs b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs index 23f2bf6..a388943 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-fs.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs
@@ -605,6 +605,7 @@ fn test_fstat() { assert_eq!(stat.st_size, 5); assert_eq!(stat.st_mode & libc::S_IFMT, libc::S_IFREG); + assert_ne!(stat.st_mode & !libc::S_IFMT, 0, "some permission should be set"); // Check that all fields are initialized. check_stat_fields(stat); @@ -625,6 +626,7 @@ fn test_stat() { assert_eq!(stat.st_size, 5); assert_eq!(stat.st_mode & libc::S_IFMT, libc::S_IFREG); + assert_ne!(stat.st_mode & !libc::S_IFMT, 0, "some permission should be set"); // Check that all fields are initialized. check_stat_fields(stat); @@ -648,6 +650,7 @@ fn test_lstat() { let stat = unsafe { stat.assume_init_ref() }; assert_eq!(stat.st_mode & libc::S_IFMT, libc::S_IFLNK); + assert_ne!(stat.st_mode & !libc::S_IFMT, 0, "some permission should be set"); // Check that all fields are initialized. check_stat_fields(stat);
diff --git a/src/tools/miri/tests/pass-dep/libc/libc-fstat-non-file.rs b/src/tools/miri/tests/pass-dep/libc/libc-fstat-non-file.rs index c29c8ce..cf848f1 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-fstat-non-file.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-fstat-non-file.rs
@@ -55,6 +55,7 @@ fn test_fstat_socketpair() { libc::S_IFSOCK, "socketpair should have S_IFSOCK mode" ); + assert_ne!(stat.st_mode & !libc::S_IFMT, 0, "socketpair should have permissions"); assert_eq!(stat.st_size, 0, "socketpair should have size 0"); assert_stat_fields_are_accessible(stat); } @@ -72,6 +73,7 @@ fn test_fstat_pipe() { let mut buf = MaybeUninit::uninit(); let stat = do_fstat(*fd, &mut buf); assert_eq!(stat.st_mode & libc::S_IFMT, libc::S_IFIFO, "pipe should have S_IFIFO mode"); + assert_ne!(stat.st_mode & !libc::S_IFMT, 0, "pipe should have permissions"); assert_eq!(stat.st_size, 0, "pipe should have size 0"); assert_stat_fields_are_accessible(stat); }
diff --git a/src/tools/miri/tests/pass/shims/fs-permissions.rs b/src/tools/miri/tests/pass/shims/fs-permissions.rs new file mode 100644 index 0000000..51b371e --- /dev/null +++ b/src/tools/miri/tests/pass/shims/fs-permissions.rs
@@ -0,0 +1,61 @@ +//@compile-flags: -Zmiri-disable-isolation +//@ignore-target: windows # shim not supported +//@ignore-host: windows # needs unix PermissionExt + +use std::fs::{self, File}; + +#[path = "../../utils/mod.rs"] +mod utils; + +macro_rules! check { + ($e:expr) => { + match $e { + Ok(t) => t, + Err(e) => panic!("{} failed with: {e}", stringify!($e)), + } + }; +} + +fn main() { + chmod_works(); + fchmod_works(); +} + +fn chmod_works() { + let tmpdir = utils::tmp(); + let file = tmpdir.join("miri_test_fs_set_permissions.txt"); + + check!(File::create(&file)); + let attr = check!(fs::metadata(&file)); + assert!(!attr.permissions().readonly()); + let mut p = attr.permissions(); + p.set_readonly(true); + check!(fs::set_permissions(&file, p.clone())); + let attr = check!(fs::metadata(&file)); + assert!(attr.permissions().readonly()); + + match fs::set_permissions(&tmpdir.join("foo"), p.clone()) { + Ok(..) => panic!("wanted an error"), + Err(..) => {} + } + + p.set_readonly(false); + check!(fs::set_permissions(&file, p)); +} + +fn fchmod_works() { + let tmpdir = utils::tmp(); + let path = tmpdir.join("miri_test_file_set_permissions.txt"); + + let file = check!(File::create(&path)); + let attr = check!(fs::metadata(&path)); + assert!(!attr.permissions().readonly()); + let mut p = attr.permissions(); + p.set_readonly(true); + check!(file.set_permissions(p.clone())); + let attr = check!(fs::metadata(&path)); + assert!(attr.permissions().readonly()); + + p.set_readonly(false); + check!(file.set_permissions(p)); +}
diff --git a/src/tools/miri/tests/pass/shims/fs.rs b/src/tools/miri/tests/pass/shims/fs.rs index e6c15c8..e0da9f6 100644 --- a/src/tools/miri/tests/pass/shims/fs.rs +++ b/src/tools/miri/tests/pass/shims/fs.rs
@@ -54,6 +54,7 @@ fn test_file() { // Test creating, writing and closing a file (closing is tested when `file` is dropped). let mut file = File::create(&path).unwrap(); + assert!(!file.metadata().unwrap().permissions().readonly()); // new file shouldn't be read-only // Writing 0 bytes should not change the file contents. file.write(&mut []).unwrap(); assert_eq!(file.metadata().unwrap().len(), 0);