linux-kernel - Re: [PATCH v2 09/22] rust: pin-init: move impl `Zeroable` for `Opaque` and `Option<KBox<T>>` into the kernel crate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <m2zfhpxjsb.fsf@kloenk.dev>
Date: Thu, 13 Mar 2025 12:16:36 +0100
From: Fiona Behrens <me@...enk.dev>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: Danilo Krummrich <dakr@...nel.org>,  Miguel Ojeda <ojeda@...nel.org>,
  Alex Gaynor <alex.gaynor@...il.com>,  Boqun Feng <boqun.feng@...il.com>,
  Gary Guo <gary@...yguo.net>,  Björn Roy Baron
 <bjorn3_gh@...tonmail.com>,
  Andreas Hindborg <a.hindborg@...nel.org>,  Alice Ryhl
 <aliceryhl@...gle.com>,  Trevor Gross <tmgross@...ch.edu>,
  rust-for-linux@...r.kernel.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 09/22] rust: pin-init: move impl `Zeroable` for
 `Opaque` and `Option<KBox<T>>` into the kernel crate

Benno Lossin <benno.lossin@...ton.me> writes:

> In order to make pin-init a standalone crate, move kernel-specific code
> directly into the kernel crate. Since `Opaque<T>` and `KBox<T>` are part
> of the kernel, move their `Zeroable` implementation into the kernel
> crate.
>
> Signed-off-by: Benno Lossin <benno.lossin@...ton.me>
> Tested-by: Andreas Hindborg <a.hindborg@...nel.org>

Reviewed-by: Fiona Behrens <me@...enk.dev>

> ---
>  rust/kernel/alloc/kbox.rs | 8 +++++++-
>  rust/kernel/types.rs      | 5 ++++-
>  rust/pin-init/src/lib.rs  | 8 +-------
>  3 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> index 39a3ea7542da..9861433559dc 100644
> --- a/rust/kernel/alloc/kbox.rs
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -15,7 +15,7 @@
>  use core::ptr::NonNull;
>  use core::result::Result;
>  
> -use crate::init::{InPlaceWrite, Init, PinInit};
> +use crate::init::{InPlaceWrite, Init, PinInit, Zeroable};
>  use crate::init_ext::InPlaceInit;
>  use crate::types::ForeignOwnable;
>  
> @@ -100,6 +100,12 @@
>  /// ```
>  pub type KVBox<T> = Box<T, super::allocator::KVmalloc>;
>  
> +// SAFETY: All zeros is equivalent to `None` (option layout optimization guarantee).
> +//
> +// In this case we are allowed to use `T: ?Sized`, since all zeros is the `None` variant and there
> +// is no problem with a VTABLE pointer being null.
> +unsafe impl<T: ?Sized, A: Allocator> Zeroable for Option<Box<T, A>> {}
> +
>  // SAFETY: `Box` is `Send` if `T` is `Send` because the `Box` owns a `T`.
>  unsafe impl<T, A> Send for Box<T, A>
>  where
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 2bbaab83b9d6..9f75bd3866e8 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -2,7 +2,7 @@
>  
>  //! Kernel types.
>  
> -use crate::init::{self, PinInit};
> +use crate::init::{self, PinInit, Zeroable};
>  use core::{
>      cell::UnsafeCell,
>      marker::{PhantomData, PhantomPinned},
> @@ -309,6 +309,9 @@ pub struct Opaque<T> {
>      _pin: PhantomPinned,
>  }
>  
> +// SAFETY: `Opaque<T>` allows the inner value to be any bit pattern, including all zeros.
> +unsafe impl<T> Zeroable for Opaque<T> {}
> +
>  impl<T> Opaque<T> {
>      /// Creates a new opaque value.
>      pub const fn new(value: T) -> Self {
> diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
> index f88465e0bb76..aad6486d33fc 100644
> --- a/rust/pin-init/src/lib.rs
> +++ b/rust/pin-init/src/lib.rs
> @@ -211,10 +211,7 @@
>  //! [`pin_data`]: ::macros::pin_data
>  //! [`pin_init!`]: crate::pin_init!
>  
> -use crate::{
> -    alloc::KBox,
> -    types::{Opaque, ScopeGuard},
> -};
> +use crate::{alloc::KBox, types::ScopeGuard};
>  use core::{
>      cell::UnsafeCell,
>      convert::Infallible,
> @@ -1342,8 +1339,6 @@ macro_rules! impl_zeroable {
>  
>      // SAFETY: Type is allowed to take any value, including all zeros.
>      {<T>} MaybeUninit<T>,
> -    // SAFETY: Type is allowed to take any value, including all zeros.
> -    {<T>} Opaque<T>,
>  
>      // SAFETY: `T: Zeroable` and `UnsafeCell` is `repr(transparent)`.
>      {<T: ?Sized + Zeroable>} UnsafeCell<T>,
> @@ -1358,7 +1353,6 @@ macro_rules! impl_zeroable {
>      //
>      // In this case we are allowed to use `T: ?Sized`, since all zeros is the `None` variant.
>      {<T: ?Sized>} Option<NonNull<T>>,
> -    {<T: ?Sized>} Option<KBox<T>>,
>  
>      // SAFETY: `null` pointer is valid.
>      //