Skip to content

Commit d29be1f

Browse files
tiifRalfJung
authored andcommitted
Pass pointer and len to FileDescription::write and change the type of len in read to usize
1 parent 503b6af commit d29be1f

File tree

4 files changed

+60
-47
lines changed

4 files changed

+60
-47
lines changed

src/tools/miri/src/shims/unix/fd.rs

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,21 +34,23 @@ pub trait FileDescription: std::fmt::Debug + Any {
3434
_self_ref: &FileDescriptionRef,
3535
_communicate_allowed: bool,
3636
_ptr: Pointer,
37-
_len: u64,
37+
_len: usize,
3838
_dest: &MPlaceTy<'tcx>,
3939
_ecx: &mut MiriInterpCx<'tcx>,
4040
) -> InterpResult<'tcx> {
4141
throw_unsup_format!("cannot read from {}", self.name());
4242
}
4343

4444
/// Writes as much as possible from the given buffer, and returns the number of bytes written.
45-
/// `bytes` is the buffer of bytes supplied by the caller to be written.
45+
/// `ptr` is the pointer to the user supplied read buffer.
46+
/// `len` indicates how many bytes the user requested.
4647
/// `dest` is where the return value should be stored.
4748
fn write<'tcx>(
4849
&self,
4950
_self_ref: &FileDescriptionRef,
5051
_communicate_allowed: bool,
51-
_bytes: &[u8],
52+
_ptr: Pointer,
53+
_len: usize,
5254
_dest: &MPlaceTy<'tcx>,
5355
_ecx: &mut MiriInterpCx<'tcx>,
5456
) -> InterpResult<'tcx> {
@@ -65,7 +67,7 @@ pub trait FileDescription: std::fmt::Debug + Any {
6567
_communicate_allowed: bool,
6668
_offset: u64,
6769
_ptr: Pointer,
68-
_len: u64,
70+
_len: usize,
6971
_dest: &MPlaceTy<'tcx>,
7072
_ecx: &mut MiriInterpCx<'tcx>,
7173
) -> InterpResult<'tcx> {
@@ -74,12 +76,14 @@ pub trait FileDescription: std::fmt::Debug + Any {
7476

7577
/// Writes as much as possible from the given buffer starting at a given offset,
7678
/// and returns the number of bytes written.
77-
/// `bytes` is the buffer of bytes supplied by the caller to be written.
79+
/// `ptr` is the pointer to the user supplied read buffer.
80+
/// `len` indicates how many bytes the user requested.
7881
/// `dest` is where the return value should be stored.
7982
fn pwrite<'tcx>(
8083
&self,
8184
_communicate_allowed: bool,
82-
_bytes: &[u8],
85+
_ptr: Pointer,
86+
_len: usize,
8387
_offset: u64,
8488
_dest: &MPlaceTy<'tcx>,
8589
_ecx: &mut MiriInterpCx<'tcx>,
@@ -142,11 +146,11 @@ impl FileDescription for io::Stdin {
142146
_self_ref: &FileDescriptionRef,
143147
communicate_allowed: bool,
144148
ptr: Pointer,
145-
len: u64,
149+
len: usize,
146150
dest: &MPlaceTy<'tcx>,
147151
ecx: &mut MiriInterpCx<'tcx>,
148152
) -> InterpResult<'tcx> {
149-
let mut bytes = vec![0; usize::try_from(len).unwrap()];
153+
let mut bytes = vec![0; len];
150154
if !communicate_allowed {
151155
// We want isolation mode to be deterministic, so we have to disallow all reads, even stdin.
152156
helpers::isolation_abort_error("`read` from stdin")?;
@@ -169,12 +173,14 @@ impl FileDescription for io::Stdout {
169173
&self,
170174
_self_ref: &FileDescriptionRef,
171175
_communicate_allowed: bool,
172-
bytes: &[u8],
176+
ptr: Pointer,
177+
len: usize,
173178
dest: &MPlaceTy<'tcx>,
174179
ecx: &mut MiriInterpCx<'tcx>,
175180
) -> InterpResult<'tcx> {
181+
let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?.to_owned();
176182
// We allow writing to stderr even with isolation enabled.
177-
let result = Write::write(&mut { self }, bytes);
183+
let result = Write::write(&mut { self }, &bytes);
178184
// Stdout is buffered, flush to make sure it appears on the
179185
// screen. This is the write() syscall of the interpreted
180186
// program, we want it to correspond to a write() syscall on
@@ -198,13 +204,15 @@ impl FileDescription for io::Stderr {
198204
&self,
199205
_self_ref: &FileDescriptionRef,
200206
_communicate_allowed: bool,
201-
bytes: &[u8],
207+
ptr: Pointer,
208+
len: usize,
202209
dest: &MPlaceTy<'tcx>,
203210
ecx: &mut MiriInterpCx<'tcx>,
204211
) -> InterpResult<'tcx> {
212+
let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?.to_owned();
205213
// We allow writing to stderr even with isolation enabled.
206214
// No need to flush, stderr is not buffered.
207-
let result = Write::write(&mut { self }, bytes);
215+
let result = Write::write(&mut { self }, &bytes);
208216
ecx.return_written_byte_count_or_error(result, dest)
209217
}
210218

@@ -226,12 +234,13 @@ impl FileDescription for NullOutput {
226234
&self,
227235
_self_ref: &FileDescriptionRef,
228236
_communicate_allowed: bool,
229-
bytes: &[u8],
237+
_ptr: Pointer,
238+
len: usize,
230239
dest: &MPlaceTy<'tcx>,
231240
ecx: &mut MiriInterpCx<'tcx>,
232241
) -> InterpResult<'tcx> {
233242
// We just don't write anything, but report to the user that we did.
234-
let result = Ok(bytes.len());
243+
let result = Ok(len);
235244
ecx.return_written_byte_count_or_error(result, dest)
236245
}
237246
}
@@ -591,15 +600,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
591600
// `usize::MAX` because it is bounded by the host's `isize`.
592601

