linux-kernel - Re: [PATCH 1/3] timekeeping: NMI safe converter from a given time to monotonic

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: <CANDhNCpWwxXM8DD9h4zOW+bygshkOg9TWO9Z7wJO_B7bDtgEHw@mail.gmail.com>
Date:   Mon, 23 Jan 2023 23:01:18 -0800
From:   John Stultz <jstultz@...gle.com>
To:     kan.liang@...ux.intel.com
Cc:     peterz@...radead.org, mingo@...hat.com, tglx@...utronix.de,
        sboyd@...nel.org, linux-kernel@...r.kernel.org, eranian@...gle.com,
        namhyung@...nel.org, ak@...ux.intel.com
Subject: Re: [PATCH 1/3] timekeeping: NMI safe converter from a given time to monotonic

On Mon, Jan 23, 2023 at 10:27 AM <kan.liang@...ux.intel.com> wrote:
> +int notrace get_mono_fast_from_given_time(int (*get_time_fn)
> +                                               (struct system_counterval_t *sys_counterval,
> +                                               void *ctx),
> +                                         void *ctx,
> +                                         u64 *mono_ns)
> +{
> +       struct system_counterval_t system_counterval;
> +       struct tk_fast *tkf = &tk_fast_mono;
> +       u64 cycles, now, interval_start;
> +       struct tk_read_base *tkr;
> +       unsigned int seq;
> +       int ret;
> +
> +       do {
> +               seq = raw_read_seqcount_latch(&tkf->seq);
> +               tkr = tkf->base + (seq & 0x01);
> +
> +               ret = get_time_fn(&system_counterval, ctx);
> +               if (ret)
> +                       return ret;
> +
> +               /*
> +                * Verify that the clocksource associated with the given
> +                * timestamp is the same as the currently installed
> +                * timekeeper clocksource
> +                */
> +               if (tkr->clock != system_counterval.cs)
> +                       return -EOPNOTSUPP;
> +               cycles = system_counterval.cycles;
> +
> +               /*
> +                * Check whether the given timestamp is on the current
> +                * timekeeping interval.
> +                */
> +               now = tk_clock_read(tkr);
> +               interval_start = tkr->cycle_last;
> +               if (!cycle_between(interval_start, cycles, now))
> +                       return -EOPNOTSUPP;

So. I've not fully thought this out, but it seems like it would be
quite likely that you'd run into the case where the cycle_last value
is updated and your earlier TSC timestamp isn't valid for the current
interval. The get_device_system_crosststamp() logic has a big chunk of
complex code to try to handle this case by interpolating the cycle
value back in time. How well does just failing in this case work out?

thanks
-john