linux-kernel - Re: [PATCH v6 4/9] 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: <aHEWze8p40qeNBr_@Mac.home>
Date: Fri, 11 Jul 2025 06:51:09 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Benno Lossin <lossin@...nel.org>
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>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	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>,
	Alan Stern <stern@...land.harvard.edu>
Subject: Re: [PATCH v6 4/9] rust: sync: atomic: Add generic atomics

On Fri, Jul 11, 2025 at 03:34:47PM +0200, Benno Lossin wrote:
> On Fri Jul 11, 2025 at 3:22 PM CEST, Boqun Feng wrote:
> > On Fri, Jul 11, 2025 at 10:03:07AM +0200, Benno Lossin wrote:
> > [...]
> >> > +
> >> > +    /// Returns a pointer to the underlying atomic variable.
> >> > +    ///
> >> > +    /// Extra safety requirement on using the return pointer: the operations done via the pointer
> >> > +    /// cannot cause data races defined by [`LKMM`].
> >> 
> >> I don't think this is correct. I could create an atomic and then share
> >> it with the C side via this function, since I have exclusive access, the
> >> writes to this pointer don't need to be atomic.
> >> 
> >
> > that's why it says "the operations done via the pointer cannot cause
> > data races .." instead of saying "it must be atomic".
> 
> Ah right I misread... But then the safety requirement is redundant? Data
> races are already UB...
> 
> >> We also don't document additional postconditions like this... If you
> >
> > Please see how Rust std document their `as_ptr()`:
> >
> > 	https://doc.rust-lang.org/std/sync/atomic/struct.AtomicI32.html#method.as_ptr
> >
> > It mentions that "Doing non-atomic reads and writes on the resulting
> > integer can be a data race." (although the document is a bit out of
> > date, since non-atomic read and atomic read are no longer data race now,
> > see [1])
> 
> That's very different from the comment you wrote though. It's not an
> additional safety requirement, but rather a note to users of the API
> that they should be careful with the returned pointer.
> 
> > I think we can use the similar document structure here: providing more
> > safety requirement on the returning pointers, and...
> >
> >> really would have to do it like this (which you shouldn't given the
> >> example above), you would have to make this function `unsafe`, otherwise
> >> there is no way to ensure that people adhere to it (since it isn't part
> >> of the safety docs).
> >> 
> >
> > ...since dereferencing pointers is always `unsafe`, users need to avoid
> > data races anyway, hence this is just additional information that helps
> > reasoning.
> 
> I disagree.
> 
> As mentioned above, data races are already forbidden for raw pointers.
> We should indeed add a note that says that non-atomic operations might
> result in data races. But that's very different from adding an
> additional safety requirement for using the pointer.
> 
> And I don't think that we can add additional safety requirements to
> dereferencing a raw pointer without an additional `unsafe` block.
> 

So all your disagreement is about the "extra safety requirement" part?
How about I drop that:

    /// Returns a pointer to the underlying atomic `T`.
    pub const fn as_ptr(&self) -> *mut T {
        self.0.get()
    }

? I tried to add something additional information:

/// Note that non-atomic reads and writes via the returned pointer may
/// cause data races if racing with atomic reads and writes per [LKMM].

but that seems redundant, because as you said, data races are UB anyway.

Thoughts?

Regards,
Boqun

> ---
> Cheers,
> Benno