[PATCH v2 0/3] rust: add SRCU abstraction

[PATCH v2 0/3] rust: add SRCU abstraction

[Onur Özkan]

The immediate motivation is the Tyr reset infrastructure [1] which needs
to serialize reset sensitive hardware access against reset and teardown
paths. That reset series started to require many independent dependencies
so this SRCU support is split out as a standalone Rust API to keep the
reset series focused on the reset logic and easier to review, rebase and
land.

[1]: https://lore.kernel.org/all/20260416171728.205141-1-work@onurozkan.dev

Changes since v1:

- Made the owned SRCU read-side guard API unsafe and added a safe closure
  based helper for callers that do not need to keep the guard. This is to
  avoid UB on the C side cleanup_srcu_struct where the SRCU struct is freed
  while there are still active guards, which can happen if the caller leaks
  the guard e.g., with mem::forget().
- Improved doc comments.

v1: https://lore.kernel.org/all/20260428103437.156236-1-work@onurozkan.dev

Onur Özkan (3):
  rust: helpers: add SRCU helpers
  rust: sync: add SRCU abstraction
  MAINTAINERS: add Rust SRCU files to SRCU entry

 MAINTAINERS              |   3 +
 rust/helpers/helpers.c   |   1 +
 rust/helpers/srcu.c      |  24 +++++++
 rust/kernel/sync.rs      |   2 +
 rust/kernel/sync/srcu.rs | 152 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 182 insertions(+)
 create mode 100644 rust/helpers/srcu.c
 create mode 100644 rust/kernel/sync/srcu.rs

-- 
2.51.2

[PATCH v2 1/3] rust: helpers: add SRCU helpers

[Onur Özkan]

Add helper wrappers for SRCU functions that are exposed to Rust
through generated bindings.

Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 rust/helpers/helpers.c |  1 +
 rust/helpers/srcu.c    | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)
 create mode 100644 rust/helpers/srcu.c

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 875a9788ad40..052fef89d5f0 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -60,6 +60,7 @@
 #include "signal.c"
 #include "slab.c"
 #include "spinlock.c"
+#include "srcu.c"
 #include "sync.c"
 #include "task.c"
 #include "time.c"
diff --git a/rust/helpers/srcu.c b/rust/helpers/srcu.c
new file mode 100644
index 000000000000..e9f723d7f8c9
--- /dev/null
+++ b/rust/helpers/srcu.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/srcu.h>
+
+__rust_helper int rust_helper_init_srcu_struct_with_key(struct srcu_struct *ssp,
+							const char *name,
+							struct lock_class_key *key)
+{
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	return __init_srcu_struct(ssp, name, key);
+#else /* !CONFIG_DEBUG_LOCK_ALLOC */
+	return init_srcu_struct(ssp);
+#endif /* CONFIG_DEBUG_LOCK_ALLOC */
+}
+
+__rust_helper int rust_helper_srcu_read_lock(struct srcu_struct *ssp)
+{
+	return srcu_read_lock(ssp);
+}
+
+__rust_helper void rust_helper_srcu_read_unlock(struct srcu_struct *ssp, int idx)
+{
+	srcu_read_unlock(ssp, idx);
+}
-- 
2.51.2

[PATCH v2 2/3] rust: sync: add SRCU abstraction

[Onur Özkan]

Add a Rust abstraction for sleepable RCU.

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.

Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 rust/kernel/sync.rs      |   2 +
 rust/kernel/sync/srcu.rs | 152 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 154 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..7bd713e96375
