-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Closed
Closed
Copy link
Labels
C-bugCategory: This is a bug.Category: This is a bug.
Description
Code
#![feature(read_buf)]
use std::io::{self, BufWriter, Read, ReadBuf};
struct BadReader;
impl Read for BadReader {
fn read(&mut self, _buf: &mut [u8]) -> io::Result<usize> {
unreachable!()
}
fn read_buf(&mut self, read_buf: &mut ReadBuf<'_>) -> io::Result<()> {
let vec = vec![0; 2048];
let mut buf = ReadBuf::new(vec.leak());
buf.append(&[123; 2048]);
*read_buf = buf;
Ok(())
}
}
fn main() {
let mut buf = Vec::new();
{
let mut buf_ = BufWriter::with_capacity(1024, &mut buf);
let mut input = BadReader.take(4096);
io::copy(&mut input, &mut buf_).unwrap();
}
// read the memory to trigger miri to realize that the memory is uninitialized
println!("buf[0]: {}", buf[0]);
}
Playground link: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=483f0b4f2f881519f459c3cd273a50f4 (run miri on it)
Unsound implementation
rust/library/std/src/io/copy.rs
Lines 80 to 122 in 92ed874
fn copy_to<R: Read + ?Sized>(reader: &mut R, writer: &mut Self) -> Result<u64> { | |
if writer.capacity() < DEFAULT_BUF_SIZE { | |
return stack_buffer_copy(reader, writer); | |
} | |
let mut len = 0; | |
let mut init = 0; | |
loop { | |
let buf = writer.buffer_mut(); | |
let mut read_buf = ReadBuf::uninit(buf.spare_capacity_mut()); | |
// SAFETY: init is either 0 or the initialized_len of the previous iteration | |
unsafe { | |
read_buf.assume_init(init); | |
} | |
if read_buf.capacity() >= DEFAULT_BUF_SIZE { | |
match reader.read_buf(&mut read_buf) { | |
Ok(()) => { | |
let bytes_read = read_buf.filled_len(); | |
if bytes_read == 0 { | |
return Ok(len); | |
} | |
init = read_buf.initialized_len() - bytes_read; | |
// SAFETY: ReadBuf guarantees all of its filled bytes are init | |
unsafe { buf.set_len(buf.len() + bytes_read) }; | |
len += bytes_read as u64; | |
// Read again if the buffer still has enough capacity, as BufWriter itself would do | |
// This will occur if the reader returns short reads | |
continue; | |
} | |
Err(ref e) if e.kind() == ErrorKind::Interrupted => continue, | |
Err(e) => return Err(e), | |
} | |
} | |
writer.flush_buf()?; | |
} | |
} |
(we're basically tricking it into calling set_len
with a length higher than what has actually been filled)
Documentation
Should this be considered a possible footgun when implementing something that calls read_buf
? Should it be documented?
Metadata
Metadata
Assignees
Labels
C-bugCategory: This is a bug.Category: This is a bug.