Re: [PATCH v4 1/3] rust: add UnsafePinned type

["Benno Lossin" <lossin@kernel.org>]

On Thu Jun 5, 2025 at 7:57 PM CEST, Christian Schrefl wrote:
> On 05.06.25 7:30 PM, Benno Lossin wrote:
>> On Thu Jun 5, 2025 at 7:03 PM CEST, Christian Schrefl wrote:
>>> On 11.05.25 8:21 PM, Christian Schrefl wrote:
>>>> +/// This type provides a way to opt-out of typical aliasing rules;
>>>> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
>>>> +///
>>>> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
>>>> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
>>>> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
>>>> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
>>>> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
>>>> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
>>>> +///
>>>> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
>>>> +/// the public API of a library. It is an internal implementation detail of libraries that need to
>>>> +/// support aliasing mutable references.
>>>> +///
>>>> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
>>>> +/// Use [`UnsafeCell`] for that.
>>>
>>> The upstream rust PR [0] that changes this was just merged. So now `UnsafePinned` includes
>>> `UnsafeCell` semantics. It's probably best to also change this in the kernel docs.
>>> Though it's still the case that removing the guarantee is simpler than adding it back later,
>>> so let me know what you all think.
>> 
>> Depends on how "stable" this decision is. I haven't followed the
>> discussion, but given that this once changed to the "non-backwards"
>> compatible case it feels permanent.
>
> It seems pretty permanent, from what I understand its hard to
> define the exact semantics `UnsafePinned` without `UnsafeCell`
> in a way that is sound and because of some interactions with
> `Pin::deref` it would have some backwards compatibility issues.
> See this comment by Ralf on github [1].

Oh yeah that seems like a done deal.

> [1]: https://github.com/rust-lang/rust/pull/137043#discussion_r1973978597
>
>> 
>> How close is it to stabilization?
>> 
>> If it's close-ish, then I'd suggest we change this to reflect the new
>> semantics. If not, then we should leave it as-is.
>
> It's pretty new, I'm not sure how long it's going to stay in nightly,
> but it's probably going to be quite some time.
>
> I wouldn't change it if it would already be in the kernel, but I think
> its probably good to add the current state of the feature. This
> would also reduce the difference between docs and implementation.

Makes sense, I agree with changing it. Thanks for your work!

---
Cheers,
Benno