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] [day] [month] [year] [list]

Message-ID: <aFJINI8ImfxMnvrx@Mac.home>
Date: Tue, 17 Jun 2025 22:01:40 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: a.hindborg@...nel.org, 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 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 {
        ...
    }

Regards,
Boqun

> Regards,
> Boqun
> 
> > Introduce a `private::Sealed` trait and implement it for all types
> > that implement `ClockSource`.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@...il.com>
> > ---
> >  rust/kernel/time.rs | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> > index eaa6d9ab5737..b1961652c884 100644
> > --- a/rust/kernel/time.rs
> > +++ b/rust/kernel/time.rs
> > @@ -51,6 +51,15 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
> >      unsafe { bindings::__msecs_to_jiffies(msecs) }
> >  }
> >  
> > +mod private {
> > +    pub trait Sealed {}
> > +
> > +    impl Sealed for super::Monotonic {}
> > +    impl Sealed for super::RealTime {}
> > +    impl Sealed for super::BootTime {}
> > +    impl Sealed for super::Tai {}
> > +}
> > +
> >  /// Trait for clock sources.
> >  ///
> >  /// Selection of the clock source depends on the use case. In some cases the usage of a
> > @@ -58,7 +67,7 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
> >  /// cases the user of the clock has to decide which clock is best suited for the
> >  /// purpose. In most scenarios clock [`Monotonic`] is the best choice as it
> >  /// provides a accurate monotonic notion of time (leap second smearing ignored).
> > -pub trait ClockSource {
> > +pub trait ClockSource: private::Sealed {
> >      /// The kernel clock ID associated with this clock source.
> >      ///
> >      /// This constant corresponds to the C side `clockid_t` value.
> > 
> > base-commit: 994393295c89711531583f6de8f296a30b0d944a
> > -- 
> > 2.43.0
> >