linux-kernel - Re: [PATCH] rust: time: Seal the ClockSource trait

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: <aFMTsz63qOo5LQOE@tardis.local>
Date: Wed, 18 Jun 2025 12:29:55 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, alex.gaynor@...il.com,
	ojeda@...nel.org, aliceryhl@...gle.com, anna-maria@...utronix.de,
	bjorn3_gh@...tonmail.com, dakr@...nel.org, frederic@...nel.org,
	gary@...yguo.net, jstultz@...gle.com, linux-kernel@...r.kernel.org,
	lossin@...nel.org, lyude@...hat.com, rust-for-linux@...r.kernel.org,
	sboyd@...nel.org, tglx@...utronix.de, tmgross@...ch.edu
Subject: Re: [PATCH] rust: time: Seal the ClockSource trait

On Wed, Jun 18, 2025 at 09:13:07PM +0200, Andreas Hindborg wrote:
> "Boqun Feng" <boqun.feng@...il.com> writes:
> 
> > On Tue, Jun 17, 2025 at 05:10:42PM -0700, Boqun Feng wrote:
> >> On Wed, Jun 18, 2025 at 08:20:53AM +0900, FUJITA Tomonori wrote:
> >> > Prevent downstream crates or drivers from implementing `ClockSource`
> >> > for arbitrary types, which could otherwise leads to unsupported
> >> > behavior.
> >> >
> >>
> >> Hmm.. I don't think other impl of `ClockSource` is a problem, IIUC, as
> >> long as the ktime_get() can return a value in [0, i64::MAX). Also this
> >> means ClockSource should be an `unsafe` trait, because the correct
> >> implementaion relies on ktime_get() returns the correct value. This is
> >> needed even if you sealed ClockSource trait.
> >>
> >> Could you drop this and fix that the ClockSource trait instead? Thanks!
> >>
> >
> > For example:
> >
> >     /// Trait for clock sources.
> >     ///
> >     /// ...
> >     /// # Safety
> >     ///
> >     /// Implementers must ensure `ktime_get()` return a value in [0,
> >     //  KTIME_MAX (i.e. i64::MAX)).
> >     pub unsafe trait ClockSource {
> >         ...
> >     }
> 
> Nice catch, it definitely needs to be unsafe. We should also require
> correlation between ID and the value fetched by `ktime_get`.
> 
> But I still think it is fine to seal the trait, why not?
> 

There could be potential users of a customized clock source, for
example, a device which also has a timestamp register itself:

	https://lore.kernel.org/rust-for-linux/Z9xb1r1x5tOzAIZT@boqun-archlinux/

So I think with ClockSource being unsafe and well documented, making it
not sealed wouldn't be a problem. IMO, sealing is for the cases where we
must not have downstream impls, ClockSource is not such a case.

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
>