Skip to content

Commit

Permalink
Update error code for fs ops in isolation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
atsmtat committed Jul 18, 2021
1 parent 9e6be84 commit 98f8815
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 18 deletions.
44 changes: 26 additions & 18 deletions src/shims/posix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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)?;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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()?;
Expand Down Expand Up @@ -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);
}

Expand Down
54 changes: 54 additions & 0 deletions tests/run-pass/fs_with_isolation.rs
Original file line number Diff line number Diff line change
@@ -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));
}
20 changes: 20 additions & 0 deletions tests/run-pass/fs_with_isolation.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 98f8815

Please sign in to comment.