--- /dev/null
+++ b/rust/kernel/sync/srcu.rs
@@ -0,0 +1,152 @@
+// 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.
+#[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 may sleep.
+///
+/// # 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 {
+            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.
+    ///
+    /// # Safety
+    ///
+    /// The returned [`Guard`] must not be leaked. Leaking it with [`core::mem::forget`]
+    /// leaves the SRCU read-side critical section active.
+    #[inline]
+    pub unsafe 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,
+        }
+    }
+
+    /// Runs `f` in an SRCU read-side critical section.
+    #[inline]
+    pub fn with_read_lock<T>(&self, f: impl FnOnce(&Guard<'_>) -> T) -> T {
+        // SAFETY: The guard is owned within this function and is not leaked.
+        let guard = unsafe { self.read_lock() };
+
+        f(&guard)
+    }
+
+    /// 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`.
+        unsafe { bindings::srcu_barrier(ptr) };
+        // SAFETY: Same as above.
+        unsafe { bindings::cleanup_srcu_struct(ptr) };
+    }
+}
+
+// SAFETY: `srcu_struct` may be shared and used across threads.
+unsafe impl Send for Srcu {}
+// SAFETY: `srcu_struct` may be shared and used concurrently.
+unsafe impl Sync for Srcu {}
+
+/// Guard for an active SRCU read-side critical section on a particular [`Srcu`].
+///
+/// Leaking this guard with [`core::mem::forget`] leaves the SRCU read-side
+/// critical section active.
+///
+/// # Invariants
+///
+/// `idx` is the index returned by `srcu_read_lock()` for `srcu`.
+pub struct Guard<'a> {
+    srcu: &'a Srcu,
+    idx: i32,
+    _not_send: NotThreadSafe,
+}
+
+impl Guard<'_> {
+    /// Explicitly releases the SRCU read-side critical section.
+    #[inline]
+    pub fn unlock(self) {}
+}
+
+impl Drop for Guard<'_> {
+    #[inline]
+    fn drop(&mut self) {
+        // SAFETY: `Guard` is only constructible through `Srcu::read_lock()`,
+        // which returns a valid index for the SRCU instance.
+        unsafe { bindings::srcu_read_unlock(self.srcu.inner.get(), self.idx) };
+    }
+}
-- 
2.51.2

[PATCH v2 3/3] MAINTAINERS: add Rust SRCU files to SRCU entry

[Onur Özkan]

Include Rust side implementation files to the SRCU maintainer
entry.

Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9c93fa23654c..8c1588e2f6fe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24405,6 +24405,7 @@ SLEEPABLE READ-COPY UPDATE (SRCU)
 M:	Lai Jiangshan <jiangshanlai@gmail.com>
 M:	"Paul E. McKenney" <paulmck@kernel.org>
 M:	Josh Triplett <josh@joshtriplett.org>
+M:	Onur Özkan <work@onurozkan.dev> (RUST)
 R:	Steven Rostedt <rostedt@goodmis.org>
 R:	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
 L:	rcu@vger.kernel.org
@@ -24413,6 +24414,8 @@ W:	http://www.rdrop.com/users/paulmck/RCU/
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/rcu/linux.git rcu/dev
 F:	include/linux/srcu*.h
 F:	kernel/rcu/srcu*.c
+F:	rust/helpers/srcu.c
+F:	rust/kernel/sync/srcu.rs
 
 SMACK SECURITY MODULE
 M:	Casey Schaufler <casey@schaufler-ca.com>
-- 
2.51.2

Re: [PATCH v2 2/3] rust: sync: add SRCU abstraction

[Gary Guo]

On Sat May 2, 2026 at 5:27 PM BST, Onur Özkan wrote:
> Add a Rust abstraction for sleepable RCU.
>
> 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.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
>  rust/kernel/sync.rs      |   2 +
>  rust/kernel/sync/srcu.rs | 152 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 154 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..7bd713e96375
> --- /dev/null
> +++ b/rust/kernel/sync/srcu.rs
> @@ -0,0 +1,152 @@
> +// 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.
> +#[macro_export]

#[doc(hidden)]

Given this is a new macro, let's not encourage user from using it from crate
root.

> +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 may sleep.
> +///
> +/// # 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 {
> +            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.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The returned [`Guard`] must not be leaked. Leaking it with [`core::mem::forget`]
> +    /// leaves the SRCU read-side critical section active.

I generally would prefer if we could use guard-like API instead of forcing a
callback.

I suppose the only reason that this is unsafe is the "just leak it" condition
when cleaning up SRCU struct, which skips cleaning up delayed work, which could
call into `process_srcu`, which accesses `srcu_struct`. This however is *not*
leaked, because it's controlled by the user. Only the auxillary data allocated
by SRCU is leaked. So UAF is going to happen.

