[PATCH 4/6] rust: hrtimer: restrict expires() to exclusive access

From: FUJITA Tomonori <[email protected]>

HrTimer::expires() read node.expires through a volatile load on a
shared reference. The read is unsynchronized: a concurrent start
operation rewrites the expiry under the timer base lock, and the
64-bit load can tear on 32-bit architectures. The volatile idiom
narrows the race but does not remove it.

Change expires() to take Pin<&mut Self>. Wherever an exclusive
reference to the timer is reachable, no start operation can run
concurrently: the timer handles own or borrow the containing object
exclusively for the box and pinned pointer types, and no exclusive
reference is reachable through an Arc. Route the read through
hrtimer_get_expires() via a helper instead of duplicating the field
access on the Rust side, and provide the unsafe expires_unchecked()
for contexts that can guarantee exclusive access by other means.

Reading the expiry from within the timer callback is served by the
expiry snapshot passed to HrTimerCallback::run(), so no callback
context accessor is needed.

Fixes: 4b0147494275 ("rust: hrtimer: Add HrTimer::expires()")
Closes: https://lore.kernel.org/rust-for-linux/[email protected]/
Signed-off-by: FUJITA Tomonori <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[ Andreas - Reword commit message and rebase on expiry injection patches. ]
Signed-off-by: Andreas Hindborg <[email protected]>
---
 rust/helpers/time.c         |  6 ++++++
 rust/kernel/time/hrtimer.rs | 37 +++++++++++++++++++++++--------------
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/rust/helpers/time.c b/rust/helpers/time.c
index 32f4959704939..205a38839532a 100644
--- a/rust/helpers/time.c
+++ b/rust/helpers/time.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/delay.h>
+#include <linux/hrtimer.h>
 #include <linux/ktime.h>
 #include <linux/timekeeping.h>
 
@@ -38,3 +39,8 @@ __rust_helper void rust_helper_udelay(unsigned long usec)
 {
 	udelay(usec);
 }
+
+__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
+{
+	return hrtimer_get_expires(timer);
+}
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index e6570a6162035..bdb6aaa228396 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -567,27 +567,36 @@ pub fn forward_now(self: Pin<&mut Self>, interval: Delta) -> u64
         self.forward(HrTimerInstant::<T>::now(), interval)
     }
 
+    /// Return the time expiry for this [`HrTimer`].
+    ///
+    /// # Safety
+    ///
+    /// The caller must have exclusive access to `self`.
+    #[inline]
+    unsafe fn expires_unchecked(&self) -> HrTimerInstant<T>
+    where
+        T: HasHrTimer<T>,
+    {
+        // SAFETY:
+        // - The C API requirements for this function are fulfilled by our safety contract.
+        // - Timers cannot have negative `ktime_t` values as their expiration time.
+        unsafe { Instant::from_ktime(bindings::hrtimer_get_expires(Self::raw_get(self))) }
+    }
+
     /// Return the time expiry for this [`HrTimer`].
     ///
     /// This value should only be used as a snapshot, as the actual expiry time could change after
-    /// this function is called.
-    pub fn expires(&self) -> HrTimerInstant<T>
+    /// this function is called. To read the expiry from within the timer callback, use the value
+    /// passed to [`HrTimerCallback::run`] instead.
+    pub fn expires(self: Pin<&mut Self>) -> HrTimerInstant<T>
     where
         T: HasHrTimer<T>,
     {
-        // SAFETY: `self` is an immutable reference and thus always points to a valid `HrTimer`.
-        let c_timer_ptr = unsafe { HrTimer::raw_get(self) };
+        // SAFETY: `expires_unchecked` does not move `Self`.
+        let this = unsafe { self.get_unchecked_mut() };
 
-        // SAFETY:
-        // - Timers cannot have negative ktime_t values as their expiration time.
-        // - There's no actual locking here, a racy read is fine and expected
-        unsafe {
-            Instant::from_ktime(
-                // This `read_volatile` is intended to correspond to a READ_ONCE call.
-                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
-                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
-            )
-        }
+        // SAFETY: By existence of `Pin<&mut Self>`, we have exclusive access to `Self`.
+        unsafe { this.expires_unchecked() }
     }
 }
 

-- 
2.51.2