[PATCH v2 0/3] Add and use abstraction for synchronize_rcu()

[PATCH v2 0/3] Add and use abstraction for synchronize_rcu()

[Philipp Stanner]

Changes since v1:
  - Vertically format imports. (Onur)
  - Make the new function #[inline]. (Alice)
  - Add Alice's R-b.

Philipp Stanner (3):
  rust: sync: Add abstraction for synchronize_rcu()
  rust: revocable: Use safe synchronize_rcu() abstraction
  rust: sync: Use safe synchronize_rcu() abstraction in poll

 rust/kernel/revocable.rs |  9 ++++++---
 rust/kernel/sync/poll.rs | 10 ++++++----
 rust/kernel/sync/rcu.rs  |  9 +++++++++
 3 files changed, 21 insertions(+), 7 deletions(-)


base-commit: 43a393185e33e573a374c1d4f7ddf6481484ef8d
-- 
2.54.0

[PATCH v2 1/3] rust: sync: Add abstraction for synchronize_rcu()

[Philipp Stanner]

synchronize_rcu() is a frequently used C function which is always safe
to be called.

Add a safe abstraction for synchronize_rcu().

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 rust/kernel/sync/rcu.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/rust/kernel/sync/rcu.rs b/rust/kernel/sync/rcu.rs
index a32bef6e490b..0d438ef31766 100644
--- a/rust/kernel/sync/rcu.rs
+++ b/rust/kernel/sync/rcu.rs
@@ -50,3 +50,12 @@ fn drop(&mut self) {
 pub fn read_lock() -> Guard {
     Guard::new()
 }
+
+/// Wait for one RCU grace period.
+///
+/// You typically do this to wait for everyone holding a [`Guard`].
+#[inline]
+pub fn synchronize_rcu() {
+    // SAFETY: `synchronize_rcu()` is always safe to be called. It just waits for a grace period.
+    unsafe { bindings::synchronize_rcu() };
+}
-- 
2.54.0

[PATCH v2 2/3] rust: revocable: Use safe synchronize_rcu() abstraction

[Philipp Stanner]

We now have a safe wrapper for the foreign function synchronize_rcu().

Use it in revocable.rs.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 rust/kernel/revocable.rs | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 0f4ae673256d..f539603349f1 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -7,7 +7,11 @@
 
 use pin_init::Wrapper;
 
-use crate::{bindings, prelude::*, sync::rcu, types::Opaque};
+use crate::{
+    prelude::*,
+    sync::rcu,
+    types::Opaque, //
+};
 use core::{
     marker::PhantomData,
     ops::Deref,
@@ -161,8 +165,7 @@ unsafe fn revoke_internal<const SYNC: bool>(&self) -> bool {
 
         if revoke {
             if SYNC {
-                // SAFETY: Just an FFI call, there are no further requirements.
-                unsafe { bindings::synchronize_rcu() };
+                rcu::synchronize_rcu();
             }
 
             // SAFETY: We know `self.data` is valid because only one CPU can succeed the
-- 
2.54.0

[PATCH v2 3/3] rust: sync: Use safe synchronize_rcu() abstraction in poll

[Philipp Stanner]

We now have a safe wrapper for the foreign function synchronize_rcu().

Use it in poll.rs.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/sync/poll.rs | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/sync/poll.rs b/rust/kernel/sync/poll.rs
index 0ec985d560c8..30ebeea1702f 100644
--- a/rust/kernel/sync/poll.rs
+++ b/rust/kernel/sync/poll.rs
@@ -8,7 +8,11 @@
     bindings,
     fs::File,
     prelude::*,
-    sync::{CondVar, LockClassKey},
+    sync::{
+        CondVar,
+        LockClassKey,
+        rcu::synchronize_rcu, //
+    }, //
 };
 use core::{marker::PhantomData, ops::Deref};
 
@@ -99,8 +103,6 @@ fn drop(self: Pin<&mut Self>) {
         unsafe { bindings::__wake_up_pollfree(self.inner.wait_queue_head.get()) };
 
         // Wait for epoll items to be properly removed.
-        //
-        // SAFETY: Just an FFI call.
-        unsafe { bindings::synchronize_rcu() };
+        synchronize_rcu();
     }
 }
-- 
2.54.0

Re: [PATCH v2 0/3] Add and use abstraction for synchronize_rcu()

[Onur Özkan]

On Mon, 22 Jun 2026 19:32:48 +0200
Philipp Stanner <phasta@kernel.org> wrote:

> Changes since v1:
>   - Vertically format imports. (Onur)
>   - Make the new function #[inline]. (Alice)
>   - Add Alice's R-b.
> 
> Philipp Stanner (3):
>   rust: sync: Add abstraction for synchronize_rcu()
>   rust: revocable: Use safe synchronize_rcu() abstraction
>   rust: sync: Use safe synchronize_rcu() abstraction in poll
> 
>  rust/kernel/revocable.rs |  9 ++++++---
>  rust/kernel/sync/poll.rs | 10 ++++++----
>  rust/kernel/sync/rcu.rs  |  9 +++++++++
>  3 files changed, 21 insertions(+), 7 deletions(-)
> 
> 
> base-commit: 43a393185e33e573a374c1d4f7ddf6481484ef8d
> -- 
> 2.54.0
> 

For the whole series:

Reviewed-by: Onur Özkan <work@onurozkan.dev>

Re: [PATCH v2 1/3] rust: sync: Add abstraction for synchronize_rcu()

[Gary Guo]

On Mon Jun 22, 2026 at 6:32 PM BST, Philipp Stanner wrote:
> synchronize_rcu() is a frequently used C function which is always safe
> to be called.
> 
> Add a safe abstraction for synchronize_rcu().
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/sync/rcu.rs | 9 +++++++++
>  1 file changed, 9 insertions(+)

Re: [PATCH v2 2/3] rust: revocable: Use safe synchronize_rcu() abstraction

[Gary Guo]

On Mon Jun 22, 2026 at 6:32 PM BST, Philipp Stanner wrote:
> We now have a safe wrapper for the foreign function synchronize_rcu().
> 
> Use it in revocable.rs.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/revocable.rs | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Re: [PATCH v2 3/3] rust: sync: Use safe synchronize_rcu() abstraction in poll

[Gary Guo]

On Mon Jun 22, 2026 at 6:32 PM BST, Philipp Stanner wrote:
> We now have a safe wrapper for the foreign function synchronize_rcu().
> 
> Use it in poll.rs.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/sync/poll.rs | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Re: [PATCH v2 1/3] rust: sync: Add abstraction for synchronize_rcu()

[Danilo Krummrich]

On Mon Jun 22, 2026 at 7:32 PM CEST, Philipp Stanner wrote:
> +/// Wait for one RCU grace period.
> +///
> +/// You typically do this to wait for everyone holding a [`Guard`].

NIT: "typically" reads a bit as if there were other reasons to call
synchronize_rcu() than to wait for all concurrent RCU read side critical
sections.

Also, while it's implicit, it might still be worth to explicitly call out that
this means concurrently held Guard objects (concurrent read side critical
sections), i.e. subsequent read side critical sections may still run
concurrently.

> +#[inline]
> +pub fn synchronize_rcu() {
> +    // SAFETY: `synchronize_rcu()` is always safe to be called. It just waits for a grace period.
> +    unsafe { bindings::synchronize_rcu() };
> +}
> -- 
> 2.54.0

Re: [PATCH v2 0/3] Add and use abstraction for synchronize_rcu()

[Danilo Krummrich]

On Mon Jun 22, 2026 at 7:32 PM CEST, Philipp Stanner wrote:
>   rust: sync: Add abstraction for synchronize_rcu()
>   rust: revocable: Use safe synchronize_rcu() abstraction
>   rust: sync: Use safe synchronize_rcu() abstraction in poll

Reviewed-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH v2 1/3] rust: sync: Add abstraction for synchronize_rcu()

[Philipp Stanner]

On Mon, 2026-06-22 at 20:46 +0200, Danilo Krummrich wrote:
> On Mon Jun 22, 2026 at 7:32 PM CEST, Philipp Stanner wrote:
> > +/// Wait for one RCU grace period.
> > +///
> > +/// You typically do this to wait for everyone holding a [`Guard`].
> 
> NIT: "typically" reads a bit as if there were other reasons to call
> synchronize_rcu() than to wait for all concurrent RCU read side critical
> sections.

The reason I wrote "typically" is because my mind had the potential
future use-case of ours prefetched where we might have to do a
synchronize_rcu() to wait for a C backend to be done with something,
where no one really holds a Rust `Guard` (though of course the read
lock).

> 
> Also, while it's implicit, it might still be worth to explicitly call out that
> this means concurrently held Guard objects (concurrent read side critical
> sections), i.e. subsequent read side critical sections may still run
> concurrently.

That's quite generic RCU knowledge IMO. I'm not sure to what degree one
wants to document RCU in general at this new function here, vs just the
Rust API.


Rewording the documentation is fine by me, but since we're in a nitty
domain here I would then ask you to provide a few draft sentences that
would satisfy your basic requirements.


P.

> 
> > +#[inline]
> > +pub fn synchronize_rcu() {
> > +    // SAFETY: `synchronize_rcu()` is always safe to be called. It just waits for a grace period.
> > +    unsafe { bindings::synchronize_rcu() };
> > +}
> > -- 
> > 2.54.0

Re: [PATCH v2 1/3] rust: sync: Add abstraction for synchronize_rcu()

[Pedro Falcato]

On Mon, Jun 22, 2026 at 07:32:49PM +0200, Philipp Stanner wrote:
> synchronize_rcu() is a frequently used C function which is always safe
> to be called.
> 
> Add a safe abstraction for synchronize_rcu().
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  rust/kernel/sync/rcu.rs | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/rust/kernel/sync/rcu.rs b/rust/kernel/sync/rcu.rs
> index a32bef6e490b..0d438ef31766 100644
> --- a/rust/kernel/sync/rcu.rs
> +++ b/rust/kernel/sync/rcu.rs
> @@ -50,3 +50,12 @@ fn drop(&mut self) {
>  pub fn read_lock() -> Guard {
>      Guard::new()
>  }
> +
> +/// Wait for one RCU grace period.
> +///
> +/// You typically do this to wait for everyone holding a [`Guard`].
> +#[inline]
> +pub fn synchronize_rcu() {
> +    // SAFETY: `synchronize_rcu()` is always safe to be called. It just waits for a grace period.

Commething randomly here (I know as much rust as the next not-knowing-rust-guy, sadly),
but synchronize_rcu() is not always safe to be called. You cannot call it if
you have the rcu read lock, or cannot sleep for any reason. I don't know if
you can encode these in the rust type system though.

-- 
Pedro

Re: [PATCH v2 1/3] rust: sync: Add abstraction for synchronize_rcu()

[Philipp Stanner]

On Tue, 2026-06-23 at 09:56 +0100, Pedro Falcato wrote:
> On Mon, Jun 22, 2026 at 07:32:49PM +0200, Philipp Stanner wrote:
> > synchronize_rcu() is a frequently used C function which is always safe
> > to be called.
> > 
> > Add a safe abstraction for synchronize_rcu().
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > ---
> >  rust/kernel/sync/rcu.rs | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/rust/kernel/sync/rcu.rs b/rust/kernel/sync/rcu.rs
> > index a32bef6e490b..0d438ef31766 100644
> > --- a/rust/kernel/sync/rcu.rs
> > +++ b/rust/kernel/sync/rcu.rs
> > @@ -50,3 +50,12 @@ fn drop(&mut self) {
> >  pub fn read_lock() -> Guard {
> >      Guard::new()
> >  }
> > +
> > +/// Wait for one RCU grace period.
> > +///
> > +/// You typically do this to wait for everyone holding a [`Guard`].
> > +#[inline]
> > +pub fn synchronize_rcu() {
> > +    // SAFETY: `synchronize_rcu()` is always safe to be called. It just waits for a grace period.
> 
> Commething randomly here (I know as much rust as the next not-knowing-rust-guy, sadly),
> but synchronize_rcu() is not always safe to be called. You cannot call it if
> you have the rcu read lock, or cannot sleep for any reason. I don't know if
> you can encode these in the rust type system though.

Preventing illegal actions in atomic context seems very difficult to
achieve in Rust and AFAIK the only viable path forward currently is to
have klint check for rule violations. Gary is working on that.

But it would be interesting to know more about how in general Rust's
unsafe comments are related to problems beyond UAF issues, and to what
degree we want to document context requirements.

C documents functions like synchronize_rcu() in much detail, but its
documentation trusts on the readers general familiarity with RCU,
assuming no one would come up with the idea of calling
synchronize_rcu() in an atomic context.


P.

Re: [PATCH v2 1/3] rust: sync: Add abstraction for synchronize_rcu()

[Miguel Ojeda]

On Tue, Jun 23, 2026 at 11:49 AM Philipp Stanner <phasta@mailbox.org> wrote:
>
> But it would be interesting to know more about how in general Rust's
> unsafe comments are related to problems beyond UAF issues, and to what
> degree we want to document context requirements.

I am confused by the UAF there. Did you mean UB?

Rust's `unsafe` is about way more than just use-after-free -- it is
about all potential undefined behavior.

At the same time, it is not about merely "dangerous" things.

If you cannot possibly cause UB, then it is not in scope. Otherwise,
it is very much in scope and the safety preconditions/requirements
need to be clearly documented (`# Safety`) or justified (`//
SAFETY:`).

Now, sometimes it may not make a lot of sense to duplicate a ton of
information, so sometimes we lift text to the Rust module docs and
refer to it; and sometimes it may also make more sense to refer to
external docs. One way or another, the goal is to document the
requirements and what is going on as clearly as possible.

Cheers,
Miguel

Re: [PATCH v2 1/3] rust: sync: Add abstraction for synchronize_rcu()

[Danilo Krummrich]

On Tue Jun 23, 2026 at 8:09 AM CEST, Philipp Stanner wrote:
> The reason I wrote "typically" is because my mind had the potential
> future use-case of ours prefetched where we might have to do a
> synchronize_rcu() to wait for a C backend to be done with something,
> where no one really holds a Rust `Guard` (though of course the read
> lock).

In that case I'd just say "read side critical section" and then mention the
rcu::Guard type as one way to end up in such.

In comparison with other synchronization primitives, RCU is a global mechanism,
so only talking about the rcu::Guard specifically might indeed be misleading.

>> Also, while it's implicit, it might still be worth to explicitly call out that
>> this means concurrently held Guard objects (concurrent read side critical
>> sections), i.e. subsequent read side critical sections may still run
>> concurrently.
>
> That's quite generic RCU knowledge IMO. I'm not sure to what degree one
> wants to document RCU in general at this new function here, vs just the
> Rust API.

One or two precise sentences should be enough; no need to document RCU in
general.

> Rewording the documentation is fine by me, but since we're in a nitty
> domain here I would then ask you to provide a few draft sentences that
> would satisfy your basic requirements.

Should be as simple as adding "in progress", e.g. considering the above:

	This waits for all RCU read-side critical sections already in progress,
	including those established by an [`rcu::Guard`].
	
	This does not prevent new read-side critical sections from starting;
	those may begin and run while this call blocks.

Re: [PATCH v2 1/3] rust: sync: Add abstraction for synchronize_rcu()

[Philipp Stanner]

On Tue, 2026-06-23 at 12:24 +0200, Miguel Ojeda wrote:
> On Tue, Jun 23, 2026 at 11:49 AM Philipp Stanner <phasta@mailbox.org> wrote:
> > 
> > But it would be interesting to know more about how in general Rust's
> > unsafe comments are related to problems beyond UAF issues, and to what
> > degree we want to document context requirements.
> 
> I am confused by the UAF there. Did you mean UB?
> 
> Rust's `unsafe` is about way more than just use-after-free -- it is
> about all potential undefined behavior.
> 
> At the same time, it is not about merely "dangerous" things.
> 
> If you cannot possibly cause UB, then it is not in scope. Otherwise,
> it is very much in scope and the safety preconditions/requirements
> need to be clearly documented (`# Safety`) or justified (`//
> SAFETY:`).
> 
> Now, sometimes it may not make a lot of sense to duplicate a ton of
> information, so sometimes we lift text to the Rust module docs and
> refer to it; and sometimes it may also make more sense to refer to
> external docs. One way or another, the goal is to document the
> requirements and what is going on as clearly as possible.

Well, commonly, deadlock is not regarded to be UB.

For RCU the question really is to what extend one wants to have it. The
overall robustness requirement here is definitely for the Rust function
rcu::synchronize_rcu(), since the API caller is the one in charge of
the execution context.

If all potential failures one can cause by calling that function at the
wrong place were regarded to be undefined behavior, then a
synchronize_rcu() Rust function would have to be an unsafe function
always, making a wrapper pointless.

Similarly, Rust's drop() implementations might be potentially "unsafe"
with a hyper-strict definition (note that I'm unsure whether calling
synchronize_rcu() in atomic context is actually even defined behavior;
I think it is. I'm just brainstorming here)

I think briefly documenting the context requirement is fine, but from a
consistency and pragmatism perspective I would not make that a formal
safety requirement.


P.

> 
> Cheers,
> Miguel

Re: [PATCH v2 1/3] rust: sync: Add abstraction for synchronize_rcu()

[Danilo Krummrich]

On Tue Jun 23, 2026 at 1:04 PM CEST, Philipp Stanner wrote:
> with a hyper-strict definition (note that I'm unsure whether calling
> synchronize_rcu() in atomic context is actually even defined behavior;
> I think it is. I'm just brainstorming here)

From the Rust side of things this is well defined; please see [1] and [2].

However, given the kernel context, things such as deadlocks have more
consequences than in userspace. Which is why it is still desirable to catch them
(e.g. with tools like klint).

[1] https://doc.rust-lang.org/reference/behavior-considered-undefined.html
[2] https://doc.rust-lang.org/reference/behavior-not-considered-unsafe.html