So in some aspect, the leak caused by `srcu_readers_active(ssp)` can cause more
damage compared to just continuing cleanup despite active users? I think this
could be changed in one of these ways:
* Have SRCU allocate all memory instead, and user-side would just have a
  `struct srcu_struct*`; then leaking would be safe. This is probably a bit
  difficult to change because it affects many users.
* Continue to flush delayed work and stop the timer, and then leak before the
  actual kfree happens.
* Trigger a `BUG` when the leak condition is hit for Rust users.
* Declare the `WARN_ON` to be a sufficient protection and say this can be
  considered safe. Kinda similar to the strategy we take to the
  sleep-inside-atomic context issue.

> +    #[inline]
> +    pub unsafe 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,
> +        }
> +    }
> +
> +    /// Runs `f` in an SRCU read-side critical section.
> +    #[inline]
> +    pub fn with_read_lock<T>(&self, f: impl FnOnce(&Guard<'_>) -> T) -> T {
> +        // SAFETY: The guard is owned within this function and is not leaked.
> +        let guard = unsafe { self.read_lock() };
> +
> +        f(&guard)
> +    }
> +
> +    /// 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();
> +

This needs a comment on why this is invoked. Something like:

    // Ensure all SRCU callbacks have been finished before freeing.

Best,
Gary

> +        // SAFETY: By the type invariants, `self` contains a valid and pinned `struct srcu_struct`.
> +        unsafe { bindings::srcu_barrier(ptr) };
> +        // SAFETY: Same as above.
> +        unsafe { bindings::cleanup_srcu_struct(ptr) };
> +    }
> +}
> +
> +// SAFETY: `srcu_struct` may be shared and used across threads.
> +unsafe impl Send for Srcu {}
> +// SAFETY: `srcu_struct` may be shared and used concurrently.
> +unsafe impl Sync for Srcu {}

Re: [PATCH v2 2/3] rust: sync: add SRCU abstraction

[Onur Özkan]

On Sat, 02 May 2026 18:55:56 +0100
Gary Guo <gary@garyguo.net> wrote:

> On Sat May 2, 2026 at 5:27 PM BST, Onur Özkan wrote:
> > Add a Rust abstraction for sleepable RCU.
> >
> > 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.
> >
> > Signed-off-by: Onur Özkan <work@onurozkan.dev>
> > ---
> >  rust/kernel/sync.rs      |   2 +
> >  rust/kernel/sync/srcu.rs | 152 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 154 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..7bd713e96375
> > --- /dev/null
> > +++ b/rust/kernel/sync/srcu.rs
> > @@ -0,0 +1,152 @@
> > +// 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.
> > +#[macro_export]
> 
> #[doc(hidden)]
> 
> Given this is a new macro, let's not encourage user from using it from crate
> root.
> 
> > +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 may sleep.
> > +///
> > +/// # 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 {
> > +            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.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// The returned [`Guard`] must not be leaked. Leaking it with [`core::mem::forget`]
> > +    /// leaves the SRCU read-side critical section active.
> 
> I generally would prefer if we could use guard-like API instead of forcing a
> callback.

Me too and developers can still do that. I think the safety contract here is
very simple to handle. It's essentially this:

	// SAFETY: Guard is not leaked.
	let _guard = unsafe { x.read_lock() };

To me it's very simple and straightforward for both the developer and the
reviewer. It doesn't add any overhead to the implementation and it ensures
that the developer (and later the reviewer) is aware of the potential issue.

Of course, there's also the safe option if the developer is happy with
closure-based API:

	x.with_read_lock(|_guard| {
		...
	});

So it allows you to use the guard-based approach directly with the requirement
of a safety comment and also provides a safe API for developers who don't want
to deal with that. I am not sure if you fall into the third category, which is
"I don't like writing safety comments and I don't like the closure-based
approach" :)

