Skip to content

Commit

Permalink
Restrict MaybeUninit trait impls to fix soundness (#309)
Browse files Browse the repository at this point in the history
Previously, we implemented `FromZeroes` and `FromBytes` for
`MaybeUninit<T>` with no bound on `T`. This resulted in a soundness hole
in which `T` - and thus `MaybeUninit<T>` - could contain an
`UnsafeCell`, which is a violation of the contracts of `FromZeroes` and
`FromBytes`.

This is a breaking change, but it's very unlikely to be one that code is
currently relying on. In this commit, we publish 0.6.4, and we will yank
all preceding 0.6.x versions as soon as 0.6.4 is published.

This is a backport of #308
  • Loading branch information
joshlf authored Sep 2, 2023
1 parent 9bc48cc commit c33bc31
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 5 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
[package]
edition = "2021"
name = "zerocopy"
version = "0.6.3"
version = "0.6.4"
authors = ["Joshua Liebow-Feeser <joshlf@google.com>"]
description = "Utilities for zero-copy parsing and serialization"
license = "BSD-2-Clause"
Expand All @@ -38,7 +38,7 @@ simd-nightly = ["simd"]
__internal_use_only_features_that_work_on_stable = ["alloc", "simd"]

[dependencies]
zerocopy-derive = { version = "=0.6.3", path = "zerocopy-derive" }
zerocopy-derive = { version = "=0.6.4", path = "zerocopy-derive" }

[dependencies.byteorder]
version = "1.3"
Expand Down
14 changes: 12 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -869,11 +869,21 @@ safety_comment! {
//
/// SAFETY:
/// - `FromBytes`: `MaybeUninit<T>` has no restrictions on its contents.
/// Unfortunately, in addition to bit validity, `FromZeroes` and
/// `FromBytes` also require that implementers contain no `UnsafeCell`s.
/// Thus, we require `T: FromZeroes` and `T: FromBytes` in order to ensure
/// that `T` - and thus `MaybeUninit<T>` - contains to `UnsafeCell`s.
/// Thus, requiring that `T` implement each of these traits is sufficient
/// - `Unaligned`: `MaybeUninit<T>` is guaranteed by its documentation [1]
/// to have the same alignment as `T`.
///
/// [1] https://doc.rust-lang.org/nightly/core/mem/union.MaybeUninit.html#layout-1
unsafe_impl!(T => FromBytes for MaybeUninit<T>);
///
/// TODO(https://github.com/google/zerocopy/issues/251): If we split
/// `FromBytes` and `RefFromBytes`, or if we introduce a separate
/// `NoCell`/`Freeze` trait, we can relax the trait bounds for `FromZeroes`
/// and `FromBytes`.
unsafe_impl!(T: FromBytes => FromBytes for MaybeUninit<T>);
unsafe_impl!(T: Unaligned => Unaligned for MaybeUninit<T>);
assert_unaligned!(MaybeUninit<()>, MaybeUninit<u8>);
}
Expand Down Expand Up @@ -4007,7 +4017,7 @@ mod tests {
assert_impls!(ManuallyDrop<[NotZerocopy]>: !FromBytes, !AsBytes, !Unaligned);

assert_impls!(MaybeUninit<u8>: FromBytes, Unaligned, !AsBytes);
assert_impls!(MaybeUninit<NotZerocopy>: FromBytes, !AsBytes, !Unaligned);
assert_impls!(MaybeUninit<NotZerocopy>: !FromBytes, !AsBytes, !Unaligned);

assert_impls!(Wrapping<u8>: FromBytes, AsBytes, Unaligned);
assert_impls!(Wrapping<NotZerocopy>: !FromBytes, !AsBytes, !Unaligned);
Expand Down
2 changes: 1 addition & 1 deletion zerocopy-derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
[package]
edition = "2021"
name = "zerocopy-derive"
version = "0.6.3"
version = "0.6.4"
authors = ["Joshua Liebow-Feeser <joshlf@google.com>"]
description = "Custom derive for traits from the zerocopy crate"
license = "BSD-2-Clause"
Expand Down

0 comments on commit c33bc31

Please sign in to comment.