linux-kernel - Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case

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:   Tue, 18 Sep 2018 16:03:01 -0700
From:   Andy Lutomirski <luto@...capital.net>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     John Stultz <john.stultz@...aro.org>,
        Andy Lutomirski <luto@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Matt Rickard <matt@...trans.com.au>,
        Stephen Boyd <sboyd@...nel.org>,
        Florian Weimer <fweimer@...hat.com>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        devel@...uxdriverproject.org,
        Linux Virtualization <virtualization@...ts.linux-foundation.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Arnd Bergmann <arnd@...db.de>, Juergen Gross <jgross@...e.com>
Subject: Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case



> On Sep 18, 2018, at 3:46 PM, Thomas Gleixner <tglx@...utronix.de> wrote:
> 
> On Tue, 18 Sep 2018, Andy Lutomirski wrote:
>>> On Sep 18, 2018, at 12:52 AM, Thomas Gleixner <tglx@...utronix.de> wrote:
>>> 
>>>>> On Mon, 17 Sep 2018, John Stultz wrote:
>>>>> On Mon, Sep 17, 2018 at 12:25 PM, Andy Lutomirski <luto@...nel.org> wrote:
>>>>> Also, I'm not entirely convinced that this "last" thing is needed at
>>>>> all.  John, what's the scenario under which we need it?
>>>> 
>>>> So my memory is probably a bit foggy, but I recall that as we
>>>> accelerated gettimeofday, we found that even on systems that claimed
>>>> to have synced TSCs, they were actually just slightly out of sync.
>>>> Enough that right after cycles_last had been updated, a read on
>>>> another cpu could come in just behind cycles_last, resulting in a
>>>> negative interval causing lots of havoc.
>>>> 
>>>> So the sanity check is needed to avoid that case.
>>> 
>>> Your memory serves you right. That's indeed observable on CPUs which
>>> lack TSC_ADJUST.
>>> 
>>> @Andy: Welcome to the wonderful world of TSC.
>>> 
>> 
>> Do we do better if we use signed arithmetic for the whole calculation?
>> Then a small backwards movement would result in a small backwards result.
>> Or we could offset everything so that we’d have to go back several
>> hundred ms before we cross zero.
> 
> That would be probably the better solution as signed math would be
> problematic when the resulting ns value becomes negative. As the delta is
> really small, otherwise the TSC sync check would have caught it, the caller
> should never be able to observe time going backwards.
> 
> I'll have a look into that. It needs some thought vs. the fractional part
> of the base time, but it should be not rocket science to get that
> correct. Famous last words...
> 

It’s also fiddly to tune. If you offset it too much, then the fancy divide-by-repeated-subtraction loop will hurt more than the comparison to last.