linux-kernel - Re: [PATCH v11 1/4] rust: types: Add Ownable/Owned types

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

Message-Id: <DC6KN0JN4X4D.1PHXPQ46O5J1Q@kernel.org>
Date: Tue, 19 Aug 2025 19:13:56 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Andreas Hindborg" <a.hindborg@...nel.org>, "Oliver Mangold"
 <oliver.mangold@...me>
Cc: "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>, "Alice Ryhl" <aliceryhl@...gle.com>, "Trevor
 Gross" <tmgross@...ch.edu>, "Asahi Lina" <lina+kernel@...hilina.net>,
 <rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v11 1/4] rust: types: Add Ownable/Owned types

On Tue Aug 19, 2025 at 10:53 AM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@...nel.org> writes:
>> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
>>> On 250819 0027, Benno Lossin wrote:
>>>> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
>>>> > On 250818 1446, Andreas Hindborg wrote:
>>>> >> "Oliver Mangold" <oliver.mangold@...me> writes:
>>>> >> > +impl<T: OwnableMut> DerefMut for Owned<T> {
>>>> >> > +    fn deref_mut(&mut self) -> &mut Self::Target {
>>>> >> > +        // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
>>>> >> > +        // return a mutable reference to it.
>>>> >> > +        unsafe { self.ptr.as_mut() }
>>>> >> > +    }
>>>> >> > +}
>>>> >>
>>>> >> I think someone mentioned this before, but handing out mutable
>>>> >> references can be a problem if `T: !Unpin`. For instance, we don't want
>>>> >> to hand out `&mut Page` in case of `Owned<Page>`.
>>>> >>
>>>> >
>>>> > That was the reason, why `OwnableMut` was introduced in the first place.
>>>> > It's clear, I guess, that as-is it cannot be implemented on many classes.
>>>>
>>>> Yeah the safety requirements ensure that you can't implement it on
>>>> `!Unpin` types.
>>>>
>>>> But I'm not sure it's useful then? As you said there aren't many types
>>>> that will implement the type then, so how about we change the meaning
>>>> and make it give out a pinned mutable reference instead?
>>>
>>> Making `deref_mut()` give out a pinned type won't work. The return types of
>>> deref() are required to match.
>>
>> I meant the changes that Andreas suggested.
>
> Not sure what you are asking, but I need to assert exclusive access to
> an `Page`. I could either get this by taking a `&mut Owned<Page>` or a
> `Pin<&mut Page>`. I think the latter is more agnostic.

The former isn't really correct? It's like having a `&mut Box<Page>`
which is weird. I was saying we can have a `DerefMut` impl gated on `T:
Unpin` and a `fn get_pin_mut(&mut self) -> Pin<&mut T>`.

>>>> > Good question, I have been thinking about it, too. But it might
>>>> > be, that it isn't needed at all. As I understand, usually Rust wrappers
>>>> > are around non-movable C structs. Do we actually have a useful application
>>>> > for OwnableMut?
>>>>
>>>> Also, do we even need two different traits? Which types would only
>>>> implement `Ownable` but not `OwnableMut`?
>>>
>>> I'm not 100% sure, but on a quick glance it looks indeed be safe to
>>> substitute `OwnableMut` by `Unpin`.
>>
>> We just have to change the safety requirements of `OwnableMut`.
>
> `OwnableMut` already requires `Unpin`, it just does not say so directly:
>
>
> /// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
> ///   (i.e. most kernel types).
>
> We could remove this and then just add a trait bound on `Unpin`.

Oh I happened to not have read it that thoroughly then... I don't think
it makes sense to have `OwnableMut` then. So I agree with your suggested
change :)

>>> If we add `get_pin_mut(&mut self) -> Pin<&mut T>` as Andreas suggested,
>>> it would be possible to obtain an `&mut T` anyway, then, if T is `Unpin`.
>>
>> Well the `DerefMut` impl still is convenient in the `Unpin` case.
>
> `OwnableMut` is probably not that useful, since all the types we want to
> implement `Ownable` for is `!Unpin`. We could remove it, but I felt it
> was neat to add the `DerefMut` impl for `Unpin` types.

But we don't need `OwnableMut` in that case?

Let's just do the following if it makes sense:
* remove `OwnableMut`
* allow obtaining a `Pin<&mut T>` from `Owned<T>` via a `&mut self`
  method (we need a new safety requirement for this on `Ownable`)
* have the `DerefMut` impl require `T: Unpin`

---
Cheers,
Benno