593602
match offset {
594-
None => fd.read(&fd, communicate, buf, count, dest, this)?,
603+
None => fd.read(&fd, communicate, buf, usize::try_from(count).unwrap(), dest, this)?,
595604
Some(offset) => {
596605
let Ok(offset) = u64::try_from(offset) else {
597606
let einval = this.eval_libc("EINVAL");
598607
this.set_last_error(einval)?;
599608
this.write_int(-1, dest)?;
600609
return Ok(());
601610
};
602-
fd.pread(communicate, offset, buf, count, dest, this)?
611+
fd.pread(communicate, offset, buf, usize::try_from(count).unwrap(), dest, this)?
603612
}
604613
};
605614
Ok(())
@@ -627,7 +636,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
627636
.min(u64::try_from(isize::MAX).unwrap());
628637
let communicate = this.machine.communicate();
629638

630-
let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?.to_owned();
631639
// We temporarily dup the FD to be able to retain mutable access to `this`.
632640
let Some(fd) = this.machine.fds.get(fd_num) else {
633641
let res: i32 = this.fd_not_found()?;
@@ -636,15 +644,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
636644
};
637645

638646
match offset {
639-
None => fd.write(&fd, communicate, &bytes, dest, this)?,
647+
None => fd.write(&fd, communicate, buf, usize::try_from(count).unwrap(), dest, this)?,
640648
Some(offset) => {
641649
let Ok(offset) = u64::try_from(offset) else {
642650
let einval = this.eval_libc("EINVAL");
643651
this.set_last_error(einval)?;
644652
this.write_int(-1, dest)?;
645653
return Ok(());
646654
};
647-
fd.pwrite(communicate, &bytes, offset, dest, this)?
655+
fd.pwrite(communicate, buf, usize::try_from(count).unwrap(), offset, dest, this)?
648656
}
649657
};
650658
Ok(())

