Move shim argument checks before isolation check

This allows catching extremely incorrect arguments before rejecting
due to isolation.
This commit is contained in:
Smit Soni 2021-07-20 22:27:33 -07:00
parent da6880427a
commit 20d0f2ee26

View File

@ -304,28 +304,6 @@ fn insert_fd_with_min_fd(&mut self, file_handle: Box<dyn FileDescriptor>, min_fd
impl<'mir, 'tcx: 'mir> EvalContextExtPrivate<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
trait EvalContextExtPrivate<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
/// Emulate `stat` or `lstat` on `macos`. This function is not intended to be
/// called directly from `emulate_foreign_item_by_name`, so it does not check if isolation is
/// disabled or if the target OS is the correct one. Please use `macos_stat` or
/// `macos_lstat` instead.
fn macos_stat_or_lstat(
&mut self,
follow_symlink: bool,
path_op: &OpTy<'tcx, Tag>,
buf_op: &OpTy<'tcx, Tag>,
) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
let path_scalar = this.read_pointer(path_op)?;
let path = this.read_path_from_c_str(path_scalar)?.into_owned();
let metadata = match FileMetadata::from_path(this, &path, follow_symlink)? {
Some(metadata) => metadata,
None => return Ok(-1),
};
this.macos_stat_write_buf(metadata, buf_op)
}
fn macos_stat_write_buf(
&mut self,
metadata: FileMetadata,
@ -504,13 +482,6 @@ fn open(
) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
// 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()?;
// Get the mode. On macOS, the argument type `mode_t` is actually `u16`, but
@ -588,6 +559,13 @@ fn open(
let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?;
// 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 fd = options.open(&path).map(|file| {
let fh = &mut this.machine.file_handler;
fh.insert_fd(Box::new(FileHandle { file, writable }))
@ -599,13 +577,6 @@ fn open(
fn fcntl(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
// 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!(
"incorrect number of arguments for fcntl: got {}, expected at least 2",
@ -614,6 +585,14 @@ fn fcntl(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> {
}
let fd = this.read_scalar(&args[0])?.to_i32()?;
let cmd = this.read_scalar(&args[1])?.to_i32()?;
// 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);
}
// We only support getting the flags for a descriptor.
if cmd == this.eval_libc_i32("F_GETFD")? {
// Currently this is the only flag that `F_GETFD` returns. It is OK to just return the
@ -795,6 +774,8 @@ fn lseek64(
fn unlink(&mut self, path_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`unlink`", reject_with)?;
@ -802,8 +783,6 @@ fn unlink(&mut self, path_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
return Ok(-1);
}
let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?;
let result = remove_file(path).map(|_| 0);
this.try_unwrap_io_result(result)
}
@ -825,6 +804,8 @@ fn create_link(src: &Path, dst: &Path) -> std::io::Result<()> {
}
let this = self.eval_context_mut();
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)?)?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
@ -833,9 +814,6 @@ fn create_link(src: &Path, dst: &Path) -> std::io::Result<()> {
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)?)?;
let result = create_link(&target, &linkpath).map(|_| 0);
this.try_unwrap_io_result(result)
}
@ -848,6 +826,9 @@ fn macos_stat(
let this = self.eval_context_mut();
this.assert_target_os("macos", "stat");
let path_scalar = this.read_pointer(path_op)?;
let path = this.read_path_from_c_str(path_scalar)?.into_owned();
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`stat`", reject_with)?;
@ -857,7 +838,12 @@ fn macos_stat(
}
// `stat` always follows symlinks.
this.macos_stat_or_lstat(true, path_op, buf_op)
let metadata = match FileMetadata::from_path(this, &path, true)? {
Some(metadata) => metadata,
None => return Ok(-1),
};
this.macos_stat_write_buf(metadata, buf_op)
}
// `lstat` is used to get symlink metadata.
@ -869,6 +855,9 @@ fn macos_lstat(
let this = self.eval_context_mut();
this.assert_target_os("macos", "lstat");
let path_scalar = this.read_pointer(path_op)?;
let path = this.read_path_from_c_str(path_scalar)?.into_owned();
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`lstat`", reject_with)?;
@ -877,7 +866,12 @@ fn macos_lstat(
return Ok(-1);
}
this.macos_stat_or_lstat(false, path_op, buf_op)
let metadata = match FileMetadata::from_path(this, &path, false)? {
Some(metadata) => metadata,
None => return Ok(-1),
};
this.macos_stat_write_buf(metadata, buf_op)
}
fn macos_fstat(
@ -889,6 +883,8 @@ fn macos_fstat(
this.assert_target_os("macos", "fstat");
let fd = this.read_scalar(fd_op)?.to_i32()?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`fstat`", reject_with)?;
@ -896,8 +892,6 @@ fn macos_fstat(
return this.handle_not_found();
}
let fd = this.read_scalar(fd_op)?.to_i32()?;
let metadata = match FileMetadata::from_fd(this, fd)? {
Some(metadata) => metadata,
None => return Ok(-1),
@ -973,8 +967,10 @@ fn linux_statx(
// 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.
// `dirfd` is set to target file, and `path` is empty
// (or we would have hit the `throw_unsup_format`
// above). `EACCES` would violate the spec.
assert!(empty_path_flag);
this.eval_libc("EBADF")?
};
this.set_last_error(ecode)?;
@ -1089,13 +1085,6 @@ fn rename(
) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
// 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)?;
@ -1108,6 +1097,13 @@ fn rename(
let oldpath = this.read_path_from_c_str(oldpath_ptr)?;
let newpath = this.read_path_from_c_str(newpath_ptr)?;
// 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 result = rename(oldpath, newpath).map(|_| 0);
this.try_unwrap_io_result(result)
@ -1120,13 +1116,6 @@ fn mkdir(
) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
// 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" {
u32::from(this.read_scalar(mode_op)?.to_u16()?)
@ -1136,6 +1125,13 @@ fn mkdir(
let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?;
// 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_mut))]
let mut builder = DirBuilder::new();
@ -1155,6 +1151,8 @@ fn mkdir(
fn rmdir(&mut self, path_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`rmdir`", reject_with)?;
@ -1162,8 +1160,6 @@ fn rmdir(&mut self, path_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
return Ok(-1);
}
let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?;
let result = remove_dir(path).map(|_| 0i32);
this.try_unwrap_io_result(result)
@ -1172,6 +1168,8 @@ fn rmdir(&mut self, path_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
fn opendir(&mut self, name_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, Scalar<Tag>> {
let this = self.eval_context_mut();
let name = this.read_path_from_c_str(this.read_pointer(name_op)?)?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`opendir`", reject_with)?;
@ -1180,8 +1178,6 @@ fn opendir(&mut self, name_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, Scalar<Ta
return Ok(Scalar::null_ptr(this));
}
let name = this.read_path_from_c_str(this.read_pointer(name_op)?)?;
let result = read_dir(name);
match result {
@ -1210,6 +1206,8 @@ fn linux_readdir64_r(
this.assert_target_os("linux", "readdir64_r");
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`readdir64_r`", reject_with)?;
@ -1217,8 +1215,6 @@ fn linux_readdir64_r(
return this.handle_not_found();
}
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
let dir_iter = this.machine.dir_handler.streams.get_mut(&dirp).ok_or_else(|| {
err_unsup_format!("the DIR pointer passed to readdir64_r did not come from opendir")
})?;
@ -1309,6 +1305,8 @@ fn macos_readdir_r(
this.assert_target_os("macos", "readdir_r");
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`readdir_r`", reject_with)?;
@ -1316,8 +1314,6 @@ fn macos_readdir_r(
return this.handle_not_found();
}
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
let dir_iter = this.machine.dir_handler.streams.get_mut(&dirp).ok_or_else(|| {
err_unsup_format!("the DIR pointer passed to readdir_r did not come from opendir")
})?;
@ -1403,6 +1399,8 @@ fn macos_readdir_r(
fn closedir(&mut self, dirp_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`closedir`", reject_with)?;
@ -1410,8 +1408,6 @@ fn closedir(&mut self, dirp_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
return this.handle_not_found();
}
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
if let Some(dir_iter) = this.machine.dir_handler.streams.remove(&dirp) {
drop(dir_iter);
Ok(0)
@ -1427,6 +1423,9 @@ fn ftruncate64(
) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
let fd = this.read_scalar(fd_op)?.to_i32()?;
let length = this.read_scalar(length_op)?.to_i64()?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`ftruncate64`", reject_with)?;
@ -1434,8 +1433,6 @@ fn ftruncate64(
return this.handle_not_found();
}
let fd = this.read_scalar(fd_op)?.to_i32()?;
let length = this.read_scalar(length_op)?.to_i64()?;
if let Some(file_descriptor) = this.machine.file_handler.handles.get_mut(&fd) {
// FIXME: Support ftruncate64 for all FDs
let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
@ -1467,6 +1464,8 @@ fn fsync(&mut self, fd_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
let fd = this.read_scalar(fd_op)?.to_i32()?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`fsync`", reject_with)?;
@ -1474,7 +1473,6 @@ fn fsync(&mut self, fd_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
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) {
// FIXME: Support fsync for all FDs
let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
@ -1488,6 +1486,8 @@ fn fsync(&mut self, fd_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
fn fdatasync(&mut self, fd_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
let fd = this.read_scalar(fd_op)?.to_i32()?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`fdatasync`", reject_with)?;
@ -1495,7 +1495,6 @@ fn fdatasync(&mut self, fd_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
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) {
// FIXME: Support fdatasync for all FDs
let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
@ -1515,13 +1514,6 @@ fn sync_file_range(
) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
// 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()?;
let nbytes = this.read_scalar(nbytes_op)?.to_i64()?;
@ -1541,6 +1533,13 @@ fn sync_file_range(
return Ok(-1);
}
// 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();
}
if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
// FIXME: Support sync_data_range for all FDs
let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
@ -1559,6 +1558,10 @@ fn readlink(
) -> InterpResult<'tcx, i64> {
let this = self.eval_context_mut();
let pathname = this.read_path_from_c_str(this.read_pointer(pathname_op)?)?;
let buf = this.read_pointer(buf_op)?;
let bufsize = this.read_scalar(bufsize_op)?.to_machine_usize(this)?;
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`readlink`", reject_with)?;
@ -1567,10 +1570,6 @@ fn readlink(
return Ok(-1);
}
let pathname = this.read_path_from_c_str(this.read_pointer(pathname_op)?)?;
let buf = this.read_pointer(buf_op)?;
let bufsize = this.read_scalar(bufsize_op)?.to_machine_usize(this)?;
let result = std::fs::read_link(pathname);
match result {
Ok(resolved) => {