[PATCH v4 1/3] rust: revocable: update write invariant and fix safety comments

Marcelo Moreira
2025-06-02T23:28:51Z

This commit clarifies the write invariant of the `Revocable` type and
updates associated `SAFETY` comments. The write invariant now precisely
states that `data` is valid for writes after `is_available` transitions
from true to false, provided no thread holding an RCU read-side lock
(acquired before the change) still has access to `data`.

The `SAFETY` comment in `try_access_with_guard` is updated to reflect
this invariant, and the `PinnedDrop` `drop` implementation's `SAFETY`
comment is refined to clearly state the guarantees provided by the `&mut Self`
context regarding exclusive access and `data`'s validity for dropping.

Reported-by: Benno Lossin <lossin@kernel.org>
Closes: https://github.com/Rust-for-Linux/linux/issues/1160
Suggested-by: Benno Lossin <lossin@kernel.org>
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
---
rust/kernel/revocable.rs | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 1e5a9d25c21b..d14f9052f1ac 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -61,6 +61,15 @@
/// v.revoke();
/// assert_eq!(add_two(&v), None);
/// ```
+///
+/// # Invariants
+///
+/// - `data` is valid for reads in two cases:
+/// - while `is_available` is true, or
+/// - while the RCU read-side lock is taken and it was acquired while `is_available` was `true`.
+/// - `data` is valid for writes when `is_available` was atomically changed from `true` to `false`
+/// and no thread that has access to `data` is holding an RCU read-side lock that was acquired prior to
+/// the change in `is_available`.
#[pin_data(PinnedDrop)]
pub struct Revocable<T> {
is_available: AtomicBool,
@@ -115,8 +124,8 @@ pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
/// object.
pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a T> {
if self.is_available.load(Ordering::Relaxed) {
- // SAFETY: Since `self.is_available` is true, data is initialised and has to remain
- // valid because the RCU read side lock prevents it from being dropped.
+ // SAFETY: `Self::data` is valid for reads because of `Self`'s type invariants,
+ // as `Self::is_available` is true and `_guard` holds the RCU read-side lock
Some(unsafe { &*self.data.get() })
} else {
None
@@ -176,9 +185,10 @@ fn drop(self: Pin<&mut Self>) {
// SAFETY: We are not moving out of `p`, only dropping in place
let p = unsafe { self.get_unchecked_mut() };
if *p.is_available.get_mut() {
- // SAFETY: We know `self.data` is valid because no other CPU has changed
- // `is_available` to `false` yet, and no other CPU can do it anymore because this CPU
- // holds the only reference (mutable) to `self` now.
+ // SAFETY: `Self::data` is valid for writes because of `Self`'s type invariants,
+ // and because this `PinnedDrop` context (having `&mut Self`) guarantees exclusive access,
+ // ensuring no other thread can concurrently access or revoke `data`.
+ // This ensures `data` is valid for `drop_in_place`.
unsafe { drop_in_place(p.data.get()) };
}
}
--
2.49.0