From 62f76d2a762b64e747e77b1325b29cfcdba5a41b Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Fri, 1 Sep 2023 21:30:52 -0700 Subject: [PATCH] Restrict MaybeUninit trait impls to fix soundness (#308) Previously, we implemented `FromZeroes` and `FromBytes` for `MaybeUninit` with no bound on `T`. This resulted in a soundness hole in which `T` - and thus `MaybeUninit` - 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, especially given that the 0.7.x release train was published very recently. Thus, in this commit, we publish 0.7.3, and we will yank 0.7.{0,1,2} as soon as 0.7.3 is published. Fixes #299 --- Cargo.toml | 8 ++++---- src/lib.rs | 20 +++++++++++++++----- zerocopy-derive/Cargo.toml | 2 +- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d2ae5fad83..3126de3c5e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ [package] edition = "2021" name = "zerocopy" -version = "0.7.2" +version = "0.7.3" authors = ["Joshua Liebow-Feeser "] description = "Utilities for zero-copy parsing and serialization" license = "BSD-2-Clause" @@ -55,7 +55,7 @@ simd-nightly = ["simd"] __internal_use_only_features_that_work_on_stable = ["alloc", "simd"] [dependencies] -zerocopy-derive = { version = "=0.7.2", path = "zerocopy-derive", optional = true } +zerocopy-derive = { version = "=0.7.3", path = "zerocopy-derive", optional = true } [dependencies.byteorder] version = "1.3" @@ -66,7 +66,7 @@ optional = true # zerocopy-derive remain equal, even if the 'derive' feature isn't used. # See: https://github.com/matklad/macro-dep-test [target.'cfg(any())'.dependencies] -zerocopy-derive = { version = "=0.7.2", path = "zerocopy-derive" } +zerocopy-derive = { version = "=0.7.3", path = "zerocopy-derive" } [dev-dependencies] rand = "0.6" @@ -78,4 +78,4 @@ static_assertions = "1.1" # CI test failures. trybuild = "=1.0.80" # In tests, unlike in production, zerocopy-derive is not optional -zerocopy-derive = { version = "=0.7.2", path = "zerocopy-derive" } +zerocopy-derive = { version = "=0.7.3", path = "zerocopy-derive" } diff --git a/src/lib.rs b/src/lib.rs index a40993b0ed..c462c15d7e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -996,13 +996,23 @@ safety_comment! { // /// SAFETY: /// - `FromZeroes`, `FromBytes`: `MaybeUninit` has no restrictions on its - /// contents. + /// 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` - contains to `UnsafeCell`s. + /// Thus, requiring that `T` implement each of these traits is sufficient /// - `Unaligned`: `MaybeUninit` 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 => FromZeroes for MaybeUninit); - unsafe_impl!(T => FromBytes for MaybeUninit); + /// [1] + /// https://doc.rust-lang.org/nightly/core/mem/union.MaybeUninit.html#layout-1 + /// + /// 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: FromZeroes => FromZeroes for MaybeUninit); + unsafe_impl!(T: FromBytes => FromBytes for MaybeUninit); unsafe_impl!(T: Unaligned => Unaligned for MaybeUninit); assert_unaligned!(MaybeUninit<()>, MaybeUninit); } @@ -4059,7 +4069,7 @@ mod tests { assert_impls!(ManuallyDrop<[NotZerocopy]>: !FromZeroes, !FromBytes, !AsBytes, !Unaligned); assert_impls!(MaybeUninit: FromZeroes, FromBytes, Unaligned, !AsBytes); - assert_impls!(MaybeUninit: FromZeroes, FromBytes, !AsBytes, !Unaligned); + assert_impls!(MaybeUninit: !FromZeroes, !FromBytes, !AsBytes, !Unaligned); assert_impls!(Wrapping: FromZeroes, FromBytes, AsBytes, Unaligned); assert_impls!(Wrapping: !FromZeroes, !FromBytes, !AsBytes, !Unaligned); diff --git a/zerocopy-derive/Cargo.toml b/zerocopy-derive/Cargo.toml index 3f22d3c1b3..3d19d6ce16 100644 --- a/zerocopy-derive/Cargo.toml +++ b/zerocopy-derive/Cargo.toml @@ -5,7 +5,7 @@ [package] edition = "2021" name = "zerocopy-derive" -version = "0.7.2" +version = "0.7.3" authors = ["Joshua Liebow-Feeser "] description = "Custom derive for traits from the zerocopy crate" license = "BSD-2-Clause"