Skip to content

Commit 1339ee9

Browse files
authored
Fix soundness of FromBytes::read_from_io (#2320)
See #2319. Includes a Miri test confirming the previous unsoundness. gherrit-pr-id: Iede94c196c710c74d970c93935f1539e07446e50
1 parent 79ec7c4 commit 1339ee9

File tree

1 file changed

+40
-2
lines changed

1 file changed

+40
-2
lines changed

src/lib.rs

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4546,9 +4546,17 @@ pub unsafe trait FromBytes: FromZeros {
45464546
Self: Sized,
45474547
R: io::Read,
45484548
{
4549-
let mut buf = CoreMaybeUninit::<Self>::zeroed();
4549+
// NOTE(#2319, #2320): We do `buf.zero()` separately rather than
4550+
// constructing `let buf = CoreMaybeUninit::zeroed()` because, if `Self`
4551+
// contains padding bytes, then a typed copy of `CoreMaybeUninit<Self>`
4552+
// will not necessarily preserve zeros written to those padding byte
4553+
// locations, and so `buf` could contain uninitialized bytes.
4554+
let mut buf = CoreMaybeUninit::<Self>::uninit();
4555+
buf.zero();
4556+
45504557
let ptr = Ptr::from_mut(&mut buf);
4551-
// SAFETY: `buf` consists entirely of initialized, zeroed bytes.
4558+
// SAFETY: After `buf.zero()`, `buf` consists entirely of initialized,
4559+
// zeroed bytes.
45524560
let ptr = unsafe { ptr.assume_validity::<invariant::Initialized>() };
45534561
let ptr = ptr.as_bytes::<BecauseExclusive>();
45544562
src.read_exact(ptr.as_mut())?;
@@ -5905,6 +5913,36 @@ mod tests {
59055913
assert_eq!(bytes, want);
59065914
}
59075915

5916+
#[test]
5917+
#[cfg(feature = "std")]
5918+
fn test_read_io_with_padding_soundness() {
5919+
// This test is designed to exhibit potential UB in
5920+
// `FromBytes::read_from_io`. (see #2319, #2320).
5921+
5922+
// On most platforms (where `align_of::<u16>() == 2`), `WithPadding`
5923+
// will have inter-field padding between `x` and `y`.
5924+
#[derive(FromBytes)]
5925+
#[repr(C)]
5926+
struct WithPadding {
5927+
x: u8,
5928+
y: u16,
5929+
}
5930+
struct ReadsInRead;
5931+
impl std::io::Read for ReadsInRead {
5932+
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
5933+
// This body branches on every byte of `buf`, ensuring that it
5934+
// exhibits UB if any byte of `buf` is uninitialized.
5935+
if buf.iter().all(|&x| x == 0) {
5936+
Ok(buf.len())
5937+
} else {
5938+
buf.iter_mut().for_each(|x| *x = 0);
5939+
Ok(buf.len())
5940+
}
5941+
}
5942+
}
5943+
assert!(matches!(WithPadding::read_from_io(ReadsInRead), Ok(WithPadding { x: 0, y: 0 })));
5944+
}
5945+
59085946
#[test]
59095947
#[cfg(feature = "std")]
59105948
fn test_read_write_io() {

0 commit comments

Comments
 (0)