Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

["Gary Guo" <gary@garyguo.net>]

On Tue Feb 24, 2026 at 1:25 PM GMT, FUJITA Tomonori wrote:
> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>
> HrTimer::expires() previously read node.expires via a volatile load, which
> can race with C-side updates. Rework the API so it is only callable with
> exclusive access or from the callback context.
>
> Introduce raw_expires() with an explicit safety contract, switch
> HrTimer::expires() to Pin<&mut Self>, add
> HrTimerCallbackContext::expires(), and route the read through
> hrtimer_get_expires() via a Rust helper.
>
> Fixes: 4b0147494275 ("rust: hrtimer: Add HrTimer::expires()")
> Closes: https://lore.kernel.org/rust-for-linux/87ldi7f4o1.fsf@t14s.mail-host-address-is-not-set/
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
> v2:
> - Add Fixes and Closes tags
> - Fix and improve comments
> v1: https://lore.kernel.org/rust-for-linux/20260110115838.3109895-1-fujita.tomonori@gmail.com/
> ---
>  rust/helpers/time.c         |  6 +++++
>  rust/kernel/time/hrtimer.rs | 49 +++++++++++++++++++++++++++----------
>  2 files changed, 42 insertions(+), 13 deletions(-)
>
> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
> index 32f495970493..ef8999621399 100644
> --- a/rust/helpers/time.c
> +++ b/rust/helpers/time.c
> @@ -2,6 +2,7 @@
>  
>  #include <linux/delay.h>
>  #include <linux/ktime.h>
> +#include <linux/hrtimer.h>
>  #include <linux/timekeeping.h>
>  
>  __rust_helper void rust_helper_fsleep(unsigned long usecs)
> @@ -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 856d2d929a00..78e2343fd016 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -224,27 +224,39 @@ 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
> +    ///
> +    /// - `self_ptr` must point to a valid `Self`.
> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
> +    ///   within the context of the timer callback.
> +    #[inline]
> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>

I guess the signature of this can also be

`unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
first safety requirement.

Best,
Gary

> +    where
> +        T: HasHrTimer<T>,
> +    {
> +        // SAFETY:
> +        // - The C API requirements for this function are fulfilled by our safety contract.
> +        // - `self_ptr` is guaranteed to point to a valid `Self` via 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_ptr))) }
> +    }
> +
>      /// 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>
> +    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: `raw_expires` 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>`, the pointer passed to `raw_expires` points to a
> +        // valid `Self` that we have exclusive access to.
> +        unsafe { Self::raw_expires(this) }
>      }
>  }
>  
> @@ -729,6 +741,17 @@ pub fn forward(&mut self, now: HrTimerInstant<T>, interval: Delta) -> u64 {
>      pub fn forward_now(&mut self, duration: Delta) -> u64 {
>          self.forward(HrTimerInstant::<T>::now(), duration)
>      }
> +
> +    /// Return the time expiry for this [`HrTimer`].
> +    ///
> +    /// This function is identical to [`HrTimer::expires()`] except that it may only be used from
> +    /// within the context of a [`HrTimer`] callback.
> +    pub fn expires(&self) -> HrTimerInstant<T> {
> +        // 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_expires(self.0.as_ptr()) }
> +    }
>  }
>  
>  /// Use to implement the [`HasHrTimer<T>`] trait.
>
> base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f