linux-kernel - Re: [PATCH v11 08/13] rust: hrtimer: implement `UnsafeHrTimerPointer` for `Pin<&mut T>`

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: <D8BNFIK9U108.273II0I7NZUG1@proton.me>
Date: Sun, 09 Mar 2025 10:04:42 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Andreas Hindborg <a.hindborg@...nel.org>, Miguel Ojeda <ojeda@...nel.org>, Anna-Maria Behnsen <anna-maria@...utronix.de>, Frederic Weisbecker <frederic@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Danilo Krummrich <dakr@...nel.org>
Cc: Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, Lyude Paul <lyude@...hat.com>, Guangbo Cui <2407018371@...com>, Dirk Behme <dirk.behme@...il.com>, Daniel Almeida <daniel.almeida@...labora.com>, Tamir Duberstein <tamird@...il.com>, Markus Elfring <Markus.Elfring@....de>, rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v11 08/13] rust: hrtimer: implement `UnsafeHrTimerPointer` for `Pin<&mut T>`

On Fri Mar 7, 2025 at 10:38 PM CET, Andreas Hindborg wrote:
> +impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
> +where
> +    T: HasHrTimer<T>,
> +    T: HrTimerCallback<Pointer<'a> = Self>,
> +{
> +    type CallbackTarget<'b> = Self;
> +
> +    unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart {
> +        // `HrTimer` is `repr(C)`
> +        let timer_ptr = ptr as *mut HrTimer<T>;
> +
> +        // SAFETY: By the safety requirement of this function, `timer_ptr`
> +        // points to a `HrTimer<T>` contained in an `T`.
> +        let receiver_ptr = unsafe { T::timer_container_of(timer_ptr) };
> +
> +        // SAFETY:
> +        //  - By the safety requirement of this function, `timer_ptr`
> +        //    points to a `HrTimer<T>` contained in an `T`.
> +        //  - As per he safety requirements of the trait `HrTimerHandle`, the
> +        //    `PinMutHrTimerHandle` associated with this timer is guaranteed to
> +        //    be alive until this method returns. As the handle borrows from
> +        //    `T`, `T` is also guaranteed to be alive for the duration of this
> +        //    function.

Ah one more thing, I don't think that the second part is needed (i.e.
that `T` is alive). How about:

        //  - As per the safety requirements of the trait `HrTimerHandle`, the `PinMutHrTimerHandle`
        //  associated with this timer is guaranteed to be alive until this method returns. That
        //  handle borrows the `T` behind `receiver_ptr` mutably thus guaranteeing the validity of
        //  the reference created below.

Can you also adjust the other instances of this in the other patches?
Thanks!

---
Cheers,
Benno

> +        //  - Because `PinMutTimerHandle` borrows mutably from `T`, the
> +        //    reference we create here is guaranteed to be unique.
> +        let receiver_ref = unsafe { &mut *receiver_ptr };
> +
> +        // SAFETY: `receiver_ref` only exists as pinned, so it is safe to pin it
> +        // here.
> +        let receiver_pin = unsafe { Pin::new_unchecked(receiver_ref) };
> +
> +        T::run(receiver_pin).into_c()
> +    }
> +}
>
> --
> 2.47.0