From d29be1f90a3dd6f7964151ffc05b0c95088104ab Mon Sep 17 00:00:00 2001 From: tiif Date: Tue, 17 Sep 2024 17:40:20 +0800 Subject: [PATCH] Pass pointer and len to FileDescription::write and change the type of len in read to usize --- src/tools/miri/src/shims/unix/fd.rs | 46 +++++++++++-------- src/tools/miri/src/shims/unix/fs.rs | 20 ++++---- .../miri/src/shims/unix/linux/eventfd.rs | 23 +++++----- .../miri/src/shims/unix/unnamed_socket.rs | 18 ++++---- 4 files changed, 60 insertions(+), 47 deletions(-) diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index 4b87ce60c8b..f032eeab468 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -34,7 +34,7 @@ fn read<'tcx>( _self_ref: &FileDescriptionRef, _communicate_allowed: bool, _ptr: Pointer, - _len: u64, + _len: usize, _dest: &MPlaceTy<'tcx>, _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { @@ -42,13 +42,15 @@ fn read<'tcx>( } /// Writes as much as possible from the given buffer, and returns the number of bytes written. - /// `bytes` is the buffer of bytes supplied by the caller to be written. + /// `ptr` is the pointer to the user supplied read buffer. + /// `len` indicates how many bytes the user requested. /// `dest` is where the return value should be stored. fn write<'tcx>( &self, _self_ref: &FileDescriptionRef, _communicate_allowed: bool, - _bytes: &[u8], + _ptr: Pointer, + _len: usize, _dest: &MPlaceTy<'tcx>, _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { @@ -65,7 +67,7 @@ fn pread<'tcx>( _communicate_allowed: bool, _offset: u64, _ptr: Pointer, - _len: u64, + _len: usize, _dest: &MPlaceTy<'tcx>, _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { @@ -74,12 +76,14 @@ fn pread<'tcx>( /// Writes as much as possible from the given buffer starting at a given offset, /// and returns the number of bytes written. - /// `bytes` is the buffer of bytes supplied by the caller to be written. + /// `ptr` is the pointer to the user supplied read buffer. + /// `len` indicates how many bytes the user requested. /// `dest` is where the return value should be stored. fn pwrite<'tcx>( &self, _communicate_allowed: bool, - _bytes: &[u8], + _ptr: Pointer, + _len: usize, _offset: u64, _dest: &MPlaceTy<'tcx>, _ecx: &mut MiriInterpCx<'tcx>, @@ -142,11 +146,11 @@ fn read<'tcx>( _self_ref: &FileDescriptionRef, communicate_allowed: bool, ptr: Pointer, - len: u64, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { - let mut bytes = vec![0; usize::try_from(len).unwrap()]; + let mut bytes = vec![0; len]; if !communicate_allowed { // We want isolation mode to be deterministic, so we have to disallow all reads, even stdin. helpers::isolation_abort_error("`read` from stdin")?; @@ -169,12 +173,14 @@ fn write<'tcx>( &self, _self_ref: &FileDescriptionRef, _communicate_allowed: bool, - bytes: &[u8], + ptr: Pointer, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { + let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?.to_owned(); // We allow writing to stderr even with isolation enabled. - let result = Write::write(&mut { self }, bytes); + let result = Write::write(&mut { self }, &bytes); // Stdout is buffered, flush to make sure it appears on the // screen. This is the write() syscall of the interpreted // program, we want it to correspond to a write() syscall on @@ -198,13 +204,15 @@ fn write<'tcx>( &self, _self_ref: &FileDescriptionRef, _communicate_allowed: bool, - bytes: &[u8], + ptr: Pointer, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { + let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?.to_owned(); // We allow writing to stderr even with isolation enabled. // No need to flush, stderr is not buffered. - let result = Write::write(&mut { self }, bytes); + let result = Write::write(&mut { self }, &bytes); ecx.return_written_byte_count_or_error(result, dest) } @@ -226,12 +234,13 @@ fn write<'tcx>( &self, _self_ref: &FileDescriptionRef, _communicate_allowed: bool, - bytes: &[u8], + _ptr: Pointer, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { // We just don't write anything, but report to the user that we did. - let result = Ok(bytes.len()); + let result = Ok(len); ecx.return_written_byte_count_or_error(result, dest) } } @@ -591,7 +600,7 @@ fn read( // `usize::MAX` because it is bounded by the host's `isize`. match offset { - None => fd.read(&fd, communicate, buf, count, dest, this)?, + None => fd.read(&fd, communicate, buf, usize::try_from(count).unwrap(), dest, this)?, Some(offset) => { let Ok(offset) = u64::try_from(offset) else { let einval = this.eval_libc("EINVAL"); @@ -599,7 +608,7 @@ fn read( this.write_int(-1, dest)?; return Ok(()); }; - fd.pread(communicate, offset, buf, count, dest, this)? + fd.pread(communicate, offset, buf, usize::try_from(count).unwrap(), dest, this)? } }; Ok(()) @@ -627,7 +636,6 @@ fn write( .min(u64::try_from(isize::MAX).unwrap()); let communicate = this.machine.communicate(); - let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?.to_owned(); // We temporarily dup the FD to be able to retain mutable access to `this`. let Some(fd) = this.machine.fds.get(fd_num) else { let res: i32 = this.fd_not_found()?; @@ -636,7 +644,7 @@ fn write( }; match offset { - None => fd.write(&fd, communicate, &bytes, dest, this)?, + None => fd.write(&fd, communicate, buf, usize::try_from(count).unwrap(), dest, this)?, Some(offset) => { let Ok(offset) = u64::try_from(offset) else { let einval = this.eval_libc("EINVAL"); @@ -644,7 +652,7 @@ fn write( this.write_int(-1, dest)?; return Ok(()); }; - fd.pwrite(communicate, &bytes, offset, dest, this)? + fd.pwrite(communicate, buf, usize::try_from(count).unwrap(), offset, dest, this)? } }; Ok(()) diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index e1ac9b0adfb..0e80a45f48d 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -35,12 +35,12 @@ fn read<'tcx>( _self_ref: &FileDescriptionRef, communicate_allowed: bool, ptr: Pointer, - len: u64, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); - let mut bytes = vec![0; usize::try_from(len).unwrap()]; + let mut bytes = vec![0; len]; let result = (&mut &self.file).read(&mut bytes); ecx.return_read_bytes_and_count(ptr, &bytes, result, dest) } @@ -49,12 +49,14 @@ fn write<'tcx>( &self, _self_ref: &FileDescriptionRef, communicate_allowed: bool, - bytes: &[u8], + ptr: Pointer, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); - let result = (&mut &self.file).write(bytes); + let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?.to_owned(); + let result = (&mut &self.file).write(&bytes); ecx.return_written_byte_count_or_error(result, dest) } @@ -63,12 +65,12 @@ fn pread<'tcx>( communicate_allowed: bool, offset: u64, ptr: Pointer, - len: u64, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); - let mut bytes = vec![0; usize::try_from(len).unwrap()]; + let mut bytes = vec![0; len]; // Emulates pread using seek + read + seek to restore cursor position. // Correctness of this emulation relies on sequential nature of Miri execution. // The closure is used to emulate `try` block, since we "bubble" `io::Error` using `?`. @@ -89,7 +91,8 @@ fn pread<'tcx>( fn pwrite<'tcx>( &self, communicate_allowed: bool, - bytes: &[u8], + ptr: Pointer, + len: usize, offset: u64, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, @@ -99,10 +102,11 @@ fn pwrite<'tcx>( // Correctness of this emulation relies on sequential nature of Miri execution. // The closure is used to emulate `try` block, since we "bubble" `io::Error` using `?`. let file = &mut &self.file; + let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?.to_owned(); let mut f = || { let cursor_pos = file.stream_position()?; file.seek(SeekFrom::Start(offset))?; - let res = file.write(bytes); + let res = file.write(&bytes); // Attempt to restore cursor position even if the write has failed file.seek(SeekFrom::Start(cursor_pos)) .expect("failed to restore file position, this shouldn't be possible"); diff --git a/src/tools/miri/src/shims/unix/linux/eventfd.rs b/src/tools/miri/src/shims/unix/linux/eventfd.rs index 2375533fd17..e1531fc2fa8 100644 --- a/src/tools/miri/src/shims/unix/linux/eventfd.rs +++ b/src/tools/miri/src/shims/unix/linux/eventfd.rs @@ -4,8 +4,6 @@ use std::io::{Error, ErrorKind}; use std::mem; -use rustc_target::abi::Endian; - use crate::shims::unix::fd::FileDescriptionRef; use crate::shims::unix::linux::epoll::{EpollReadyEvents, EvalContextExt as _}; use crate::shims::unix::*; @@ -63,14 +61,14 @@ fn read<'tcx>( self_ref: &FileDescriptionRef, _communicate_allowed: bool, ptr: Pointer, - len: u64, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { // eventfd read at the size of u64. let buf_place = ecx.ptr_to_mplace_unaligned(ptr, ecx.machine.layouts.u64); // Check the size of slice, and return error only if the size of the slice < 8. - if len < U64_ARRAY_SIZE.try_into().unwrap() { + if len < U64_ARRAY_SIZE { let result = Err(Error::from(ErrorKind::InvalidInput)); return return_read_bytes_and_count_ev(&buf_place, None, result, dest, ecx); } @@ -114,20 +112,21 @@ fn write<'tcx>( &self, self_ref: &FileDescriptionRef, _communicate_allowed: bool, - bytes: &[u8], + ptr: Pointer, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { // Check the size of slice, and return error only if the size of the slice < 8. - let Some(bytes) = bytes.first_chunk::() else { + if len < U64_ARRAY_SIZE { let result = Err(Error::from(ErrorKind::InvalidInput)); return ecx.return_written_byte_count_or_error(result, dest); - }; - // Convert from bytes to int according to host endianness. - let num = match ecx.tcx.sess.target.endian { - Endian::Little => u64::from_le_bytes(*bytes), - Endian::Big => u64::from_be_bytes(*bytes), - }; + } + + // Read the user supplied value from the pointer. + let buf_place = ecx.ptr_to_mplace_unaligned(ptr, ecx.machine.layouts.u64); + let num = ecx.read_scalar(&buf_place)?.to_u64()?; + // u64::MAX as input is invalid because the maximum value of counter is u64::MAX - 1. if num == u64::MAX { let result = Err(Error::from(ErrorKind::InvalidInput)); diff --git a/src/tools/miri/src/shims/unix/unnamed_socket.rs b/src/tools/miri/src/shims/unix/unnamed_socket.rs index d5b061ea3cd..2cb9bb9b2dc 100644 --- a/src/tools/miri/src/shims/unix/unnamed_socket.rs +++ b/src/tools/miri/src/shims/unix/unnamed_socket.rs @@ -7,6 +7,8 @@ use std::io; use std::io::{Error, ErrorKind, Read}; +use rustc_target::abi::Size; + use crate::shims::unix::fd::{FileDescriptionRef, WeakFileDescriptionRef}; use crate::shims::unix::linux::epoll::{EpollReadyEvents, EvalContextExt as _}; use crate::shims::unix::*; @@ -127,15 +129,14 @@ fn read<'tcx>( _self_ref: &FileDescriptionRef, _communicate_allowed: bool, ptr: Pointer, - len: u64, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { - let request_byte_size = len; - let mut bytes = vec![0; usize::try_from(len).unwrap()]; + let mut bytes = vec![0; len]; // Always succeed on read size 0. - if request_byte_size == 0 { + if len == 0 { let result = Ok(0); return ecx.return_read_bytes_and_count(ptr, &bytes, result, dest); } @@ -200,14 +201,14 @@ fn write<'tcx>( &self, _self_ref: &FileDescriptionRef, _communicate_allowed: bool, - bytes: &[u8], + ptr: Pointer, + len: usize, dest: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { - let write_size = bytes.len(); // Always succeed on write size 0. // ("If count is zero and fd refers to a file other than a regular file, the results are not specified.") - if write_size == 0 { + if len == 0 { let result = Ok(0); return ecx.return_written_byte_count_or_error(result, dest); } @@ -243,7 +244,8 @@ fn write<'tcx>( writebuf.clock.join(clock); } // Do full write / partial write based on the space available. - let actual_write_size = write_size.min(available_space); + let actual_write_size = len.min(available_space); + let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?.to_owned(); writebuf.buf.extend(&bytes[..actual_write_size]); // Need to stop accessing peer_fd so that it can be notified.