Skip to content

Commit e329859

Browse files
committed
Fix soundness of FromBytes::read_from_io (google#2320)
See google#2319. Includes a Miri test confirming the previous unsoundness. gherrit-pr-id: Iede94c196c710c74d970c93935f1539e07446e50
1 parent 49a13ba commit e329859

File tree

1 file changed

+40
-2
lines changed

1 file changed

+40
-2
lines changed

src/lib.rs

+40-2
Original file line numberDiff line numberDiff line change
@@ -4567,9 +4567,17 @@ pub unsafe trait FromBytes: FromZeros {
45674567
Self: Sized,
45684568
R: io::Read,
45694569
{
4570-
let mut buf = CoreMaybeUninit::<Self>::zeroed();
4570+
// NOTE(#2319, #2320): We do `buf.zero()` separately rather than
4571+
// constructing `let buf = CoreMaybeUninit::zeroed()` because, if `Self`
4572+
// contains padding bytes, then a typed copy of `CoreMaybeUninit<Self>`
4573+
// will not necessarily preserve zeros written to those padding byte
4574+
// locations, and so `buf` could contain uninitialized bytes.
4575+
let mut buf = CoreMaybeUninit::<Self>::uninit();
4576+
buf.zero();
4577+
45714578
let ptr = Ptr::from_mut(&mut buf);
4572-
// SAFETY: `buf` consists entirely of initialized, zeroed bytes.
4579+
// SAFETY: After `buf.zero()`, `buf` consists entirely of initialized,
4580+
// zeroed bytes.
45734581
let ptr = unsafe { ptr.assume_validity::<invariant::Initialized>() };
45744582
let ptr = ptr.as_bytes::<BecauseExclusive>();
45754583
src.read_exact(ptr.as_mut())?;
@@ -6079,6 +6087,36 @@ mod tests {
60796087
assert_eq!(bytes, want);
60806088
}
60816089

6090+
#[test]
6091+
#[cfg(feature = "std")]
6092+
fn test_read_io_with_padding_soundness() {
6093+
// This test is designed to exhibit potential UB in
6094+
// `FromBytes::read_from_io`. (see #2319, #2320).
6095+
6096+
// On most platforms (where `align_of::<u16>() == 2`), `WithPadding`
6097+
// will have inter-field padding between `x` and `y`.
6098+
#[derive(FromBytes)]
6099+
#[repr(C)]
6100+
struct WithPadding {
6101+
x: u8,
6102+
y: u16,
6103+
}
6104+
struct ReadsInRead;
6105+
impl std::io::Read for ReadsInRead {
6106+
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
6107+
// This body branches on every byte of `buf`, ensuring that it
6108+
// exhibits UB if any byte of `buf` is uninitialized.
6109+
if buf.iter().all(|&x| x == 0) {
6110+
Ok(buf.len())
6111+
} else {
6112+
buf.iter_mut().for_each(|x| *x = 0);
6113+
Ok(buf.len())
6114+
}
6115+
}
6116+
}
6117+
assert!(matches!(WithPadding::read_from_io(ReadsInRead), Ok(WithPadding { x: 0, y: 0 })));
6118+
}
6119+
60826120
#[test]
60836121
#[cfg(feature = "std")]
60846122
fn test_read_write_io() {

0 commit comments

Comments
 (0)