]> git.lizzy.rs Git - rust.git/commitdiff
Move shim argument checks before isolation check
authorSmit Soni <atsmtat@gmail.com>
Wed, 21 Jul 2021 05:27:33 +0000 (22:27 -0700)
committerSmit Soni <atsmtat@gmail.com>
Sat, 24 Jul 2021 18:54:55 +0000 (11:54 -0700)
This allows catching extremely incorrect arguments before rejecting
due to isolation.

src/shims/posix/fs.rs

index 2693dc096286089667976b71f8bf113c4e3aaa82..ac6e878da962e83e686d0efcf1b383f3a83d0186 100644 (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) => {