On Thu, 28 May 2026 13:23:39 +0100 Gary Guo <[email protected]> wrote:
> On Thu May 28, 2026 at 9:35 AM BST, Onur Özkan wrote: > > On Thu, 28 May 2026 11:20:10 +0300 > > Onur Özkan <[email protected]> wrote: > > > >> On Thu, 28 May 2026 09:27:35 +0300 > >> Onur Özkan <[email protected]> wrote: > >> > >> > Add a Rust abstraction for sleepable RCU (SRCU), backed by C srcu_struct. > >> > Provide FFI helpers and a safe wrapper with a guard-based API for > >> > read-side > >> > critical sections. > >> > > >> > Cleanup is handled via `PinnedDrop`, which explicitly drains pending > >> > grace > >> > periods and callbacks via `synchronize_srcu` and `srcu_barrier` before > >> > executing `cleanup_srcu_struct` to guarantee memory safety e.g. when > >> > there > >> > are leaked guards (via `mem::forget($guard)`). > >> > > >> > Signed-off-by: Onur Özkan <[email protected]> > >> > --- > >> > rust/kernel/sync.rs | 2 + > >> > rust/kernel/sync/srcu.rs | 166 +++++++++++++++++++++++++++++++++++++++ > >> > 2 files changed, 168 insertions(+) > >> > create mode 100644 rust/kernel/sync/srcu.rs > >> > > >> > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs > >> > index 993dbf2caa0e..0d6a5f1300c3 100644 > >> > --- a/rust/kernel/sync.rs > >> > +++ b/rust/kernel/sync.rs > >> > @@ -21,6 +21,7 @@ > >> > pub mod rcu; > >> > mod refcount; > >> > mod set_once; > >> > +pub mod srcu; > >> > > >> > pub use arc::{Arc, ArcBorrow, UniqueArc}; > >> > pub use completion::Completion; > >> > @@ -31,6 +32,7 @@ > >> > pub use locked_by::LockedBy; > >> > pub use refcount::Refcount; > >> > pub use set_once::SetOnce; > >> > +pub use srcu::Srcu; > >> > > >> > /// Represents a lockdep class. > >> > /// > >> > diff --git a/rust/kernel/sync/srcu.rs b/rust/kernel/sync/srcu.rs > >> > new file mode 100644 > >> > index 000000000000..343f00d070c7 > >> > --- /dev/null > >> > +++ b/rust/kernel/sync/srcu.rs > >> > @@ -0,0 +1,166 @@ > >> > +// SPDX-License-Identifier: GPL-2.0 > >> > + > >> > +//! Sleepable read-copy update (SRCU) support. > >> > +//! > >> > +//! C header: [`include/linux/srcu.h`](srctree/include/linux/srcu.h) > >> > + > >> > +use crate::{ > >> > + bindings, > >> > + error::to_result, > >> > + prelude::*, > >> > + sync::LockClassKey, > >> > + types::{ > >> > + NotThreadSafe, > >> > + Opaque, // > >> > + }, > >> > +}; > >> > + > >> > +use pin_init::pin_data; > >> > + > >> > +/// Creates an [`Srcu`] initialiser with the given name and a > >> > newly-created lock class. > >> > +#[doc(hidden)] > >> > +#[macro_export] > >> > +macro_rules! new_srcu { > >> > + ($($name:literal)?) => { > >> > + $crate::sync::Srcu::new($crate::optional_name!($($name)?), > >> > $crate::static_lock_class!()) > >> > + }; > >> > +} > >> > +pub use new_srcu; > >> > + > >> > +/// Sleepable read-copy update primitive. > >> > +/// > >> > +/// SRCU readers may sleep while holding the read-side guard. > >> > +/// > >> > +/// The destructor waits for active readers and callbacks, so it may > >> > sleep. > >> > +/// If a read-side guard has been leaked, dropping an [`Srcu`] may > >> > never return. > >> > +/// > >> > +/// # Invariants > >> > +/// > >> > +/// This represents a valid `struct srcu_struct` initialized by the C > >> > SRCU API > >> > +/// and it remains pinned and valid until the pinned destructor runs. > >> > +#[repr(transparent)] > >> > +#[pin_data(PinnedDrop)] > >> > +pub struct Srcu { > >> > + #[pin] > >> > + inner: Opaque<bindings::srcu_struct>, > >> > +} > >> > + > >> > +impl Srcu { > >> > + /// Creates a new SRCU instance. > >> > + #[inline] > >> > + pub fn new(name: &'static CStr, key: Pin<&'static LockClassKey>) -> > >> > impl PinInit<Self, Error> { > >> > + try_pin_init!(Self { > >> > + // INVARIANT: On success, the C initializer creates a valid > >> > `srcu_struct` and > >> > + // it remains pinned until `PinnedDrop` runs. > >> > + inner <- Opaque::try_ffi_init(|ptr: *mut > >> > bindings::srcu_struct| { > >> > + // SAFETY: `ptr` points to valid uninitialised memory > >> > for a `srcu_struct`. > >> > + to_result(unsafe { > >> > + bindings::init_srcu_struct_with_key(ptr, > >> > name.as_char_ptr(), key.as_ptr()) > >> > + }) > >> > + }), > >> > + }) > >> > + } > >> > + > >> > + /// Enters an SRCU read-side critical section. > >> > + /// > >> > + /// Leaking the returned [`Guard`] leaves the SRCU read-side > >> > critical > >> > + /// section active and makes `drop` sleep forever. > >> > + #[inline] > >> > + pub fn read_lock(&self) -> Guard<'_> { > >> > + // SAFETY: By the type invariants, `self` contains a valid > >> > `struct srcu_struct`. > >> > + let idx = unsafe { bindings::srcu_read_lock(self.inner.get()) }; > >> > + > >> > + // INVARIANT: `idx` was returned by `srcu_read_lock()` for this > >> > `Srcu`. > >> > + Guard { > >> > + srcu: self, > >> > + idx, > >> > + _not_send: NotThreadSafe, > >> > + } > >> > + } > >> > + > >> > + /// Waits until all pre-existing SRCU readers have completed. > >> > + #[inline] > >> > + pub fn synchronize(&self) { > >> > + // SAFETY: By the type invariants, `self` contains a valid > >> > `struct srcu_struct`. > >> > + unsafe { bindings::synchronize_srcu(self.inner.get()) }; > >> > + } > >> > + > >> > + /// Waits until all pre-existing SRCU readers have completed, > >> > expedited. > >> > + /// > >> > + /// This requests a lower-latency grace period than > >> > [`Srcu::synchronize`] typically > >> > + /// at the cost of higher system-wide overhead. Prefer > >> > [`Srcu::synchronize`] by default > >> > + /// and use this variant only when reducing reset or teardown > >> > latency is more important > >> > + /// than the extra cost. > >> > + #[inline] > >> > + pub fn synchronize_expedited(&self) { > >> > + // SAFETY: By the type invariants, `self` contains a valid > >> > `struct srcu_struct`. > >> > + unsafe { bindings::synchronize_srcu_expedited(self.inner.get()) > >> > }; > >> > + } > >> > +} > >> > + > >> > +#[pinned_drop] > >> > +impl PinnedDrop for Srcu { > >> > + fn drop(self: Pin<&mut Self>) { > >> > + let ptr = self.inner.get(); > >> > + > >> > + // SAFETY: By the type invariants, `self` contains a valid and > >> > pinned `struct srcu_struct` > >> > + // and `srcu_readers_active()` only checks the active reader > >> > count. > >> > + if unsafe { bindings::srcu_readers_active(ptr) } { > >> > + crate::pr_warn!( > >> > + "Leaked `Guard` detected while dropping SRCU; drop will > >> > block forever.\n" > >> > + ); > > I think this could be a `warn_on` similar to how cleanup_srcu_struct handle > the > condition. We also call cleanup_srcu_struct below. The idea was to provide additional information, we don't need to call warn_on twice. > > >> > + } > >> > + > >> > + // `cleanup_srcu_struct()` may return early if readers are > >> > still active. Because `Srcu` > >> > + // owns the embedded `srcu_struct`, returning from `drop` in > >> > that state could free memory > >> > + // that is still referenced by the C side. > >> > + // > >> > + // Wait for all readers to complete first. If any `Guard` was > >> > leaked, `synchronize_srcu()` > >> > + // will sleep forever. > >> > + // > >> > + // SAFETY: By the type invariants, `self` contains a valid and > >> > pinned `struct srcu_struct`. > >> > + unsafe { bindings::synchronize_srcu(ptr) }; > >> > >> Sashiko got a good point here which is calling synchronize_srcu() only if > >> there > >> are active readers. That's a nice low-effort improvement we can have in > >> the next > >> version. > >> > >> Onur > > > > Actually, now I am now thinking about whether we can come up with a better > > approach when we detect leaked guards. Initially I came up with the > > synchronize_srcu() solution because it would handle leaked guards > > automatically > > without requiring any additional checks. But now that we can actually detect > > whether guards are leaked the question becomes: > > > > "Is there a better option than effectively sleeping forever when leaked > > guards are detected?" > > > > I have no plans for tomorrow other than finalizing this series including the > > question above. > > The best solution is to proceed cleanups anyway, given Rust rules ensure that > these are actual leaks and not just srcu read-side critical section that > failed > to synchronize with the destruction of SRCU. > > This obviously require changes to the SRCU code though. The issue is difficult to fix purely from the C side. Once drop returns Rust is free to destroy srcu_struct. If srcu still has pending callback associated with that srcu_struct, for example from a future call_srcu() wrapper then returning from drop while readers are active can turn into a UAF. There is also no way to handle callbacks in a reasonable way in cleanup logic while there are active readers. I mean in theory this could be fixed in the C code, but that would require to re-write srcu cores/semantics for this special case. The $clean_something helper would need know that the active readers are abandoned and will never unlock and it would also need to decide what to do with the pending callbacks, which is also a big problem (as gp will never complete, callbacks will never run). It's also worth to note that calling mem::forget on the srcu guard is WRONG CODE and very easy to catch on review (by us and also Sashiko/any LLM). So finding a solution that doesn't add too much complexity should be a key consideration here. With that in mind, keeping the synchronize_srcu() not really a bad solution. Sleeping forever is a bad failure mode, but it is better than a potential UAF and either case requires sending a fix patch for the leaked guard anyway. - Onur > > Best, > Gary >
