linux-kernel - Re: [PATCH 06/18] x86, barrier: stop speculation for failed access

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, 6 Jan 2018 20:22:13 +0000
From:   Alan Cox <gnomes@...rguk.ukuu.org.uk>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Dan Williams <dan.j.williams@...el.com>,
        linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
        Andi Kleen <ak@...ux.intel.com>, Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Peter Zijlstra <peterz@...radead.org>, netdev@...r.kernel.org,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 06/18] x86, barrier: stop speculation for failed
 access_ok

> "Value prediction consists of predicting entire 32- and 64-bit register values
> based  on  previously-seen values"

For their implementation yes

> 
> > In other words there are at least two problems with Linus proposal
> > 
> > 1. The ffff/0000 mask has to be generated and that has to involve
> > speculative flows.  
> 
> to answer above and Thomas's
> "For one particular architecture and that's not a solution for generic code."
> 
> The following:
> #define array_access(base, idx, max) ({                         \
>         union { typeof(base[0]) _val; unsigned long _bit; } __u;\
>         unsigned long _i = (idx);                               \
>         unsigned long _m = (max);                               \
>         unsigned long _mask = ~(long)(_m - 1 - _i) >> 63;       \
>         __u._val = base[_i & _mask];                            \
>         __u._bit &= _mask;                                      \
>         __u._val; })
> 
> is generic and no speculative flows.

In the value speculation case imagine it's been called 1000 times for
process A which as a limit of say 16 so that file->priv->max is 16, and
then is run for process B which is different.

A value speculating processor waiting for file->priv->max which has been
pushed out of cache by an attacker is at liberty to say 'I've no idea
what max is but hey it was 16 last time so lets plug 16 in and keep going"

So while the change in the mask computation is clever and (subject to
compiler cleverness) safe against guesses of which path will be taken I
don't think it's generically safe.

Unfortunately a lot of things we index are of different sizes as seen by
different tasks, or when passed different other index values so this does
matter.

> Even if 'mask' in 'index & mask' example is a stall the educated
> guess will come from the prior value (according to the quoted paper)

Which might be for a different set of variables when the table is say per
process like file handles, or the value is different each call.

If we have single array of fixed size then I suspect you are right but
usually we don't.

Alan