linux-kernel - Re: [PATCH v5 04/10] rust: sync: atomic: Add generic atomics

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: <aFmmYSAyvxotYfo7@tardis.local>
Date: Mon, 23 Jun 2025 12:09:21 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Gary Guo <gary@...yguo.net>
Cc: linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	lkmm@...ts.linux.dev, linux-arch@...r.kernel.org,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <lossin@...nel.org>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>, Will Deacon <will@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Mark Rutland <mark.rutland@....com>,
	Wedson Almeida Filho <wedsonaf@...il.com>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Lyude Paul <lyude@...hat.com>, Ingo Molnar <mingo@...nel.org>,
	Mitchell Levy <levymitchell0@...il.com>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v5 04/10] rust: sync: atomic: Add generic atomics

On Mon, Jun 23, 2025 at 07:30:19PM +0100, Gary Guo wrote:
> On Sun, 22 Jun 2025 22:19:44 -0700
> Boqun Feng <boqun.feng@...il.com> wrote:
> 
> > On Sat, Jun 21, 2025 at 12:32:12PM +0100, Gary Guo wrote:
> > [...]
> > > > +#[repr(transparent)]
> > > > +pub struct Atomic<T: AllowAtomic>(Opaque<T>);  
> > > 
> > > This should store `Opaque<T::Repr>` instead.
> > >   
> > 
> > "should" is a strong word ;-) If we still use `into_repr`/`from_repr`
> > it's a bit impossible, because Atomic::new() wants to be a const
> > function, so it requires const_trait_impl I believe.
> > 
> > If we require transmutability as a safety requirement for `AllowAtomic`,
> > then either `T` or `T::Repr` is fine.
> > 
> > > The implementation below essentially assumes that this is
> > > `Opaque<T::Repr>`:
> > > * atomic ops cast this to `*mut T::Repr`
> > > * load/store operates on `T::Repr` then converts to `T` with
> > >   `T::from_repr`/`T::into_repr`.
> > >   
> > 
> > Note that we only require one direction of strong transmutability, that
> > is: for every `T`, it must be able to safe transmute to a `T::Repr`, for
> > `T::Repr` -> `T` transmutation, only if it's a result of a `transmute<T,
> > T::Repr>()`. This is mostly due to potential support for unit-only enum.  
> > E.g. using an atomic variable to represent a finite state.
> > 
> > > Note tha the transparent new types restriction on `AllowAtomic` is not
> > > sufficient for this, as I can define
> > >   
> > 
> > Nice catch! I do agree we should disallow `MyWeirdI32`, and I also agree
> > that we should put transmutability as safety requirement for
> > `AllowAtomic`. However, I would suggest we still keep
> > `into_repr`/`from_repr`, and require the implementation to make them
> > provide the same results as transmute(), as a correctness precondition
> > (instead of a safety precondition), in other words, you can still write
> > a `MyWeirdI32`, and it won't cause safety issues, but it'll be
> > incorrect.
> > 
> > The reason why I think we should keep `into_repr`/`from_repr` but add
> > a correctness precondition is that they are easily to implement as safe
> > code for basic types, so it'll be better than a transmute() call. Also
> > considering `Atomic<*mut T>`, would transmuting between integers and
> > pointers act the same as expose_provenance() and
> > from_exposed_provenance()?
> 
> Okay, this is more problematic than I thought then. For pointers, you

Welcome to my nightmare ;-)

> cannot just transmute between from pointers to usize (which is its
> Repr):
> * Transmuting from pointer to usize discards provenance
> * Transmuting from usize to pointer gives invalid provenance
> 
> We want neither behaviour, so we must store `usize` directly and
> always call into repr functions.
> 

If we store `usize`, how can we support the `get_mut()` then? E.g.

    static V: i32 = 32;

    let mut x = Atomic::new(&V as *const i32 as *mut i32);
    // ^ assume we expose_provenance() in new().

    let ptr: &mut *mut i32 = x.get_mut(); // which is `&mut self.0.get()`.

    let ptr_val = *ptr; // Does `ptr_val` have the proper provenance?

> To make things cost I guess you would need an extra trait to indicate
> that transmuting is fine.

Could you maybe provide an example?

Regards,
Boqun

> 
> Best,
> Gary