Skip to content

Commit ea7c136

Browse files
author
Askar Safin
committed
Fix libc::read shim: make it write to a buffer correct amount of bytes. Add tests for the new behavior.
libc::read shim had a bug: if underlying real call libc::read(fd, buf, N) returns M, then libc::read shim writes N bytes to buf instead of M. Remaining N - M bytes are filled with zeros. This commit fixes this bug and adds tests for new behavior
1 parent c471589 commit ea7c136

File tree

4 files changed

+83
-1
lines changed

4 files changed

+83
-1
lines changed

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

+6-1
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
394394
match result {
395395
Ok(read_bytes) => {
396396
// If reading to `bytes` did not fail, we write those bytes to the buffer.
397-
this.write_bytes_ptr(buf, bytes)?;
397+
// Crucially, if fewer than `bytes.len()` bytes were read, only write
398+
// that much into the output buffer!
399+
this.write_bytes_ptr(
400+
buf,
401+
bytes[..usize::try_from(read_bytes).unwrap()].iter().copied(),
402+
)?;
398403
Ok(read_bytes)
399404
}
400405
Err(e) => {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//! We test that if we requested to read 4 bytes, but actually read 3 bytes,
2+
//! then 3 bytes (not 4) will be initialized.
3+
//@ignore-target-windows: no file system support on Windows
4+
//@compile-flags: -Zmiri-disable-isolation
5+
6+
use std::ffi::CString;
7+
use std::fs::remove_file;
8+
use std::mem::MaybeUninit;
9+
10+
#[path = "../../utils/mod.rs"]
11+
mod utils;
12+
13+
fn main() {
14+
let path =
15+
utils::prepare_with_content("fail-libc-read-and-uninit-premature-eof.txt", &[1u8, 2, 3]);
16+
let cpath = CString::new(path.clone().into_os_string().into_encoded_bytes()).unwrap();
17+
unsafe {
18+
let fd = libc::open(cpath.as_ptr(), libc::O_RDONLY);
19+
assert_ne!(fd, -1);
20+
let mut buf: MaybeUninit<[u8; 4]> = std::mem::MaybeUninit::uninit();
21+
// Read 4 bytes from a 3-byte file.
22+
assert_eq!(libc::read(fd, buf.as_mut_ptr().cast::<std::ffi::c_void>(), 4), 3);
23+
buf.assume_init(); //~ERROR: Undefined Behavior: constructing invalid value at .value[3]: encountered uninitialized memory, but expected an integer
24+
assert_eq!(libc::close(fd), 0);
25+
}
26+
remove_file(&path).unwrap();
27+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: constructing invalid value at .value[3]: encountered uninitialized memory, but expected an integer
2+
--> $DIR/libc-read-and-uninit-premature-eof.rs:LL:CC
3+
|
4+
LL | ... buf.assume_init();
5+
| ^^^^^^^^^^^^^^^^^ constructing invalid value at .value[3]: encountered uninitialized memory, but expected an integer
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at $DIR/libc-read-and-uninit-premature-eof.rs:LL:CC
11+
12+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
13+
14+
error: aborting due to 1 previous error
15+

src/tools/miri/tests/pass-dep/libc/libc-fs.rs

+35
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ fn main() {
3535
#[cfg(target_os = "linux")]
3636
test_sync_file_range();
3737
test_isatty();
38+
test_read_and_uninit();
3839
}
3940

4041
fn test_file_open_unix_allow_two_args() {
@@ -362,3 +363,37 @@ fn test_isatty() {
362363
remove_file(&path).unwrap();
363364
}
364365
}
366+
367+
fn test_read_and_uninit() {
368+
use std::mem::MaybeUninit;
369+
{
370+
// We test that libc::read initializes its buffer.
371+
let path = utils::prepare_with_content("pass-libc-read-and-uninit.txt", &[1u8, 2, 3]);
372+
let cpath = CString::new(path.clone().into_os_string().into_encoded_bytes()).unwrap();
373+
unsafe {
374+
let fd = libc::open(cpath.as_ptr(), libc::O_RDONLY);
375+
assert_ne!(fd, -1);
376+
let mut buf: MaybeUninit<[u8; 2]> = std::mem::MaybeUninit::uninit();
377+
assert_eq!(libc::read(fd, buf.as_mut_ptr().cast::<std::ffi::c_void>(), 2), 2);
378+
let buf = buf.assume_init();
379+
assert_eq!(buf, [1, 2]);
380+
assert_eq!(libc::close(fd), 0);
381+
}
382+
remove_file(&path).unwrap();
383+
}
384+
{
385+
// We test that if we requested to read 4 bytes, but actually read 3 bytes, then
386+
// 3 bytes (not 4) will be overwritten, and remaining byte will be left as-is.
387+
let path = utils::prepare_with_content("pass-libc-read-and-uninit-2.txt", &[1u8, 2, 3]);
388+
let cpath = CString::new(path.clone().into_os_string().into_encoded_bytes()).unwrap();
389+
unsafe {
390+
let fd = libc::open(cpath.as_ptr(), libc::O_RDONLY);
391+
assert_ne!(fd, -1);
392+
let mut buf = [42u8; 5];
393+
assert_eq!(libc::read(fd, buf.as_mut_ptr().cast::<std::ffi::c_void>(), 4), 3);
394+
assert_eq!(buf, [1, 2, 3, 42, 42]);
395+
assert_eq!(libc::close(fd), 0);
396+
}
397+
remove_file(&path).unwrap();
398+
}
399+
}

0 commit comments

Comments
 (0)