| 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
| ||
|
Message-ID: <aV0FxCRzXFrNLZik@tardis-2.local>
Date: Tue, 6 Jan 2026 20:53:24 +0800
From: Boqun Feng <boqun.feng@...il.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Alice Ryhl <aliceryhl@...gle.com>, Will Deacon <will@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Richard Henderson <richard.henderson@...aro.org>,
Matt Turner <mattst88@...il.com>, Magnus Lindholm <linmag7@...il.com>,
Catalin Marinas <catalin.marinas@....com>,
Miguel Ojeda <ojeda@...nel.org>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>, Mark Rutland <mark.rutland@....com>,
FUJITA Tomonori <fujita.tomonori@...il.com>,
Frederic Weisbecker <frederic@...nel.org>, Lyude Paul <lyude@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Anna-Maria Behnsen <anna-maria@...utronix.de>,
John Stultz <jstultz@...gle.com>, Stephen Boyd <sboyd@...nel.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
linux-kernel@...r.kernel.org, linux-alpha@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, rust-for-linux@...r.kernel.org,
linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 2/5] rust: sync: add READ_ONCE and WRITE_ONCE
On Tue, Jan 06, 2026 at 01:29:11PM +0100, Andreas Hindborg wrote:
> "Alice Ryhl" <aliceryhl@...gle.com> writes:
>
> > There are currently a few places in the kernel where we use volatile
> > reads when we really should be using `READ_ONCE`. To make it possible to
> > replace these with proper `READ_ONCE` calls, introduce a Rust version of
> > `READ_ONCE`.
> >
> > I've written the code to use Rust's volatile ops directly when possible.
> > This results in a small amount of code duplication, but I think it makes
> > sense for READ_ONCE and WRITE_ONCE to be implemented in pure Rust when
> > possible. Otherwise they would unconditionally be a function call unless
> > you have a system where you can perform cross-language inlining.
> >
> > I considered these functions in the bindings crate instead of kernel
> > crate. I actually think it would make a lot of sense. But it implies
> > some annoying complications on old compilers since the #![feature()]
> > invocations in kernel/lib.rs do not apply in the bindings crate.
> >
> > For now, we do not support using READ_ONCE on compound types even if
> > they have the right size. This can be added later.
> >
> > This fails checkpatch due to a misordered MAINTAINERS entry, but this is
> > a pre-existing problem.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> > ---
> > MAINTAINERS | 2 +
> > rust/helpers/helpers.c | 1 +
> > rust/helpers/rwonce.c | 34 ++++++++
> > rust/kernel/sync.rs | 2 +
> > rust/kernel/sync/rwonce.rs | 188 +++++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 227 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 12f49de7fe036c2439c00f9f4c67b2219d72a4c3..1d0cae158fe2cc7d99b6a64c11176b635e2d14e4 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -4117,9 +4117,11 @@ F: arch/*/include/asm/atomic*.h
> > F: include/*/atomic*.h
> > F: include/linux/refcount.h
> > F: scripts/atomic/
> > +F: rust/helpers/rwonce.c
> > F: rust/kernel/sync/atomic.rs
> > F: rust/kernel/sync/atomic/
> > F: rust/kernel/sync/refcount.rs
> > +F: rust/kernel/sync/rwonce.rs
> >
> > ATTO EXPRESSSAS SAS/SATA RAID SCSI DRIVER
> > M: Bradley Grove <linuxdrivers@...otech.com>
> > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> > index 79c72762ad9c4b473971e6210c9577860d2e2b08..28b79ca7844fb744e5ad128238824921c055ec82 100644
> > --- a/rust/helpers/helpers.c
> > +++ b/rust/helpers/helpers.c
> > @@ -48,6 +48,7 @@
> > #include "rcu.c"
> > #include "refcount.c"
> > #include "regulator.c"
> > +#include "rwonce.c"
> > #include "scatterlist.c"
> > #include "security.c"
> > #include "signal.c"
> > diff --git a/rust/helpers/rwonce.c b/rust/helpers/rwonce.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..55c621678cd632e728cb925b6a4a2e34e2fc4884
> > --- /dev/null
> > +++ b/rust/helpers/rwonce.c
> > @@ -0,0 +1,34 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * Copyright (C) 2025 Google LLC.
> > + */
> > +
> > +#ifdef CONFIG_ARCH_USE_CUSTOM_READ_ONCE
> > +
> > +__rust_helper u8 rust_helper_read_once_1(const u8 *ptr)
> > +{
> > + return READ_ONCE(*ptr);
> > +}
> > +
> > +__rust_helper u16 rust_helper_read_once_2(const u16 *ptr)
> > +{
> > + return READ_ONCE(*ptr);
> > +}
> > +
> > +__rust_helper u32 rust_helper_read_once_4(const u32 *ptr)
> > +{
> > + return READ_ONCE(*ptr);
> > +}
> > +
> > +__rust_helper u64 rust_helper_read_once_8(const u64 *ptr)
> > +{
> > + return READ_ONCE(*ptr);
> > +}
> > +
> > +__rust_helper void *rust_helper_read_once_ptr(void * const *ptr)
> > +{
> > + return READ_ONCE(*ptr);
> > +}
> > +
> > +#endif
> > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > index 5df87e2bd212e192b8a67644bd99f05b9d4afd75..a5bf7bdc3fa8a044786eafae39fe8844aeeef057 100644
> > --- a/rust/kernel/sync.rs
> > +++ b/rust/kernel/sync.rs
> > @@ -20,6 +20,7 @@
> > pub mod poll;
> > pub mod rcu;
> > mod refcount;
> > +pub mod rwonce;
> > mod set_once;
> >
> > pub use arc::{Arc, ArcBorrow, UniqueArc};
> > @@ -30,6 +31,7 @@
> > pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
> > pub use locked_by::LockedBy;
> > pub use refcount::Refcount;
> > +pub use rwonce::{READ_ONCE, WRITE_ONCE};
> > pub use set_once::SetOnce;
> >
> > /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
> > diff --git a/rust/kernel/sync/rwonce.rs b/rust/kernel/sync/rwonce.rs
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..a1660e43c9ef94011812d1816713cf031a73de1d
> > --- /dev/null
> > +++ b/rust/kernel/sync/rwonce.rs
> > @@ -0,0 +1,188 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +// Copyright (C) 2025 Google LLC.
> > +
> > +//! Rust version of the raw `READ_ONCE`/`WRITE_ONCE` functions.
> > +//!
> > +//! C header: [`include/asm-generic/rwonce.h`](srctree/include/asm-generic/rwonce.h)
> > +
> > +/// Read the pointer once.
> > +///
> > +/// # Safety
> > +///
> > +/// It must be safe to `READ_ONCE` the `ptr` with this type.
> > +#[inline(always)]
> > +#[must_use]
> > +#[track_caller]
> > +#[expect(non_snake_case)]
>
> I really do not think we need to have screaming snake case here. I
> understand that this is what the macro looks like in C code, but we do
> not need to carry that over.
>
We should really do a atomic_load and atomic_store (implemented by
Atomic::from_ptr()) to those usages:
unsafe atomic_load<T: AtomicType, O: AcquireOrRelaxed>(ptr: *mut T, ordering: O) -> T {
Atomic::from_ptr(ptr).load(ordering)
}
unsafe atomic_store<T: AtomicType, O: AcquireOrRelaxed>(ptr: *mut T, v: T, ordering: O) {
Atomic::from_ptr(ptr).store(v, ordering);
}
This unifies the underlying implementation and clarify the atomicity of
these operations.
Regards,
Boqun
>
> Best regards,
> Andreas Hindborg
>
Powered by blists - more mailing lists