netdev - Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative execution

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180118131837.GA20783@arm.com>
Date:   Thu, 18 Jan 2018 13:18:37 +0000
From:   Will Deacon <will.deacon@....com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Mark Rutland <mark.rutland@....com>,
        kernel-hardening@...ts.openwall.com,
        Peter Zijlstra <peterz@...radead.org>,
        Alan Cox <alan.cox@...el.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Solomon Peachy <pizza@...ftnet.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Christian Lamparter <chunkeey@...glemail.com>,
        Elena Reshetova <elena.reshetova@...el.com>,
        linux-arch@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>,
        "James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
        Linux SCSI List <linux-scsi@...r.kernel.org>,
        Jonathan Corbet <corbet@....net>,
        the arch/x86 maintainers <x86@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Ingo Molnar <mingo@...hat.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Alexey Kuznetsov <kuznet@....inr.ac.ru>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Tom Lendacky <thomas.lendacky@....com>,
        Kees Cook <keescook@...omium.org>, Jan Kara <jack@...e.com>,
        Al Viro <viro@...iv.linux.org.uk>, qla2xxx-upstream@...gic.com,
        Thomas Gleixner <tglx@...utronix.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        Alan Cox <alan@...ux.intel.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Linux Wireless List <linux-wireless@...r.kernel.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Network Development <netdev@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "David S. Miller" <davem@...emloft.net>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative
 execution

Hi Dan, Linus,

On Thu, Jan 11, 2018 at 05:41:08PM -0800, Dan Williams wrote:
> On Thu, Jan 11, 2018 at 5:19 PM, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> > On Thu, Jan 11, 2018 at 4:46 PM, Dan Williams <dan.j.williams@...el.com> wrote:
> >>
> >> This series incorporates Mark Rutland's latest ARM changes and adds
> >> the x86 specific implementation of 'ifence_array_ptr'. That ifence
> >> based approach is provided as an opt-in fallback, but the default
> >> mitigation, '__array_ptr', uses a 'mask' approach that removes
> >> conditional branches instructions, and otherwise aims to redirect
> >> speculation to use a NULL pointer rather than a user controlled value.
> >
> > Do you have any performance numbers and perhaps example code
> > generation? Is this noticeable? Are there any microbenchmarks showing
> > the difference between lfence use and the masking model?
> 
> I don't have performance numbers, but here's a sample code generation
> from __fcheck_files, where the 'and; lea; and' sequence is portion of
> array_ptr() after the mask generation with 'sbb'.
> 
>         fdp = array_ptr(fdt->fd, fd, fdt->max_fds);
>      8e7:       8b 02                   mov    (%rdx),%eax
>      8e9:       48 39 c7                cmp    %rax,%rdi
>      8ec:       48 19 c9                sbb    %rcx,%rcx
>      8ef:       48 8b 42 08             mov    0x8(%rdx),%rax
>      8f3:       48 89 fe                mov    %rdi,%rsi
>      8f6:       48 21 ce                and    %rcx,%rsi
>      8f9:       48 8d 04 f0             lea    (%rax,%rsi,8),%rax
>      8fd:       48 21 c8                and    %rcx,%rax
> 
> 
> > Having both seems good for testing, but wouldn't we want to pick one in the end?
> 
> I was thinking we'd keep it as a 'just in case' sort of thing, at
> least until the 'probably safe' assumption of the 'mask' approach has
> more time to settle out.

>From the arm64 side, the only concern I have (and this actually applies to
our CSDB sequence as well) is the calculation of the array size by the
caller. As Linus mentioned at the end of [1], if the determination of the
size argument is based on a conditional branch, then masking doesn't help
because you bound within the wrong range under speculation.

We ran into this when trying to use masking to protect our uaccess routines
where the conditional bound is either KERNEL_DS or USER_DS. It's possible
that a prior conditional set_fs(KERNEL_DS) could defeat the masking and so
we'd need to throw some heavy barriers in set_fs to make it robust.

The question then is whether or not we're likely to run into this elsewhere,
and I don't have a good feel for that.

Will

[1] http://lkml.kernel.org/r/CA+55aFz0tsreoa=5Ud2noFCpng-dizLBhT9WU9asyhpLfjdcYA@mail.gmail.com