From 98f881507fce1ced7d7e7e5e57952bbc3471b999 Mon Sep 17 00:00:00 2001 From: Smit Soni Date: Sun, 18 Jul 2021 14:40:59 -0700 Subject: [PATCH] Update error code for fs ops in isolation Change the code to either `EACCES` (if the op is performed on the path), or `EBADF` (if the op is performed the fd) Updated ops: `stat`, `opendir`, `ftruncate64`, and `readlink` Add a new test for fs ops in isolation. --- src/shims/posix/fs.rs | 44 +++++++++++--------- tests/run-pass/fs_with_isolation.rs | 54 +++++++++++++++++++++++++ tests/run-pass/fs_with_isolation.stderr | 20 +++++++++ 3 files changed, 100 insertions(+), 18 deletions(-) create mode 100644 tests/run-pass/fs_with_isolation.rs create mode 100644 tests/run-pass/fs_with_isolation.stderr diff --git a/src/shims/posix/fs.rs b/src/shims/posix/fs.rs index 3653fc43eb..2693dc0962 100644 --- a/src/shims/posix/fs.rs +++ b/src/shims/posix/fs.rs @@ -851,8 +851,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Reject if isolation is enabled. if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`stat`", reject_with)?; - // macos stat never sets "EPERM". Set error code "ENOENT". - this.set_last_error_from_io_error(ErrorKind::NotFound)?; + let eacc = this.eval_libc("EACCES")?; + this.set_last_error(eacc)?; return Ok(-1); } @@ -872,8 +872,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Reject if isolation is enabled. if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`lstat`", reject_with)?; - // macos lstat never sets "EPERM". Set error code "ENOENT". - this.set_last_error_from_io_error(ErrorKind::NotFound)?; + let eacc = this.eval_libc("EACCES")?; + this.set_last_error(eacc)?; return Ok(-1); } @@ -917,14 +917,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.assert_target_os("linux", "statx"); - // Reject if isolation is enabled. - if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { - this.reject_in_isolation("`statx`", reject_with)?; - // statx never sets "EPERM". Set error code "ENOENT". - this.set_last_error_from_io_error(ErrorKind::NotFound)?; - return Ok(-1); - } - let statxbuf_ptr = this.read_pointer(statxbuf_op)?; let pathname_ptr = this.read_pointer(pathname_op)?; @@ -973,6 +965,22 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) } + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`statx`", reject_with)?; + let ecode = if path.is_absolute() || dirfd == this.eval_libc_i32("AT_FDCWD")? { + // since `path` is provided, either absolute or + // relative to CWD, `EACCES` is the most relevant. + this.eval_libc("EACCES")? + } else { + // `dirfd` is set to target file, and `path` is + // empty. `EACCES` would violate the spec. + this.eval_libc("EBADF")? + }; + this.set_last_error(ecode)?; + return Ok(-1); + } + // the `_mask_op` paramter specifies the file information that the caller requested. // However `statx` is allowed to return information that was not requested or to not // return information that was requested. This `mask` represents the information we can @@ -1167,8 +1175,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Reject if isolation is enabled. if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`opendir`", reject_with)?; - // opendir function never sets "EPERM". Set "ENOENT". - this.set_last_error_from_io_error(ErrorKind::NotFound)?; + let eacc = this.eval_libc("EACCES")?; + this.set_last_error(eacc)?; return Ok(Scalar::null_ptr(this)); } @@ -1422,8 +1430,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Reject if isolation is enabled. if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`ftruncate64`", reject_with)?; - this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; - return Ok(-1); + // Set error code as "EBADF" (bad fd) + return this.handle_not_found(); } let fd = this.read_scalar(fd_op)?.to_i32()?; @@ -1554,8 +1562,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Reject if isolation is enabled. if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`readlink`", reject_with)?; - // readlink never sets "EPERM". Set "ENOENT". - this.set_last_error_from_io_error(ErrorKind::NotFound)?; + let eacc = this.eval_libc("EACCES")?; + this.set_last_error(eacc)?; return Ok(-1); } diff --git a/tests/run-pass/fs_with_isolation.rs b/tests/run-pass/fs_with_isolation.rs new file mode 100644 index 0000000000..7df0f04e03 --- /dev/null +++ b/tests/run-pass/fs_with_isolation.rs @@ -0,0 +1,54 @@ +// ignore-windows: File handling is not implemented yet +// compile-flags: -Zmiri-isolation-error=warn-nobacktrace +// normalize-stderr-test "(stat|statx)" -> "$$STAT" + +#![feature(rustc_private)] + +extern crate libc; + +use std::ffi::CString; +use std::os::unix; +use std::fs::{self, File}; +use std::io::{Error, ErrorKind}; + +fn main() { + // test `open` + assert_eq!(File::create("foo.txt").unwrap_err().kind(), ErrorKind::PermissionDenied); + + // test `fcntl` + unsafe { + assert_eq!(libc::fcntl(1, libc::F_DUPFD, 0), -1); + assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EPERM)); + } + + // test `unlink` + assert_eq!(fs::remove_file("foo.txt").unwrap_err().kind(), ErrorKind::PermissionDenied); + + // test `symlink` + assert_eq!(unix::fs::symlink("foo.txt", "foo_link.txt").unwrap_err().kind(), ErrorKind::PermissionDenied); + + // test `readlink` + let symlink_c_str = CString::new("foo.txt").unwrap(); + let mut buf = vec![0; "foo_link.txt".len() + 1]; + unsafe { + assert_eq!(libc::readlink(symlink_c_str.as_ptr(), buf.as_mut_ptr(), buf.len()), -1); + assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EACCES)); + } + + // test `stat` + assert_eq!(fs::metadata("foo.txt").unwrap_err().kind(), ErrorKind::PermissionDenied); + assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EACCES)); + + // test `rename` + assert_eq!(fs::rename("a.txt", "b.txt").unwrap_err().kind(), ErrorKind::PermissionDenied); + + // test `mkdir` + assert_eq!(fs::create_dir("foo/bar").unwrap_err().kind(), ErrorKind::PermissionDenied); + + // test `rmdir` + assert_eq!(fs::remove_dir("foo/bar").unwrap_err().kind(), ErrorKind::PermissionDenied); + + // test `opendir` + assert_eq!(fs::read_dir("foo/bar").unwrap_err().kind(), ErrorKind::PermissionDenied); + assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EACCES)); +} diff --git a/tests/run-pass/fs_with_isolation.stderr b/tests/run-pass/fs_with_isolation.stderr new file mode 100644 index 0000000000..ad75e42831 --- /dev/null +++ b/tests/run-pass/fs_with_isolation.stderr @@ -0,0 +1,20 @@ +warning: `open` was made to return an error due to isolation + +warning: `fcntl` was made to return an error due to isolation + +warning: `unlink` was made to return an error due to isolation + +warning: `symlink` was made to return an error due to isolation + +warning: `readlink` was made to return an error due to isolation + +warning: `$STAT` was made to return an error due to isolation + +warning: `rename` was made to return an error due to isolation + +warning: `mkdir` was made to return an error due to isolation + +warning: `rmdir` was made to return an error due to isolation + +warning: `opendir` was made to return an error due to isolation +