> 
> I suppose the only reason that this is unsafe is the "just leak it" condition
> when cleaning up SRCU struct, which skips cleaning up delayed work, which could
> call into `process_srcu`, which accesses `srcu_struct`. This however is *not*
> leaked, because it's controlled by the user. Only the auxillary data allocated
> by SRCU is leaked. So UAF is going to happen.
> 
> So in some aspect, the leak caused by `srcu_readers_active(ssp)` can cause more
> damage compared to just continuing cleanup despite active users? I think this
> could be changed in one of these ways:
> * Have SRCU allocate all memory instead, and user-side would just have a
>   `struct srcu_struct*`; then leaking would be safe. This is probably a bit
>   difficult to change because it affects many users.

We could do the same on the Rust side only. Basically instead of embedding
srcu_struct in Rust srcu, allocate it separately and store its pointer. Then,
if cleanup hits the active reader case, we could leak that allocation so any
remaining srcu work does not hit UAF. I was aware of this option but I would
prefer to avoid it because it adds an extra allocation for every Rust srcu.

> * Continue to flush delayed work and stop the timer, and then leak before the
>   actual kfree happens.

Can you say more? I didn't understand this particular solution.

> * Trigger a `BUG` when the leak condition is hit for Rust users.

We need an atomic counter to detect the leak and I thought that would be too
much overhead for this abstraction. Basically each lock and drop will call an
atomic operation so.

> * Declare the `WARN_ON` to be a sufficient protection and say this can be
>   considered safe. Kinda similar to the strategy we take to the
>   sleep-inside-atomic context issue.

I think this is a rather weak precaution.

> 
> > +    #[inline]
> > +    pub unsafe 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,
> > +        }
> > +    }
> > +
> > +    /// Runs `f` in an SRCU read-side critical section.
> > +    #[inline]
> > +    pub fn with_read_lock<T>(&self, f: impl FnOnce(&Guard<'_>) -> T) -> T {
> > +        // SAFETY: The guard is owned within this function and is not leaked.
> > +        let guard = unsafe { self.read_lock() };
> > +
> > +        f(&guard)
> > +    }
> > +
> > +    /// 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();
> > +
> 
> This needs a comment on why this is invoked. Something like:
> 
>     // Ensure all SRCU callbacks have been finished before freeing.
> 
> Best,
> Gary
> 
> > +        // SAFETY: By the type invariants, `self` contains a valid and pinned `struct srcu_struct`.
> > +        unsafe { bindings::srcu_barrier(ptr) };
> > +        // SAFETY: Same as above.
> > +        unsafe { bindings::cleanup_srcu_struct(ptr) };
> > +    }
> > +}
> > +
> > +// SAFETY: `srcu_struct` may be shared and used across threads.
> > +unsafe impl Send for Srcu {}
> > +// SAFETY: `srcu_struct` may be shared and used concurrently.
> > +unsafe impl Sync for Srcu {}

Re: [PATCH v2 2/3] rust: sync: add SRCU abstraction

[Gary Guo]

On Sun May 3, 2026 at 4:39 AM BST, Onur Özkan wrote:
>> > +/// Sleepable read-copy update primitive.
>> > +///
>> > +/// SRCU readers may sleep while holding the read-side guard.
>> > +///
>> > +/// The destructor may sleep.
>> > +///
>> > +/// # 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 {
>> > +            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.
>> > +    ///
>> > +    /// # Safety
>> > +    ///
>> > +    /// The returned [`Guard`] must not be leaked. Leaking it with [`core::mem::forget`]
>> > +    /// leaves the SRCU read-side critical section active.
>> 
>> I generally would prefer if we could use guard-like API instead of forcing a
>> callback.
>
> Me too and developers can still do that. I think the safety contract here is
> very simple to handle. It's essentially this:
>
> 	// SAFETY: Guard is not leaked.
> 	let _guard = unsafe { x.read_lock() };
>
> To me it's very simple and straightforward for both the developer and the
> reviewer. It doesn't add any overhead to the implementation and it ensures
> that the developer (and later the reviewer) is aware of the potential issue.
>
> Of course, there's also the safe option if the developer is happy with
> closure-based API:
>
> 	x.with_read_lock(|_guard| {
> 		...
> 	});
>
> So it allows you to use the guard-based approach directly with the requirement
> of a safety comment and also provides a safe API for developers who don't want
> to deal with that. I am not sure if you fall into the third category, which is
> "I don't like writing safety comments and I don't like the closure-based
> approach" :)