src/tools/miri/src/shims/unix/fs.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@ impl FileDescription for FileHandle {
3535
_self_ref: &FileDescriptionRef,
3636
communicate_allowed: bool,
3737
ptr: Pointer,
38-
len: u64,
38+
len: usize,
3939
dest: &MPlaceTy<'tcx>,
4040
ecx: &mut MiriInterpCx<'tcx>,
4141
) -> InterpResult<'tcx> {
4242
assert!(communicate_allowed, "isolation should have prevented even opening a file");
43-
let mut bytes = vec![0; usize::try_from(len).unwrap()];
43+
let mut bytes = vec![0; len];
4444
let result = (&mut &self.file).read(&mut bytes);
4545
ecx.return_read_bytes_and_count(ptr, &bytes, result, dest)
4646
}
@@ -49,12 +49,14 @@ impl FileDescription for FileHandle {
4949
&self,
5050
_self_ref: &FileDescriptionRef,
5151
communicate_allowed: bool,
52-
bytes: &[u8],
52+
ptr: Pointer,
53+
len: usize,
5354
dest: &MPlaceTy<'tcx>,
5455
ecx: &mut MiriInterpCx<'tcx>,
5556
) -> InterpResult<'tcx> {
5657
assert!(communicate_allowed, "isolation should have prevented even opening a file");
57-
let result = (&mut &self.file).write(bytes);
58+
let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?.to_owned();
59+
let result = (&mut &self.file).write(&bytes);
5860
ecx.return_written_byte_count_or_error(result, dest)
5961
}
6062

