lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <877btdpyz2.fsf@t14s.mail-host-address-is-not-set>
Date: Mon, 19 Jan 2026 13:29:37 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: FUJITA Tomonori <fujita.tomonori@...il.com>, ojeda@...nel.org
Cc: aliceryhl@...gle.com, anna-maria@...utronix.de,
 bjorn3_gh@...tonmail.com, boqun.feng@...il.com, dakr@...nel.org,
 frederic@...nel.org, gary@...yguo.net, jstultz@...gle.com,
 lossin@...nel.org, lyude@...hat.com, sboyd@...nel.org, tglx@...utronix.de,
 tmgross@...ch.edu, linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v1] rust: hrtimer: Restrict expires() to safe contexts

"FUJITA Tomonori" <fujita.tomonori@...il.com> writes:

> 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.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@...il.com>

Patch looks good to me, but I just want to check with Lyude about their
use case in the rvkms driver. I think that is why we did the racy
implementation originally. In C we have stuff like this:


    /**
    * drm_crtc_vblank_get_vblank_timeout - Returns the vblank timeout
    * @crtc: The CRTC
    * @vblank_time: Returns the next vblank timestamp
    *
    * The helper drm_crtc_vblank_get_vblank_timeout() returns the next vblank
    * timestamp of the CRTC's vblank timer according to the timer's expiry
    * time.
    */
    void drm_crtc_vblank_get_vblank_timeout(struct drm_crtc *crtc, ktime_t *vblank_time)
    {
      struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
      struct drm_vblank_crtc_timer *vtimer = &vblank->vblank_timer;
      u64 cur_count;
      ktime_t cur_time;

      if (!READ_ONCE(vblank->enabled)) {
        *vblank_time = ktime_get();
        return;
      }

      /*
      * A concurrent vblank timeout could update the expires field before
      * we compare it with the vblank time. Hence we'd compare the old
      * expiry time to the new vblank time; deducing the timer had already
      * expired. Reread until we get consistent values from both fields.
      */
      do {
        cur_count = drm_crtc_vblank_count_and_time(crtc, &cur_time);
        *vblank_time = READ_ONCE(vtimer->timer.node.expires);
      } while (cur_count != drm_crtc_vblank_count_and_time(crtc, &cur_time));

      if (drm_WARN_ON(crtc->dev, !ktime_compare(*vblank_time, cur_time)))
        return; /* Already expired */

      /*
      * To prevent races we roll the hrtimer forward before we do any
      * interrupt processing - this is how real hw works (the interrupt
      * is only generated after all the vblank registers are updated)
      * and what the vblank core expects. Therefore we need to always
      * correct the timestamp by one frame.
      */
      *vblank_time = ktime_sub(*vblank_time, vtimer->interval);
    }
    EXPORT_SYMBOL(drm_crtc_vblank_get_vblank_timeout);


Also, we got some new docs for `read_volatile` that allow us to read
memory outside Rust of any allocation that are not "valid for read" [1],
meaning racy reads are OK as far as I understand. So the original
implementation might actually be OK, although the number might not be
correct always.


Best regards,
Andreas Hindborg


[1] https://doc.rust-lang.org/std/ptr/fn.read_volatile.html


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