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  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, 26 Jun 2019 08:38:48 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Vincenzo Frascino <vincenzo.frascino@....com>
cc:     linux-arch@...r.kernel.org,
        LAK <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>, linux-mips@...r.kernel.org,
        linux-kselftest@...r.kernel.org, catalin.marinas@....com,
        Will Deacon <will.deacon@....com>,
        Arnd Bergmann <arnd@...db.de>, linux@...linux.org.uk,
        Ralf Baechle <ralf@...ux-mips.org>, paul.burton@...s.com,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        salyzyn@...roid.com, pcc@...gle.com, shuah@...nel.org,
        0x7f454c46@...il.com, linux@...musvillemoes.dk,
        huw@...eweavers.com, sthotton@...vell.com, andre.przywara@....com,
        Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCH 1/3] lib/vdso: Delay mask application in do_hres()

On Tue, 25 Jun 2019, Thomas Gleixner wrote:
> On Tue, 25 Jun 2019, Vincenzo Frascino wrote:
> > do_hres() in the vDSO generic library masks the hw counter value
> > immediately after reading it.
> > 
> > Postpone the mask application after checking if the syscall fallback is
> > enabled, in order to be able to detect a possible fallback for the
> > architectures that have masks smaller than ULLONG_MAX.
> 
> Right. This only worked on x86 because the mask is there ULLONG_MAX for all
> VDSO capable clocksources, i.e. that ever worked just by chance.

But it's actually worse than that:

> > +		cycles &= vd->mask;
> >  		if (cycles > last)
> >  			ns += (cycles - last) * vd->mult;
> >  		ns >>= vd->shift;

This is broken for any clocksource which can legitimately wrap around. The
core timekeeping does the right thing:

     		 (cycles - last) & mask

That makes sure that a wraparound is correctly handled. With the above the
wrap around would be ignored due to

     	    if (cycles > last)

Stupid me. I should have added big fat comments to the x86 vdso why this
all works correctly and only correctly for the x86 crud. That was part of
squeezing the last cycles out of the vdso.

Sorry for not noticing earlier. Working on a fix.

Thanks,

	tglx


Powered by blists - more mailing lists