From 5e0fb23076b9e9d8bc9585cd2bb4a056e3b23261 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Sun, 10 Feb 2019 14:07:58 -0800 Subject: [PATCH] Deprecate Read::initializer in favor of ptr::freeze Read implementations should only write into the buffer passed to them, but have the ability to read from it. Access of uninitialized memory can easily cause UB, so there's then a question of what a user of a reader should do to initialize buffers. Previously, we allowed a Read implementation to promise it wouldn't look at the contents of the buffer, which allows the user to pass uninitialized memory to it. Instead, this PR adds a method to "freeze" undefined bytes into arbitrary-but-defined bytes. This is currently done via an inline assembly directive noting the address as an output, so LLVM no longer knows it's uninitialized. There is a proposed "freeze" operation in LLVM itself that would do this directly, but it hasn't been fully implemented. Some targets don't support inline assembly, so there we instead pass the pointer to an extern "C" function, which is similarly opaque to LLVM. The current approach is very low level. If we stabilize, we'll probably want to add something like `slice.freeze()` to make this easier to use. --- src/libcore/ptr.rs | 50 +++++++++++++++++++++++++++++++++ src/libstd/fs.rs | 6 +++- src/libstd/io/buffered.rs | 8 ++++-- src/libstd/io/cursor.rs | 5 +++- src/libstd/io/impls.rs | 7 ++++- src/libstd/io/mod.rs | 21 ++++++++++++-- src/libstd/io/stdio.rs | 7 ++++- src/libstd/io/util.rs | 9 ++++-- src/libstd/lib.rs | 1 + src/libstd/net/tcp.rs | 6 +++- src/libstd/process.rs | 6 +++- src/libstd/sys/redox/ext/net.rs | 6 +++- src/libstd/sys/unix/ext/net.rs | 6 +++- src/libstd/sys/unix/fd.rs | 5 +++- 14 files changed, 128 insertions(+), 15 deletions(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 02eef07afd7ab..6d0a5bc3d0201 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -946,6 +946,56 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { intrinsics::volatile_store(dst, src); } +/// Freezes `count * size_of::()` bytes of memory, converting undefined data into +/// defined-but-arbitrary data. +/// +/// Uninitialized memory has undefined contents, and interation with that data +/// can easily cause undefined behavior. This function "freezes" memory +/// contents, converting uninitialized memory to initialized memory with +/// arbitrary conents so that use of it is well defined. +/// +/// This function has no runtime effect; it is purely an instruction to the +/// compiler. In particular, it does not actually write anything to the memory. +/// +/// # Safety +/// +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `dst` must be [valid] for reads. +/// +/// Note that even if `T` has size `0`, the pointer must be non-NULL and properly aligned. +/// +/// [valid]: ../ptr/index.html#safety +/// +/// # Examples +/// +/// ```ignore (io-is-in-std) +/// use std::io::{self, Read}; +/// use std::mem; +/// use std::ptr; +/// +/// pub fn read_le_u32(reader: &mut R) -> io::Result +/// where +/// R: Read, +/// { +/// unsafe { +/// // We're passing this buffer to an arbitrary reader and aren't +/// // guaranteed they won't read from it, so freeze to avoid UB. +/// let mut buf: [u8; 4] = mem::uninitialized(); +/// ptr::freeze(&mut buf, 1); +/// reader.read_exact(&mut buf)?; +/// +/// Ok(u32::from_le_bytes(buf)) +/// } +/// } +/// ``` +#[inline] +#[unstable(feature = "ptr_freeze", issue = "0")] +pub unsafe fn freeze(dst: *mut T, count: usize) { + let _ = count; + asm!("" : "=*m"(dst) : ); +} + #[lang = "const_ptr"] impl *const T { /// Returns `true` if the pointer is null. diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index 3538816c1124c..a7a442373e584 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -9,7 +9,9 @@ use fmt; use ffi::OsString; -use io::{self, SeekFrom, Seek, Read, Initializer, Write}; +use io::{self, SeekFrom, Seek, Read, Write}; +#[allow(deprecated)] +use io::Initializer; use path::{Path, PathBuf}; use sys::fs as fs_imp; use sys_common::{AsInnerMut, FromInner, AsInner, IntoInner}; @@ -600,6 +602,7 @@ impl Read for File { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -624,6 +627,7 @@ impl<'a> Read for &'a File { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/io/buffered.rs b/src/libstd/io/buffered.rs index 056aa7c0c4263..b557ba5907d5c 100644 --- a/src/libstd/io/buffered.rs +++ b/src/libstd/io/buffered.rs @@ -5,8 +5,11 @@ use io::prelude::*; use cmp; use error; use fmt; -use io::{self, Initializer, DEFAULT_BUF_SIZE, Error, ErrorKind, SeekFrom}; +use io::{self, DEFAULT_BUF_SIZE, Error, ErrorKind, SeekFrom}; +#[allow(deprecated)] +use io::Initializer; use memchr; +use ptr; /// The `BufReader` struct adds buffering to any reader. /// @@ -92,7 +95,7 @@ impl BufReader { unsafe { let mut buffer = Vec::with_capacity(cap); buffer.set_len(cap); - inner.initializer().initialize(&mut buffer); + ptr::freeze(buffer.as_mut_ptr(), cap); BufReader { inner, buf: buffer.into_boxed_slice(), @@ -236,6 +239,7 @@ impl Read for BufReader { } // we can't skip unconditionally because of the large buffer case in read. + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { self.inner.initializer() } diff --git a/src/libstd/io/cursor.rs b/src/libstd/io/cursor.rs index b205f7888389f..d1923c35fd471 100644 --- a/src/libstd/io/cursor.rs +++ b/src/libstd/io/cursor.rs @@ -2,7 +2,9 @@ use io::prelude::*; use core::convert::TryInto; use cmp; -use io::{self, Initializer, SeekFrom, Error, ErrorKind}; +use io::{self, SeekFrom, Error, ErrorKind}; +#[allow(deprecated)] +use io::Initializer; /// A `Cursor` wraps an in-memory buffer and provides it with a /// [`Seek`] implementation. @@ -229,6 +231,7 @@ impl Read for Cursor where T: AsRef<[u8]> { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/io/impls.rs b/src/libstd/io/impls.rs index ec75a87aec34f..9bb0ebd116156 100644 --- a/src/libstd/io/impls.rs +++ b/src/libstd/io/impls.rs @@ -1,5 +1,7 @@ use cmp; -use io::{self, SeekFrom, Read, Initializer, Write, Seek, BufRead, Error, ErrorKind}; +use io::{self, SeekFrom, Read, Write, Seek, BufRead, Error, ErrorKind}; +#[allow(deprecated)] +use io::Initializer; use fmt; use mem; @@ -14,6 +16,7 @@ impl<'a, R: Read + ?Sized> Read for &'a mut R { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { (**self).initializer() } @@ -83,6 +86,7 @@ impl Read for Box { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { (**self).initializer() } @@ -172,6 +176,7 @@ impl<'a> Read for &'a [u8] { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index 28a6fbd48cf09..81956f0163624 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -367,7 +367,7 @@ fn read_to_end_with_reservation(r: &mut R, g.buf.reserve(reservation_size); let capacity = g.buf.capacity(); g.buf.set_len(capacity); - r.initializer().initialize(&mut g.buf[g.len..]); + ptr::freeze(g.buf.as_mut_ptr().add(g.len), g.buf.len() - g.len); } } @@ -543,6 +543,8 @@ pub trait Read { /// [`Initializer::nop()`]: ../../std/io/struct.Initializer.html#method.nop /// [`Initializer`]: ../../std/io/struct.Initializer.html #[unstable(feature = "read_initializer", issue = "42788")] + #[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")] + #[allow(deprecated)] #[inline] unsafe fn initializer(&self) -> Initializer { Initializer::zeroing() @@ -869,12 +871,22 @@ pub trait Read { /// A type used to conditionally initialize buffers passed to `Read` methods. #[unstable(feature = "read_initializer", issue = "42788")] -#[derive(Debug)] +#[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")] pub struct Initializer(bool); +#[allow(deprecated)] +#[unstable(feature = "read_initializer", issue = "42788")] +impl fmt::Debug for Initializer { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_tuple("Initializer").field(&self.0).finish() + } +} + +#[allow(deprecated)] impl Initializer { /// Returns a new `Initializer` which will zero out buffers. #[unstable(feature = "read_initializer", issue = "42788")] + #[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")] #[inline] pub fn zeroing() -> Initializer { Initializer(true) @@ -889,6 +901,7 @@ impl Initializer { /// the method accurately reflects the number of bytes that have been /// written to the head of the buffer. #[unstable(feature = "read_initializer", issue = "42788")] + #[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")] #[inline] pub unsafe fn nop() -> Initializer { Initializer(false) @@ -896,6 +909,7 @@ impl Initializer { /// Indicates if a buffer should be initialized. #[unstable(feature = "read_initializer", issue = "42788")] + #[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")] #[inline] pub fn should_initialize(&self) -> bool { self.0 @@ -903,6 +917,7 @@ impl Initializer { /// Initializes a buffer if necessary. #[unstable(feature = "read_initializer", issue = "42788")] + #[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")] #[inline] pub fn initialize(&self, buf: &mut [u8]) { if self.should_initialize() { @@ -1698,6 +1713,7 @@ impl Read for Chain { self.second.read(buf) } + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { let initializer = self.first.initializer(); if initializer.should_initialize() { @@ -1895,6 +1911,7 @@ impl Read for Take { Ok(n) } + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { self.inner.initializer() } diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index 4068c0f9c7de5..e0d92fb7f3ed8 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -3,7 +3,9 @@ use io::prelude::*; use cell::RefCell; use fmt; use io::lazy::Lazy; -use io::{self, Initializer, BufReader, LineWriter}; +use io::{self, BufReader, LineWriter}; +#[allow(deprecated)] +use io::Initializer; use sync::{Arc, Mutex, MutexGuard}; use sys::stdio; use sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard}; @@ -67,6 +69,7 @@ impl Read for StdinRaw { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -297,6 +300,7 @@ impl Read for Stdin { self.lock().read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -317,6 +321,7 @@ impl<'a> Read for StdinLock<'a> { self.inner.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/io/util.rs b/src/libstd/io/util.rs index 8df961a9add6b..54ddb767a1fe3 100644 --- a/src/libstd/io/util.rs +++ b/src/libstd/io/util.rs @@ -1,8 +1,11 @@ #![allow(missing_copy_implementations)] use fmt; -use io::{self, Read, Initializer, Write, ErrorKind, BufRead}; +use io::{self, Read, Write, ErrorKind, BufRead}; +#[allow(deprecated)] +use io::Initializer; use mem; +use ptr; /// Copies the entire contents of a reader into a writer. /// @@ -45,7 +48,7 @@ pub fn copy(reader: &mut R, writer: &mut W) -> io::Result< { let mut buf = unsafe { let mut buf: [u8; super::DEFAULT_BUF_SIZE] = mem::uninitialized(); - reader.initializer().initialize(&mut buf); + ptr::freeze(&mut buf, 1); buf }; @@ -97,6 +100,7 @@ impl Read for Empty { fn read(&mut self, _buf: &mut [u8]) -> io::Result { Ok(0) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -153,6 +157,7 @@ impl Read for Repeat { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 8ecba3ecd68fd..ca65e79e33a37 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -264,6 +264,7 @@ #![feature(panic_unwind)] #![feature(prelude_import)] #![feature(ptr_internals)] +#![feature(ptr_freeze)] #![feature(raw)] #![feature(hash_raw_entry)] #![feature(rustc_attrs)] diff --git a/src/libstd/net/tcp.rs b/src/libstd/net/tcp.rs index 86ecb10edf2f9..a78a1fb6cba4a 100644 --- a/src/libstd/net/tcp.rs +++ b/src/libstd/net/tcp.rs @@ -1,7 +1,9 @@ use io::prelude::*; use fmt; -use io::{self, Initializer}; +use io; +#[allow(deprecated)] +use io::Initializer; use net::{ToSocketAddrs, SocketAddr, Shutdown}; use sys_common::net as net_imp; use sys_common::{AsInner, FromInner, IntoInner}; @@ -570,6 +572,7 @@ impl Read for TcpStream { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -584,6 +587,7 @@ impl<'a> Read for &'a TcpStream { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/process.rs b/src/libstd/process.rs index 1263ef82e4872..1dfec6cc6d4cb 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -111,7 +111,9 @@ use io::prelude::*; use ffi::OsStr; use fmt; use fs; -use io::{self, Initializer}; +use io; +#[allow(deprecated)] +use io::Initializer; use path::Path; use str; use sys::pipe::{read2, AnonPipe}; @@ -272,6 +274,7 @@ impl Read for ChildStdout { self.inner.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -319,6 +322,7 @@ impl Read for ChildStderr { self.inner.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/sys/redox/ext/net.rs b/src/libstd/sys/redox/ext/net.rs index 76c68829b7f1b..f2664ffc54f86 100644 --- a/src/libstd/sys/redox/ext/net.rs +++ b/src/libstd/sys/redox/ext/net.rs @@ -3,7 +3,9 @@ //! Unix-specific networking functionality use fmt; -use io::{self, Error, ErrorKind, Initializer}; +use io::{self, Error, ErrorKind}; +#[allow(deprecated)] +use io::Initializer; use net::Shutdown; use os::unix::io::{RawFd, AsRawFd, FromRawFd, IntoRawFd}; use path::Path; @@ -410,6 +412,7 @@ impl io::Read for UnixStream { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -422,6 +425,7 @@ impl<'a> io::Read for &'a UnixStream { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/sys/unix/ext/net.rs b/src/libstd/sys/unix/ext/net.rs index a3ae5943f6038..c9798b0062a0f 100644 --- a/src/libstd/sys/unix/ext/net.rs +++ b/src/libstd/sys/unix/ext/net.rs @@ -18,7 +18,9 @@ mod libc { use ascii; use ffi::OsStr; use fmt; -use io::{self, Initializer}; +use io; +#[allow(deprecated)] +use io::Initializer; use mem; use net::{self, Shutdown}; use os::unix::ffi::OsStrExt; @@ -552,6 +554,7 @@ impl io::Read for UnixStream { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -564,6 +567,7 @@ impl<'a> io::Read for &'a UnixStream { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/sys/unix/fd.rs b/src/libstd/sys/unix/fd.rs index 2cbd9536f4da7..e287b50e08d65 100644 --- a/src/libstd/sys/unix/fd.rs +++ b/src/libstd/sys/unix/fd.rs @@ -1,7 +1,9 @@ #![unstable(reason = "not public", issue = "0", feature = "fd")] use cmp; -use io::{self, Read, Initializer}; +use io::{self, Read}; +#[allow(deprecated)] +use io::Initializer; use libc::{self, c_int, c_void, ssize_t}; use mem; use sync::atomic::{AtomicBool, Ordering}; @@ -262,6 +264,7 @@ impl<'a> Read for &'a FileDesc { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() }