[PATCH v1 2/2] rust: hrtimer: Use hrtimer_forward_safe_from_callback()

[FUJITA Tomonori]

From: FUJITA Tomonori <fujita.tomonori@xxxxxxxxx>

Safe Rust code can start a timer while its callback runs. A timer
started through Arc<T> or Pin<&T> can be started again from another
clone or copy of the pointer. HrTimerCallbackContext::forward() calls
hrtimer_forward(), which writes the expiry without the base lock, so
the write can race with the concurrent hrtimer_start().

Call hrtimer_forward_safe_from_callback() instead. It does the update
with the base lock held. If the timer is already queued by a concurrent
start, the expiry is left alone and 0 is returned.

HrTimer::forward() keeps calling hrtimer_forward(). It takes
Pin<&mut Self>, so no other reference to the timer exists and no
concurrent start is possible. Narrow the safety requirement of
raw_forward() to exclusive access, since the callback context no longer
uses it.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxx>
---
 rust/kernel/time/hrtimer.rs | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index a53f7f1341be..bf4094d84822 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -521,8 +521,7 @@ pub(crate) unsafe fn raw_cancel(this: *const Self) -> bool {
     /// # Safety
     ///
     /// - `self_ptr` must point to a valid `Self`.
-    /// - The caller must either have exclusive access to the data pointed at by `self_ptr`, or be
-    ///   within the context of the timer callback.
+    /// - The caller must have exclusive access to the data pointed at by `self_ptr`.
     #[inline]
     unsafe fn raw_forward(self_ptr: *mut Self, now: HrTimerInstant<T>, interval: Delta) -> u64
     where
@@ -1060,13 +1059,25 @@ pub(crate) unsafe fn from_raw(timer: *mut HrTimer<T>) -> Self {
 
     /// Conditionally forward the timer.
     ///
-    /// This function is identical to [`HrTimer::forward()`] except that it may only be used from
-    /// within the context of a [`HrTimer`] callback.
+    /// This function does the same as [`HrTimer::forward()`], but it may only be used from within
+    /// the context of a [`HrTimer`] callback, and it is safe against a concurrent start operation.
+    /// If the timer was started by a concurrent start operation, the expiry is not changed, this
+    /// function returns 0, and the expiry set by the start operation is used.
     pub fn forward(&mut self, now: HrTimerInstant<T>, interval: Delta) -> u64 {
+        // SAFETY: By our type invariants, `self.0` always points to a valid `HrTimer<T>`.
+        let c_timer_ptr = unsafe { HrTimer::<T>::raw_get(self.0.as_ptr()) };
+
         // SAFETY:
-        // - We are guaranteed to be within the context of a timer callback by our type invariants
-        // - By our type invariants, `self.0` always points to a valid `HrTimer<T>`
-        unsafe { HrTimer::<T>::raw_forward(self.0.as_ptr(), now, interval) }
+        // - `c_timer_ptr` is initialized and valid.
+        // - By our type invariants, we are within the context of the timer callback, as
+        //   `hrtimer_forward_safe_from_callback()` requires.
+        unsafe {
+            bindings::hrtimer_forward_safe_from_callback(
+                c_timer_ptr,
+                now.as_nanos(),
+                interval.as_nanos(),
+            )
+        }
     }
 
     /// Conditionally forward the timer.
-- 
2.43.0