We have been avoiding using callback-based API if there's an alternatively way
to achieve this. There has been quite a very precedents with this:

- spin_lock_irqsave requires taking and releasing in correct order, which is
  easy to solve with a callback approach. The same logic reasoning can be used
  to provide an unsafe API + safe callback API, but Boqun & Lyude reworked the
  spinlock IRQ design so we don't need that anymore.

- `Task::current` API could easily be replaced callback-based approach, but we
  used a macro to achieve without unsafe.

The API here is not inherently impossible to use guard API. It's not safe today
because a very specific detail. The callback-API is the "path of least
resistence" approach, but it's not the optimal one.

>
>> 
>> I suppose the only reason that this is unsafe is the "just leak it" condition
>> when cleaning up SRCU struct, which skips cleaning up delayed work, which could
>> call into `process_srcu`, which accesses `srcu_struct`. This however is *not*
>> leaked, because it's controlled by the user. Only the auxillary data allocated
>> by SRCU is leaked. So UAF is going to happen.
>> 
>> So in some aspect, the leak caused by `srcu_readers_active(ssp)` can cause more
>> damage compared to just continuing cleanup despite active users? I think this
>> could be changed in one of these ways:
>> * Have SRCU allocate all memory instead, and user-side would just have a
>>   `struct srcu_struct*`; then leaking would be safe. This is probably a bit
>>   difficult to change because it affects many users.
>
> We could do the same on the Rust side only. Basically instead of embedding
> srcu_struct in Rust srcu, allocate it separately and store its pointer. Then,
> if cleanup hits the active reader case, we could leak that allocation so any
> remaining srcu work does not hit UAF. I was aware of this option but I would
> prefer to avoid it because it adds an extra allocation for every Rust srcu.
>
>> * Continue to flush delayed work and stop the timer, and then leak before the
>>   actual kfree happens.
>
> Can you say more? I didn't understand this particular solution.

I was thinking that doing this _might_ be sufficient. I have to admit that I've
not very familar with the internal implementation of SRCU to make it an
assertion though.

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 0d01cd8c4b4a..5d75a4dbb6e5 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -717,8 +717,6 @@ void cleanup_srcu_struct(struct srcu_struct *ssp)
 	raw_spin_unlock_irq_rcu_node(ssp->srcu_sup);
 	if (WARN_ON(!delay))
 		return; /* Just leak it! */
-	if (WARN_ON(srcu_readers_active(ssp)))
-		return; /* Just leak it! */
 	/* Wait for irq_work to finish first as it may queue a new work. */
 	irq_work_sync(&sup->irq_work);
 	flush_delayed_work(&sup->work);

But after taking another look, I am not even sure if this is needed. A quick
glance of the code it appears that __srcu_read_unlock doesn't do anything apart
from adjusting the counter, and the SRCU grace period and thus the timers won't
actually start unless there's a pending grace period, which won't start unless
there's a call_srcu or sychronize_srcu. And we *know* that none of them would
happen, as the lifetime guarantees that nothing accesses the `Srcu` struct when
`drop` starts, and inside drop we have already invoked `srcu_barrier()`.

So I think, even if we hit the "Just leak it" scenario, we can still safely
deallocate the backing storage of `srcu_struct` and nothing should break?

>
>> * Trigger a `BUG` when the leak condition is hit for Rust users.
>
> We need an atomic counter to detect the leak and I thought that would be too
> much overhead for this abstraction. Basically each lock and drop will call an
> atomic operation so.

You could just check if srcu_sup is NULL after calling `cleanup_srcu_struct`.

Best,
Gary

>
>> * Declare the `WARN_ON` to be a sufficient protection and say this can be
>>   considered safe. Kinda similar to the strategy we take to the
>>   sleep-inside-atomic context issue.
>
> I think this is a rather weak precaution.
>