linux-kernel - Re: [PATCH v6 8/9] rust: sync: Add memory barriers

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: <aHFWCsOfcGLSUPAP@tardis-2.local>
Date: Fri, 11 Jul 2025 11:20:58 -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>, Ralf Jung <post@...fj.de>
Subject: Re: [PATCH v6 8/9] rust: sync: Add memory barriers

On Fri, Jul 11, 2025 at 10:57:48AM +0200, Benno Lossin wrote:
> On Thu Jul 10, 2025 at 8:00 AM CEST, Boqun Feng wrote:
> > diff --git a/rust/kernel/sync/barrier.rs b/rust/kernel/sync/barrier.rs
> > new file mode 100644
> > index 000000000000..df4015221503
> > --- /dev/null
> > +++ b/rust/kernel/sync/barrier.rs
> > @@ -0,0 +1,65 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Memory barriers.
> > +//!
> > +//! These primitives have the same semantics as their C counterparts: and the precise definitions
> > +//! of semantics can be found at [`LKMM`].
> > +//!
> > +//! [`LKMM`]: srctree/tools/memory-model/
> > +
> > +/// A compiler barrier.
> > +///
> > +/// A barrier that prevents compiler from reordering memory accesses across the barrier.
> > +pub(crate) fn barrier() {
> > +    // By default, Rust inline asms are treated as being able to access any memory or flags, hence
> > +    // it suffices as a compiler barrier.
> 
> I don't know about this, but it also isn't my area of expertise... I
> think I heard Ralf talk about this at Rust Week, but I don't remember...
> 

Easy, let's Cc Ralf ;-)

Ralf, I believe the question here is:

In kernel C, we define a compiler barrier (barrier()), which is
implemented as:

# define barrier() __asm__ __volatile__("": : :"memory")

Now we want to have a Rust version, and I think an empty `asm!()` should
be enough as an equivalent as a barrier() in C, because an empty
`asm!()` in Rust implies "memory" as the clobber:

	https://godbolt.org/z/3z3fnWYjs

?

I know you have some opinions on C++ compiler_fence() [1]. But in LKMM,
barrier() and other barriers work for all memory accesses not just
atomics, so the problem "So, if your program contains no atomic
accesses, but some atomic fences, those fences do nothing." doesn't
exist for us. And our barrier() is strictly weaker than other barriers.

And based on my understanding of the consensus on Rust vs LKMM, "do
whatever kernel C does and rely on whatever kernel C relies" is the
general suggestion, so I think an empty `asm!()` works here. Of course
if in practice, we find an issue, I'm happy to look for solutions ;-)

Thoughts?

[1]: https://github.com/rust-lang/unsafe-code-guidelines/issues/347

Regards,
Boqun

> > +    //
> > +    // SAFETY: An empty asm block should be safe.
> 
>     // SAFETY: An empty asm block.
> 
> > +    unsafe {
> > +        core::arch::asm!("");
> > +    }
> 
>     unsafe { core::arch::asm!("") };
> 
> > +}
> > +
> > +/// A full memory barrier.
> > +///
> > +/// A barrier that prevents compiler and CPU from reordering memory accesses across the barrier.
> > +pub fn smp_mb() {
> > +    if cfg!(CONFIG_SMP) {
> > +        // SAFETY: `smp_mb()` is safe to call.
> > +        unsafe {
> > +            bindings::smp_mb();
> 
> Does this really work? How does the Rust compiler know this is a memory
> barrier?
> 
> ---
> Cheers,
> Benno
> 
> > +        }
> > +    } else {
> > +        barrier();
> > +    }
> > +}