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

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

Message-ID: <87ldnfd766.fsf@t14s.mail-host-address-is-not-set>
Date: Tue, 19 Aug 2025 10:53:05 +0200
From: Andreas Hindborg <a.hindborg@...nel.org>
To: Benno Lossin <lossin@...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

"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.

>
>>> > 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`.

>
>> 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.


Best regards,
Andreas Hindborg