linux-kernel - Re: [PATCH v10 5/5] rust: Add OwnableRefCounted and SimpleOwnableRefCounted

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: <aBr7dm_L7JEQIVxC@google.com>
Date: Wed, 7 May 2025 06:19:34 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: 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>, Benno Lossin <benno.lossin@...ton.me>, 
	Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>, 
	Asahi Lina <lina@...hilina.net>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 5/5] rust: Add OwnableRefCounted and SimpleOwnableRefCounted

On Tue, May 06, 2025 at 11:42:08AM +0000, Oliver Mangold wrote:
> On 250502 1143, Alice Ryhl wrote:
> > On Fri, May 02, 2025 at 09:03:04AM +0000, Oliver Mangold wrote:
> > > +pub unsafe trait SimpleOwnableRefCounted: RefCounted {
> > > +    /// Checks if exactly one [`ARef`] to the object exists. In case the object is [`Sync`], the
> > > +    /// check needs to be race-free.
> > > +    fn is_unique(&self) -> bool;
> > > +}
> > > +
> > > +#[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
> > > +// SAFETY: Safe by the requirements on implementation of [`SimpleOwnableRefCounted`].
> > > +unsafe impl<T: SimpleOwnableRefCounted> OwnableRefCounted for T {
> > > +    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
> > > +        if T::is_unique(&*this) {
> > > +            // SAFETY: Safe by the requirements on implementation of [`SimpleOwnable`].
> > > +            Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
> > > +        } else {
> > > +            Err(this)
> > > +        }
> > > +    }
> > > +}
> > > +
> > > +#[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
> > > +// SAFETY: Safe by the requirements on implementation of [`SimpleOwnableRefCounted`].
> > > +unsafe impl<T: SimpleOwnableRefCounted> Ownable for T {
> > > +    unsafe fn release(this: NonNull<Self>) {
> > > +        // SAFETY: Safe by the requirements on implementation of
> > > +        // [`SimpleOwnableRefCounted::dec_ref()`].
> > > +        unsafe { RefCounted::dec_ref(this) };
> > > +    }
> > > +}
> > 
> > I wonder if this is too limiting. It will limit our ability to write
> > other blanket impls for Ownable and OwnableRefCounted. Using e.g. a
> > derive macro might be better?
> > 
> > Alice
> 
> You might be right. I don't have a strong opinion on the matter.
> 
> I'm not really familiar with procmacros, though. So maybe I should just do
> away with SimpleOwnableRefCounted for now?
> 
> I mean Andreas doesn't need it for his current use case and this or a derive
> macro can always be added later if it makes sense.

That's okay for now.

Alice