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] [thread-next>] [day] [month] [year] [list]

Message-Id: <D9L6XO6T7JEU.CK47C5BOQ0NG@kernel.org>
Date: Fri, 02 May 2025 00:51:40 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Christian Schrefl" <chrisi.schrefl@...il.com>, "Sky" <sky@...9.dev>,
 "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>, "Benno Lossin"
 <benno.lossin@...ton.me>, "Andreas Hindborg" <a.hindborg@...nel.org>,
 "Alice Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>,
 "Danilo Krummrich" <dakr@...nel.org>, Gerald Wisböck
 <gerald.wisboeck@...ther.ink>
Cc: <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] rust: add UnsafePinned type

On Thu May 1, 2025 at 9:11 PM CEST, Christian Schrefl wrote:
> On 01.05.25 8:51 PM, Benno Lossin wrote:
>> On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
>>> On 30.04.25 11:45 AM, Benno Lossin wrote:
>>>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>>>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>>>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>>>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>>>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>>>>> +/// required.
>>>>> +///
>>>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>>>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>>>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
>>>>
>>>> I would make this last paragraph a normal comment, I don't think we
>>>> should expose it in the docs.
>>>
>>> I added this as docs since I wanted it to be a bit more visible,
>>> but I can replace the comment text (about `UnsafeCell`) with this paragraph
>>> and drop it from the docs if you want.
>> 
>> I think we shouldn't talk about these implementation details in the
>> docs.
>
> Alright, what do you think of:
>
> // As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`

There are two '`' after PhantomPinned.

> // - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`

s/ a / an /

I find the phrasing 'avoid needing <negative impl>' a bit weird, I'd
just say "`PhantomPinned` to ensure the struct always is `!Unpin` and
thus enables the `!Unpin` hack".

If you have a link to somewhere that explains that hack, then I'd also
put it there. I forgot if it's written down somewhere.

> //   Required to use the `!Unpin hack`.
> // - In order to disable niche optimizations this implementation uses `UnsafeCell` internally,
> //   the upstream version however currently does not. This will most likely change in the future
> //   but for now we don't expose this in the documentation, since adding the guarantee is simpler
> //   than removing it. Meaning that for now the fact that `UnsafePinned` contains an `UnsafeCell`
> //   must not be relied on (Other than the niche blocking).

---
Cheers,
Benno