From a1cabac72707020e30a787f44740b855583fc341 Mon Sep 17 00:00:00 2001 From: Smit Soni Date: Tue, 15 Jun 2021 17:33:18 -0700 Subject: [PATCH] Fix use of deprecated `check_no_isolation` in posix fs ops Update posix fs shims to use new API `reject_in_isolation`, which allows rejection with error code instead of always forcing abort. Error code chosen for each op is the most appropriate one from the list in corresponding syscall's manual. Updated helper APIs to not use quotes (`) around input name while preparing the message. This allows callers to pass multi-word string like -- "`read` from stdin". --- src/diagnostics.rs | 2 +- src/helpers.rs | 2 +- src/shims/env.rs | 8 +-- src/shims/posix/fs.rs | 155 ++++++++++++++++++++++++++++++++++++------ 4 files changed, 140 insertions(+), 27 deletions(-) diff --git a/src/diagnostics.rs b/src/diagnostics.rs index b5b75a7fc318..f66f66e0088f 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -328,7 +328,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx CreatedAlloc(AllocId(id)) => format!("created allocation with id {}", id), FreedAlloc(AllocId(id)) => format!("freed allocation with id {}", id), RejectedIsolatedOp(ref op) => - format!("`{}` was made to return an error due to isolation", op), + format!("{} was made to return an error due to isolation", op), }; let (title, diag_level) = match e { diff --git a/src/helpers.rs b/src/helpers.rs index 363aefa62d2f..e09ef2865fb2 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -409,7 +409,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx RejectOpWith::WarningWithoutBacktrace => { this.tcx .sess - .warn(&format!("`{}` was made to return an error due to isolation", op_name)); + .warn(&format!("{} was made to return an error due to isolation", op_name)); Ok(()) } RejectOpWith::Warning => { diff --git a/src/shims/env.rs b/src/shims/env.rs index ddd2b6158898..9290ec022b57 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -322,7 +322,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let size = this.read_scalar(&size_op)?.to_machine_usize(&*this.tcx)?; if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { - this.reject_in_isolation("getcwd", reject_with)?; + this.reject_in_isolation("`getcwd`", reject_with)?; this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; return Ok(Pointer::null()); } @@ -355,7 +355,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let buf = this.read_pointer(buf_op)?; if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { - this.reject_in_isolation("GetCurrentDirectoryW", reject_with)?; + this.reject_in_isolation("`GetCurrentDirectoryW`", reject_with)?; this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; return Ok(0); } @@ -380,7 +380,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?; if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { - this.reject_in_isolation("chdir", reject_with)?; + this.reject_in_isolation("`chdir`", reject_with)?; this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; return Ok(-1); @@ -408,7 +408,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let path = this.read_path_from_wide_str(this.read_pointer(path_op)?)?; if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { - this.reject_in_isolation("SetCurrentDirectoryW", reject_with)?; + this.reject_in_isolation("`SetCurrentDirectoryW`", reject_with)?; this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; return Ok(0); diff --git a/src/shims/posix/fs.rs b/src/shims/posix/fs.rs index a14a9c907eed..3653fc43ebb1 100644 --- a/src/shims/posix/fs.rs +++ b/src/shims/posix/fs.rs @@ -4,7 +4,7 @@ use std::convert::{TryFrom, TryInto}; use std::fs::{ read_dir, remove_dir, remove_file, rename, DirBuilder, File, FileType, OpenOptions, ReadDir, }; -use std::io::{self, Read, Seek, SeekFrom, Write}; +use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write}; use std::path::Path; use std::time::SystemTime; @@ -504,7 +504,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - this.check_no_isolation("`open`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`open`", reject_with)?; + this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; + return Ok(-1); + } let flag = this.read_scalar(flag_op)?.to_i32()?; @@ -594,7 +599,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn fcntl(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - this.check_no_isolation("`fcntl`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`fcntl`", reject_with)?; + this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; + return Ok(-1); + } if args.len() < 2 { throw_ub_format!( @@ -785,7 +795,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn unlink(&mut self, path_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - this.check_no_isolation("`unlink`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`unlink`", reject_with)?; + this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; + return Ok(-1); + } let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?; @@ -811,7 +826,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let this = self.eval_context_mut(); - this.check_no_isolation("`symlink`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`symlink`", reject_with)?; + this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; + return Ok(-1); + } let target = this.read_path_from_c_str(this.read_pointer(target_op)?)?; let linkpath = this.read_path_from_c_str(this.read_pointer(linkpath_op)?)?; @@ -827,7 +847,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); this.assert_target_os("macos", "stat"); - this.check_no_isolation("`stat`")?; + + // 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)?; + return Ok(-1); + } + // `stat` always follows symlinks. this.macos_stat_or_lstat(true, path_op, buf_op) } @@ -840,7 +868,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); this.assert_target_os("macos", "lstat"); - this.check_no_isolation("`lstat`")?; + + // 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)?; + return Ok(-1); + } + this.macos_stat_or_lstat(false, path_op, buf_op) } @@ -852,7 +888,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let this = self.eval_context_mut(); this.assert_target_os("macos", "fstat"); - this.check_no_isolation("`fstat`")?; + + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`fstat`", reject_with)?; + // Set error code as "EBADF" (bad fd) + return this.handle_not_found(); + } let fd = this.read_scalar(fd_op)?.to_i32()?; @@ -874,7 +916,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let this = self.eval_context_mut(); this.assert_target_os("linux", "statx"); - this.check_no_isolation("`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)?; @@ -1032,7 +1081,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - this.check_no_isolation("`rename`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`rename`", reject_with)?; + this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; + return Ok(-1); + } let oldpath_ptr = this.read_pointer(oldpath_op)?; let newpath_ptr = this.read_pointer(newpath_op)?; @@ -1058,7 +1112,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - this.check_no_isolation("`mkdir`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`mkdir`", reject_with)?; + this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; + return Ok(-1); + } #[cfg_attr(not(unix), allow(unused_variables))] let mode = if this.tcx.sess.target.os == "macos" { @@ -1088,7 +1147,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn rmdir(&mut self, path_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - this.check_no_isolation("`rmdir`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`rmdir`", reject_with)?; + this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; + return Ok(-1); + } let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?; @@ -1100,7 +1164,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn opendir(&mut self, name_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - this.check_no_isolation("`opendir`")?; + // 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)?; + return Ok(Scalar::null_ptr(this)); + } let name = this.read_path_from_c_str(this.read_pointer(name_op)?)?; @@ -1131,7 +1201,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let this = self.eval_context_mut(); this.assert_target_os("linux", "readdir64_r"); - this.check_no_isolation("`readdir64_r`")?; + + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`readdir64_r`", reject_with)?; + // Set error code as "EBADF" (bad fd) + return this.handle_not_found(); + } let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?; @@ -1224,7 +1300,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let this = self.eval_context_mut(); this.assert_target_os("macos", "readdir_r"); - this.check_no_isolation("`readdir_r`")?; + + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`readdir_r`", reject_with)?; + // Set error code as "EBADF" (bad fd) + return this.handle_not_found(); + } let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?; @@ -1313,7 +1395,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn closedir(&mut self, dirp_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - this.check_no_isolation("`closedir`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`closedir`", reject_with)?; + // Set error code as "EBADF" (bad fd) + return this.handle_not_found(); + } let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?; @@ -1332,7 +1419,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - this.check_no_isolation("`ftruncate64`")?; + // 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); + } let fd = this.read_scalar(fd_op)?.to_i32()?; let length = this.read_scalar(length_op)?.to_i64()?; @@ -1367,7 +1459,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let this = self.eval_context_mut(); - this.check_no_isolation("`fsync`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`fsync`", reject_with)?; + // Set error code as "EBADF" (bad fd) + return this.handle_not_found(); + } let fd = this.read_scalar(fd_op)?.to_i32()?; if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { @@ -1383,7 +1480,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn fdatasync(&mut self, fd_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - this.check_no_isolation("`fdatasync`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`fdatasync`", reject_with)?; + // Set error code as "EBADF" (bad fd) + return this.handle_not_found(); + } let fd = this.read_scalar(fd_op)?.to_i32()?; if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { @@ -1405,7 +1507,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - this.check_no_isolation("`sync_file_range`")?; + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`sync_file_range`", reject_with)?; + // Set error code as "EBADF" (bad fd) + return this.handle_not_found(); + } let fd = this.read_scalar(fd_op)?.to_i32()?; let offset = this.read_scalar(offset_op)?.to_i64()?; @@ -1444,7 +1551,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx, i64> { let this = self.eval_context_mut(); - this.check_no_isolation("readlink")?; + // 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)?; + return Ok(-1); + } let pathname = this.read_path_from_c_str(this.read_pointer(pathname_op)?)?; let buf = this.read_pointer(buf_op)?;