@@ -63,12 +65,12 @@ impl FileDescription for FileHandle {
6365
communicate_allowed: bool,
6466
offset: u64,
6567
ptr: Pointer,
66-
len: u64,
68+
len: usize,
6769
dest: &MPlaceTy<'tcx>,
6870
ecx: &mut MiriInterpCx<'tcx>,
6971
) -> InterpResult<'tcx> {
7072
assert!(communicate_allowed, "isolation should have prevented even opening a file");
71-
let mut bytes = vec![0; usize::try_from(len).unwrap()];
73+
let mut bytes = vec![0; len];
7274
// Emulates pread using seek + read + seek to restore cursor position.
7375
// Correctness of this emulation relies on sequential nature of Miri execution.
7476
// The closure is used to emulate `try` block, since we "bubble" `io::Error` using `?`.
@@ -89,7 +91,8 @@ impl FileDescription for FileHandle {
8991
fn pwrite<'tcx>(
9092
&self,
9193
communicate_allowed: bool,
92-
bytes: &[u8],
94+
ptr: Pointer,
95+
len: usize,
9396
offset: u64,
9497
dest: &MPlaceTy<'tcx>,
9598
ecx: &mut MiriInterpCx<'tcx>,
@@ -99,10 +102,11 @@ impl FileDescription for FileHandle {
99102
// Correctness of this emulation relies on sequential nature of Miri execution.
100103
// The closure is used to emulate `try` block, since we "bubble" `io::Error` using `?`.
101104
let file = &mut &self.file;
105+
let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?.to_owned();
102106
let mut f = || {
103107
let cursor_pos = file.stream_position()?;
104108
file.seek(SeekFrom::Start(offset))?;
105-
let res = file.write(bytes);
109+
let res = file.write(&bytes);
106110
// Attempt to restore cursor position even if the write has failed
107111
file.seek(SeekFrom::Start(cursor_pos))
108112
.expect("failed to restore file position, this shouldn't be possible");

src/tools/miri/src/shims/unix/linux/eventfd.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ use std::io;
44
use std::io::{Error, ErrorKind};
55
use std::mem;
66

7-
use rustc_target::abi::Endian;
8-
97
use crate::shims::unix::fd::FileDescriptionRef;
108
use crate::shims::unix::linux::epoll::{EpollReadyEvents, EvalContextExt as _};
119
use crate::shims::unix::*;
@@ -63,14 +61,14 @@ impl FileDescription for Event {
6361
self_ref: &FileDescriptionRef,
6462
_communicate_allowed: bool,
6563
ptr: Pointer,
66-
len: u64,
64+
len: usize,
6765
dest: &MPlaceTy<'tcx>,
6866
ecx: &mut MiriInterpCx<'tcx>,
6967
) -> InterpResult<'tcx> {
7068
// eventfd read at the size of u64.
7169
let buf_place = ecx.ptr_to_mplace_unaligned(ptr, ecx.machine.layouts.u64);
7270
// Check the size of slice, and return error only if the size of the slice < 8.
73-
if len < U64_ARRAY_SIZE.try_into().unwrap() {
71+
if len < U64_ARRAY_SIZE {
7472
let result = Err(Error::from(ErrorKind::InvalidInput));
7573
return return_read_bytes_and_count_ev(&buf_place, None, result, dest, ecx);
7674
}
@@ -114,20 +112,21 @@ impl FileDescription for Event {
114112
&self,
115113
self_ref: &FileDescriptionRef,
116114
_communicate_allowed: bool,
117-
bytes: &[u8],
115+
ptr: Pointer,
116+
len: usize,
118117
dest: &MPlaceTy<'tcx>,
119118
ecx: &mut MiriInterpCx<'tcx>,
120119
) -> InterpResult<'tcx> {
121120
// Check the size of slice, and return error only if the size of the slice < 8.
122-
let Some(bytes) = bytes.first_chunk::<U64_ARRAY_SIZE>() else {
121+
if len < U64_ARRAY_SIZE {
123122
let result = Err(Error::from(ErrorKind::InvalidInput));
124123
return ecx.return_written_byte_count_or_error(result, dest);
125-
};
126-
// Convert from bytes to int according to host endianness.
127-
let num = match ecx.tcx.sess.target.endian {
128-
Endian::Little => u64::from_le_bytes(*bytes),
129-
Endian::Big => u64::from_be_bytes(*bytes),
130-
};
124+
}
125+
126+
// Read the user supplied value from the pointer.
127+
let buf_place = ecx.ptr_to_mplace_unaligned(ptr, ecx.machine.layouts.u64);
128+
let num = ecx.read_scalar(&buf_place)?.to_u64()?;
129+
131130
// u64::MAX as input is invalid because the maximum value of counter is u64::MAX - 1.
132131
if num == u64::MAX {
133132
let result = Err(Error::from(ErrorKind::InvalidInput));

src/tools/miri/src/shims/unix/unnamed_socket.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ use std::collections::VecDeque;
77
use std::io;
88
use std::io::{Error, ErrorKind, Read};
99

10+
use rustc_target::abi::Size;
11+
1012
use crate::shims::unix::fd::{FileDescriptionRef, WeakFileDescriptionRef};
1113
use crate::shims::unix::linux::epoll::{EpollReadyEvents, EvalContextExt as _};
1214
use crate::shims::unix::*;
@@ -127,15 +129,14 @@ impl FileDescription for AnonSocket {
127129
_self_ref: &FileDescriptionRef,
128130
_communicate_allowed: bool,
129131
ptr: Pointer,
130-
len: u64,
132+
len: usize,
131133
dest: &MPlaceTy<'tcx>,
132134
ecx: &mut MiriInterpCx<'tcx>,
133135
) -> InterpResult<'tcx> {
134-
let request_byte_size = len;
135-
let mut bytes = vec![0; usize::try_from(len).unwrap()];
136+
let mut bytes = vec![0; len];
136137

137138
// Always succeed on read size 0.
138-
if request_byte_size == 0 {
139+
if len == 0 {
139140
let result = Ok(0);
140141
return ecx.return_read_bytes_and_count(ptr, &bytes, result, dest);
141142
}
@@ -200,14 +201,14 @@ impl FileDescription for AnonSocket {
200201
&self,
201202
_self_ref: &FileDescriptionRef,
202203
_communicate_allowed: bool,
203-
bytes: &[u8],
204+
ptr: Pointer,
205+
len: usize,
204206
dest: &MPlaceTy<'tcx>,
205207
ecx: &mut MiriInterpCx<'tcx>,
206208
) -> InterpResult<'tcx> {
207-
let write_size = bytes.len();
208209
// Always succeed on write size 0.
209210
// ("If count is zero and fd refers to a file other than a regular file, the results are not specified.")
210-
if write_size == 0 {
211+
if len == 0 {
211212
let result = Ok(0);
212213
return ecx.return_written_byte_count_or_error(result, dest);
213214
}
@@ -243,7 +244,8 @@ impl FileDescription for AnonSocket {
243244
writebuf.clock.join(clock);
244245
}
245246
// Do full write / partial write based on the space available.
246-
let actual_write_size = write_size.min(available_space);
247+
let actual_write_size = len.min(available_space);
248+
let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?.to_owned();
247249
writebuf.buf.extend(&bytes[..actual_write_size]);
248250

249251
// Need to stop accessing peer_fd so that it can be notified.

0 commit comments

Comments
 (0)