linux-kernel - Re: [PATCH v2 1/3] rust: add UnsafePinned type

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <D8QHB0Y7AZ31.PVET84S32Q6X@proton.me>
Date: Wed, 26 Mar 2025 20:26:41 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Christian Schrefl <chrisi.schrefl@...il.com>, 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>, Arnd Bergmann <arnd@...db.de>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Lee Jones <lee@...nel.org>, Daniel Almeida <daniel.almeida@...labora.com>, Danilo Krummrich <dakr@...nel.org>
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] rust: add UnsafePinned type

On Fri Jan 31, 2025 at 4:08 PM CET, Christian Schrefl wrote:
> @@ -573,3 +576,57 @@ pub enum Either<L, R> {
>  /// [`NotThreadSafe`]: type@...ThreadSafe
>  #[allow(non_upper_case_globals)]
>  pub const NotThreadSafe: NotThreadSafe = PhantomData;
> +
> +/// Stores a value that may be used from multiple mutable pointers.
> +///
> +/// `UnsafePinned` gets rid of some of the usual assumptions that Rust has for a value:
> +/// - The value is allowed to be mutated, when a `&UnsafePinned<T>` exists on the Rust side.
> +/// - No uniqueness for mutable references: it is fine to have multiple `&mut UnsafePinned<T>`
> +///   point to the same value.

We have another patch series [1] in transit that changes the wording on
the `Opaque<T>` type. I think we should mirror the same wording here.

[1]: https://lore.kernel.org/rust-for-linux/20250305053438.1532397-2-dirk.behme@de.bosch.com/

> +///
> +/// To avoid the ability to use [`core::mem::swap`] this still needs to be used through a

IIRC typing out the whole path makes it also appear in the docs. I think
we should just use `mem::swap` and omit `core` of course you need to
add this to make it work:
    
    /// [`mem::swap`]: core::mem::swap

> +/// [`core::pin::Pin`] reference.

Here, I would link to `Pin`, but say "[pinned](core::pin::Pin) pointer."
instead, since eg `Pin<Box<T>>` also is okay to use.

> +///
> +/// This is useful for cases where a value might be shared with C code
> +/// but not interpreted by it or in cases where it can not always be guaranteed that the
> +/// references are unique.
> +///
> +/// This is similar to [`Opaque<T>`] but is guaranteed to always contain valid data and will
> +/// call the [`Drop`] implementation of `T` when dropped.
> +#[repr(transparent)]
> +pub struct UnsafePinned<T> {
> +    value: UnsafeCell<T>,
> +    _pin: PhantomPinned,
> +}
> +
> +impl<T> UnsafePinned<T> {
> +    /// Creates a new [`UnsafePinned`] value.
> +    pub const fn new(value: T) -> Self {
> +        Self {
> +            value: UnsafeCell::new(value),
> +            _pin: PhantomPinned,
> +        }
> +    }
> +
> +    /// Create an [`UnsafePinned`] pin-initializer from the given pin-initializer.
> +    pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> +        // SAFETY:
> +        //   - In case of an error in `value` the error is returned, otherwise `slot` is fully
> +        //     initialized, since `self.value` is initialized and `_pin` is a zero sized type.
> +        //   - The `Pin` invariants of `self.value` are upheld, since no moving occurs.
> +        unsafe { init::pin_init_from_closure(move |slot| value.__pinned_init(Self::raw_get(slot))) }

Ah this is a bit suboptimal, but I guess there currently isn't a better
way to do this. I'll add it to my list of things to improve with
pin-init.

> +    }
> +
> +    /// Returns a raw pointer to the contained data.
> +    pub const fn get(&self) -> *mut T {
> +        UnsafeCell::get(&self.value).cast::<T>()
> +    }
> +
> +    /// Gets the value behind `this`.
> +    ///
> +    /// This function is useful to get access to the value without creating intermediate
> +    /// references.
> +    pub const fn raw_get(this: *const Self) -> *mut T {
> +        UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>()

Why the cast to `MaybeUninit<T>`? I think this can just be:

    UnsafeCell::raw_get(&raw const this.value)

---
Cheers,
Benno

> +    }
> +}