linux-kernel - Re: [RFC PATCH v2 07/10] lib: vdso: don't use READ_ONCE() in __c_kernel

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, 24 Dec 2019 20:04:33 +0800
From:   Andy Lutomirski <luto@...capital.net>
To:     christophe leroy <christophe.leroy@....fr>
Cc:     Andy Lutomirski <luto@...nel.org>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Arnd Bergmann <arnd@...db.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Vincenzo Frascino <vincenzo.frascino@....com>,
        LKML <linux-kernel@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        "open list:MIPS" <linux-mips@...r.kernel.org>,
        X86 ML <x86@...nel.org>
Subject: Re: [RFC PATCH v2 07/10] lib: vdso: don't use READ_ONCE() in __c_kernel_time()


> On Dec 24, 2019, at 7:12 PM, christophe leroy <christophe.leroy@....fr> wrote:
> 
> 
> 
>> Le 24/12/2019 à 02:58, Andy Lutomirski a écrit :
>>> On Mon, Dec 23, 2019 at 6:31 AM Christophe Leroy
>>> <christophe.leroy@....fr> wrote:
>>> 
>>> READ_ONCE() forces the read of the 64 bit value of
>>> vd[CS_HRES_COARSE].basetime[CLOCK_REALTIME].sec allthough
>>> only the lower part is needed.
>> Seems reasonable and very unlikely to be harmful.  That being said,
>> this function really ought to be considered deprecated -- 32-bit
>> time_t is insufficient.
>> Do you get even better code if you move the read into the if statement?
> 
> Euh ...
> 
> How can you return t when time pointer is NULL if you read t only when time pointer is not NULL ?
> 
> 

Duh, never mind.

But this means your patch may be buggy: you need to make sure the compiler returns the *same* value it stores. Maybe you’re saved by the potential aliasing between the data page and the passed parameter and the value you read, but that’sa bad thing to rely on.

Try barrier() after the read.