Unsound `BufWriter` copy_to specialization with the unstable `read_buf` feature · Issue #93305 · rust-lang/rust (original) (raw)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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 { 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

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?

Unsound BufWriter copy_to specialization with the unstable read_buf feature · Issue #93305 · rust-lang/rust (original) (raw)

Navigation Menu