linux-kernel - Re: [PATCH v8 02/14] rust: hrtimer: introduce hrtimer support

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: <Z7iQcDa72XnJ5zGC@Mac.home>
Date: Fri, 21 Feb 2025 06:40:48 -0800
From: Boqun Feng <boqun.feng@...il.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Tamir Duberstein <tamird@...il.com>, 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>,
	Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	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>,
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 02/14] rust: hrtimer: introduce hrtimer support

On Fri, Feb 21, 2025 at 02:28:58PM +0100, Andreas Hindborg wrote:
> Andreas Hindborg <a.hindborg@...nel.org> writes:
> 
> > "Tamir Duberstein" <tamird@...il.com> writes:
> >
> >> On Thu, Feb 20, 2025 at 4:19 PM Andreas Hindborg <a.hindborg@...nel.org> wrote:
> >>>
> >>> "Tamir Duberstein" <tamird@...il.com> writes:
> >>>
> >>> > On Tue, Feb 18, 2025 at 8:29 AM Andreas Hindborg <a.hindborg@...nel.org> wrote:
> 
> [...]
> 
> >>> >> +    /// Get a pointer to the contained `bindings::hrtimer`.
> >>> >> +    ///
> >>> >> +    /// # Safety
> >>> >> +    ///
> >>> >> +    /// `ptr` must point to a live allocation of at least the size of `Self`.
> >>> >> +    unsafe fn raw_get(ptr: *const Self) -> *mut bindings::hrtimer {
> >>> >> +        // SAFETY: The field projection to `timer` does not go out of bounds,
> >>> >> +        // because the caller of this function promises that `ptr` points to an
> >>> >> +        // allocation of at least the size of `Self`.
> >>> >> +        unsafe { Opaque::raw_get(core::ptr::addr_of!((*ptr).timer)) }
> >>> >> +    }
> >>> >
> >>> > Can you help me understand why the various functions here operate on
> >>> > *const Self? I understand the need to obtain a C pointer to interact
> >>> > with bindings, but I don't understand why we're dealing in raw
> >>> > pointers to the abstraction rather than references.
> >>>
> >>> We cannot reference the `bindings::hrtimer` without wrapping it in
> >>> `Opaque`. This would be the primary reason. At other times, we cannot
> >>> produce references because we might not be able to prove that we satisfy
> >>> the safety requirements for turning a pointer into a reference. If we
> >>> are just doing offset arithmetic anyway, we don't need a reference.
> >>
> >> Why do we have a pointer, rather than a reference, to Self in the
> >> first place? I think this is the key thing I don't understand.
> >
> > Perhaps it makes more sense if you look at the context. One of the entry
> > points to `HrTimer::raw_get` is via `<ArcHrTimerHandle as
> > HrTimerHandle>::cancel`. This user facing method takes `&mut self`. The
> > handle contains an arc to a type that contains a `Timer` and implements
> > `HasHrTImer`. To get to the timer, we need to do pointer manipulation.
> > We only know how to get the `Timer` field via the `OFFSET`. The natural
> > return value from the offset operation is a raw pointer. Rather than
> > convert back to a reference, we stay in pointer land when we call
> > `HrTimer::raw_cancel`, because we need a pointer to the
> > `bindings::hrtimer` anyway, not a reference.
> 
> I changed `HasHrTimer::start` to take a reference, and I think that
> makes sense 👍 Taking an `impl AsRef` does not work out when `Self` is
> `Pin<&T>`. I'll go over the whole thing and see of other places could
> benefit.
> 

Hmm... if you mean:

trait HasHrTimer {
    unsafe fn start(&self, expires: Ktime) {
        ...
    }
}

Then it'll be problematic because the pointer derived from `&self`
doesn't have write provenance, therefore in a timer callback, the
pointer cannot be used for write, which means for example you cannot
convert the pointer back into a `Pin<Box<HasTimer>>`.

To answer Tamir's question, pointers are heavily used here because we
need to preserve the provenance.

Regards,
Boqun

> Best regards,
> Andreas Hindborg
> 
> 
>