linux-kernel - Re: [PATCH RFC rebase 2/9] powerpc: Use barrier_nospec in copy_from

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:   Sat, 17 Mar 2018 00:22:54 +1100
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Michal Suchanek <msuchanek@...e.de>
Cc:     Kate Stewart <kstewart@...uxfoundation.org>,
        Madhavan Srinivasan <maddy@...ux.vnet.ibm.com>,
        Mahesh Salgaonkar <mahesh@...ux.vnet.ibm.com>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Neuling <mikey@...ling.org>,
        "Bryant G. Ly" <bryantly@...ux.vnet.ibm.com>,
        "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>,
        Daniel Axtens <dja@...ens.net>,
        Nicholas Piggin <npiggin@...il.com>,
        Cédric Le Goater <clg@...d.org>,
        David Gibson <david@...son.dropbear.id.au>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Andrew Donnellan <andrew.donnellan@....ibm.com>,
        Philippe Ombredanne <pombredanne@...b.com>,
        Joe Perches <joe@...ches.com>,
        Oliver O'Halloran <oohall@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Tobin C. Harding" <me@...in.cc>,
        ppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH RFC rebase 2/9] powerpc: Use barrier_nospec in copy_from_user

Linus Torvalds <torvalds@...ux-foundation.org> writes:

> On Thu, Mar 15, 2018 at 12:15 PM, Michal Suchanek <msuchanek@...e.de> wrote:
>> This is based on x86 patch doing the same.
>>
>> Signed-off-by: Michal Suchanek <msuchanek@...e.de>
>> ---
>> --- a/arch/powerpc/include/asm/uaccess.h
>> +++ b/arch/powerpc/include/asm/uaccess.h
>> @@ -258,8 +259,10 @@ do {                                                               \
>>         long __gu_err = -EFAULT;                                        \
>>         unsigned long  __gu_val = 0;                                    \
>>         const __typeof__(*(ptr)) __user *__gu_addr = (ptr);             \
>> +       int can_access = access_ok(VERIFY_READ, __gu_addr, (size));     \
>>         might_fault();                                                  \
>> -       if (access_ok(VERIFY_READ, __gu_addr, (size)))                  \
>> +       barrier_nospec();                                               \
>> +       if (can_access)                                                 \
>>                 __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
>>         (x) = (__force __typeof__(*(ptr)))__gu_val;                             \
>>         __gu_err;                                                       \
>
> Is the above really correct? The barrier is *before* the conditional
> branch that might be mis-predicted.
>
> I don't know how the ppc barrier works, but that sounds completely bogus.

Yeah it should be after the branch.

I don't have a formal spec for the barrier yet, it should be defined in
a hopefully soon to be released revision of the ISA.

But the gist is it will stall execution until any older branches are no
longer speculating.

It doesn't order any two arbitrary instructions, such as a comparison
and a branch, which I suspect is how Michal was interpreting it.

cheers