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

[FUJITA Tomonori <tomo@flapping.org>]

On Wed, 15 Jul 2026 15:51:23 +0100
"Gary Guo" <gary@garyguo.net> wrote:

>>> "FUJITA Tomonori" <tomo@aliasing.net> writes:
>>> 
>>>> 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 expires_unchecked() 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>
>>> 
>>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>
>> One thing I'd like to double check before merging this,
>> HrTImerCallbackContext::expires() now does:
>>
>> unsafe { self.0.as_ref().expires_unchecked() }
>>
>> which briefly makes a &HrTimer<T> from the NonNull<HrTimer<T>>.
>>
>> This is only sound today because `HrTimer<T>`'s sole field is
>> `Opaque<bindings::hrtimer>`. As you pointed out earlier, if we add a
>> field to HrTimer, "stuff breaks".
> 
> What breaks? You're just converting it to shared reference.

Yeah, two shared refs on their own never conflict.

But pin_mut.rs/tbox_rs's run() build a live Pin<&mut T> from the same
memory as the HrTimerCallbackContext they hand to the same call. So
isn't .as_ref() there forming &HrTimer<T> while that Pin<&mut T> is
still alive?

If so, doesn't that maean every field of HrTimer<T> has to stay behind
Opaque for the reference to be valid? Hold today but feels like a real
invariant now, not something forward()'s raw-pointer path ever needed.