linux-kernel - Re: [RFC PATCH v2 3/6] timekeeping: Fix cross-timestamp interpolation for non-x86

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]

Date:   Wed, 13 Sep 2023 11:11:33 +0200
From:   Peter Hilber <peter.hilber@...nsynergy.com>
To:     John Stultz <jstultz@...gle.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Stephen Boyd <sboyd@...nel.org>,
        "Christopher S. Hall" <christopher.s.hall@...el.com>
Subject: Re: [RFC PATCH v2 3/6] timekeeping: Fix cross-timestamp interpolation
 for non-x86

On Fri, Aug 25, 2023 6:04 John Stultz <jstultz@...gle.com> wrote:
> On Thu, Aug 17, 2023 at 6:20 PM Peter Hilber
> <peter.hilber@...nsynergy.com> wrote:
>>
>> So far, get_device_system_crosststamp() unconditionally passes
>> system_counterval.cycles to timekeeping_cycles_to_ns(). But when
>> interpolating system time (do_interp == true), system_counterval.cycles is
>> before tkr_mono.cycle_last, contrary to the timekeeping_cycles_to_ns()
>> expectations.
>>
>> On x86, CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE will mitigate on
>> interpolating, setting delta to 0. With delta == 0, xtstamp->sys_monoraw
>> and xtstamp->sys_realtime are then set to the last update time, as
>> implicitly expected by adjust_historical_crosststamp(). On other
>> architectures, the resulting nonsense xtstamp->sys_monoraw and
>> xtstamp->sys_realtime corrupt the xtstamp (ts) adjustment in
>> adjust_historical_crosststamp().
>>
>> Fix this by deriving xtstamp->sys_monoraw and xtstamp->sys_realtime from
>> the last update time when interpolating, by using the local variable
>> "cycles". The local variable already has the right value when
>> interpolating, unlike system_counterval.cycles.
>>
>> Fixes: 2c756feb18d9 ("time: Add history to cross timestamp interface supporting slower devices")
>> Signed-off-by: Peter Hilber <peter.hilber@...nsynergy.com>
>
> Thanks again for iterating on this.  This looks much better!
> 
> Now, I've never had an environment that used this logic, so I'm
> trusting you've tested it well?
> 
> Assuming so:
> Acked-by: John Stultz <jstultz@...gle.com>

Thanks for re-reviewing!

I did automated tests with various chrony [1] configurations. The tests
check that all PTP_SYS_OFFSET_PRECISE2 ioctls issued by chrony are
successful for a combined test time of many hours, and that the
cross-timestamps look plausible.

I will add a description of the relevant tests when changing the series to
non-RFC.

[1] https://chrony-project.org/