linux-kernel - Re: [RFC PATCH 1/2] time/timekeeping: Fix possible inconsistencies in

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANDhNCqzHYJfaap5-TYPmtbaPm6AkS85hKdeSnprhEqMCAYcPw@mail.gmail.com>
Date: Sat, 15 Mar 2025 16:22:31 -0700
From: John Stultz <jstultz@...gle.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: LKML <linux-kernel@...r.kernel.org>, Stephen Boyd <sboyd@...nel.org>, 
	Anna-Maria Behnsen <anna-maria@...utronix.de>, Frederic Weisbecker <frederic@...nel.org>, 
	Shuah Khan <shuah@...nel.org>, Miroslav Lichvar <mlichvar@...hat.com>, linux-kselftest@...r.kernel.org, 
	kernel-team@...roid.com, Lei Chen <lei.chen@...rtx.com>
Subject: Re: [RFC PATCH 1/2] time/timekeeping: Fix possible inconsistencies in
 _COARSE clockids

On Sat, Mar 15, 2025 at 12:23 PM Thomas Gleixner <tglx@...utronix.de> wrote:
> On Fri, Mar 14 2025 at 17:37, John Stultz wrote:
> > Here because we did not accumulate anything, we have a window
> > where the _COARSE clockids that don't utilize the mult*offset
> > value, can see an inconsistency.
> >
> > So to fix this, rework the timekeeping_advance() logic a bit
> > so that when we are called from do_adjtimex() and the offset
> > is smaller then cycle_interval, that we call
> > timekeeping_forward(), to first accumulate the sub-interval
> > time into xtime_nsec. Then with no unaccumulated cycles in
> > offset, we can do the mult adjustment without worry of the
> > subtraction having an impact.
>
> It's a smart solution. I briefly pondered something similar, but I'm not
> really fond of the fact, that it causes a clock_was_set() event for no
> good reason.
>
> clock_was_set() means that there is a time jump. But that's absolutely
> not the case with do_adjtimex() changing the frequency for quick
> adjustments. That does not affect continuity at all.

Oh, good point.  I wasn't thinking clearly about the semantics, and
was being a little paranoid there (as most calls to
timekeeping_forward_now() have clock_was_set() calls that follow). I
suspect I can do away with that bit and avoid it.  I'll respin later
this week.

> That event causes avoidable overhead in the kernel, but it's also
> exposed to user space via timerfd TFD_TIMER_CANCEL_ON_SET.
>
> I have no really strong opinion about that, but the route through
> clock_was_set() triggers my semantical mismatch sensors :)

Yeah, it's a fair point, thanks for raising it!

> > NOTE: This was implemented as a potential alternative to
> > Thomas' approach here:
> >    https://lore.kernel.org/lkml/87cyej5rid.ffs@tglx/
> >
> > And similarly, it needs some additional review and testing,
> > as it was developed while packing for conference travel.
>
> We can debate that next week over your favourite beverage :)

Looking forward to it :)
-john