linux-kernel - Re: infinite loop in read_hpet from ktime_get_boot_fast

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: <CAHmME9qa-8J0-x8zmcBrSg_iyPNS02h5CFvhFfXpNth60OQsBg@mail.gmail.com>
Date:   Fri, 14 Jun 2019 11:14:12 +0200
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Peter Zijlstra <peterz@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Clemens Ladisch <clemens@...isch.de>,
        Sultan Alsawaf <sultan@...neltoast.com>,
        Waiman Long <longman@...hat.com>, X86 ML <x86@...nel.org>
Subject: Re: infinite loop in read_hpet from ktime_get_boot_fast_ns

Hey Thomas,

> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
>         } while (read_seqcount_retry(&tk_core.seq, seq));
>
> -       return base;
> -
> +       return base + nsecs;

The rest of the file seems to use `ktime_add_ns(base, nsecs)`. I
realize, of course, that these days that macro is the same thing as
what you wrote, though.

I can confirm that this fixes it (see below), so you can add my
Tested-by if you want or care.

One thing I'm curious about is the performance comparison with various
ways of using jiffies directly:

ktime_mono_to_any(ns_to_ktime(jiffies64_to_nsecs(get_jiffies_64())),
TK_OFFS_BOOT)

Or really giving up on the locking:

ktime_to_ns(tk_core.timekeeper.offs_boot) + jiffies64_to_nsecs(get_jiffies_64())

Or keeping things in units of jiffies, though that incurs a div_u64:

nsecs_to_jiffies64(ktime_to_ns(tk_core.timekeeper.offs_boot)) + get_jiffies_64()

But since offs_boot is updated somewhat rarely, that div_u64 could be
precomputed each time offs_boot is updated, allowing hypothetically:

tk_core.timekeeper.offs_boot_jiffies + get_jiffies_64()

Which then could be remade into a wrapper such as:

get_jiffies_boot_64()

The speed is indeed an important factor to me in accessing this time
value. Are any of these remotely interesting to you in that light?
Maybe I'll send a patch for the latter.

Jason

8<-----------------

int __init mod_init(void)
{
  u64 j1 = 0, j2, k1 = 0, k2, c1 = 0, c2, l1 = 0, l2;
  for (;;) {
    j2 = jiffies64_to_nsecs(get_jiffies_64());
    k2 = ktime_to_ns(ktime_mono_to_any(ns_to_ktime(jiffies64_to_nsecs(get_jiffies_64())),
TK_OFFS_BOOT));
    c2 = ktime_get_coarse_boottime();
    l2 = local_clock();
    pr_err("%llu %llu %llu %llu\n", j2 - j1, k2 - k1, c2 - c1, l2 - l1);
    j1 = j2;
    k1 = k2;
    c1 = c2;
    l1 = l2;
    msleep(200);
  }
  return 0;
}

[    0.420861] wireguard: 17179569472000000 17179569472000000
312696682 420860781
[    0.628656] wireguard: 208000000 208000000 208000000 207791083
[    0.836591] wireguard: 208000000 208000000 208000000 207934734
[    1.044728] wireguard: 208000000 208000000 208000000 208137167
[    1.252593] wireguard: 208000000 208000000 208000000 207862974
[    1.460815] wireguard: 208000000 208000000 208000000 208223514
[    1.668667] wireguard: 208000000 208000000 208000000 207852437
[    1.876438] wireguard: 208000000 208000000 208000000 207773658
[    2.084627] wireguard: 208000000 208000000 208000000 208185643
[    2.292690] wireguard: 208000000 208000000 208000000 208063377
[    2.500672] wireguard: 208000000 208000000 208000000 207982209
[    2.708658] wireguard: 208000000 208000000 208000000 207986527
[    2.916686] wireguard: 208000000 208000000 208000000 208026945
[    3.124732] wireguard: 208000000 208000000 208000000 208046153
[    3.332684] wireguard: 208000000 208000000 208000000 207952302
[    3.540668] wireguard: 208000000 208000000 208000000 207978195
[    3.748633] wireguard: 208000000 208000000 208000000 207970981
[    3.956686] wireguard: 208000000 208000000 208000000 208053094
[    4.164690] wireguard: 208000000 208000000 208000000 207995376
[    4.372660] wireguard: 208000000 208000000 208000000 207978324
[    4.580787] wireguard: 208000000 208000000 208000000 208126491
[    4.788716] wireguard: 208000000 208000000 208000000 207930106
[    4.996685] wireguard: 208000000 208000000 208000000 207968555
[    5.204673] wireguard: 208000000 208000000 208000000 207988295
[    5.412676] wireguard: 208000000 208000000 208000000 207991396
[    5.620648] wireguard: 208000000 208000000 208000000 207983671
[    5.828822] wireguard: 208000000 208000000 208000000 208174230
[    6.036596] wireguard: 208000000 208000000 208000000 207773401
[    6.244615] wireguard: 208000000 208000000 208000000 208017986
[    6.452625] wireguard: 208000000 208000000 208000000 208011215
[    6.660678] wireguard: 208000000 208000000 208000000 208053134
[    6.868609] wireguard: 208000000 208000000 208000000 207929536