kernel-hardening - Re: [PATCH v3 2/4] x86/syscalls: Specific usage of verify_pre_usermode

Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 14 Mar 2017 09:30:34 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Andy Lutomirski <luto@...capital.net>,
        Thomas Garnier <thgarnie@...gle.com>
Cc: Ingo Molnar <mingo@...nel.org>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        David Howells <dhowells@...hat.com>, Arnd Bergmann <arnd@...db.de>,
        Al Viro <viro@...iv.linux.org.uk>, Dave Hansen <dave.hansen@...el.com>,
        René Nyffenegger <mail@...enyffenegger.ch>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kees Cook
 <keescook@...omium.org>,
        "Paul E . McKenney" <paulmck@...ux.vnet.ibm.com>,
        Andy Lutomirski <luto@...nel.org>,
        Ard Biesheuvel
 <ard.biesheuvel@...aro.org>,
        Nicolas Pitre <nicolas.pitre@...aro.org>,
        Petr Mladek <pmladek@...e.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Helge Deller <deller@....de>, Rik van Riel <riel@...hat.com>,
        John Stultz <john.stultz@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>, Oleg Nesterov <oleg@...hat.com>,
        Stephen Smalley <sds@...ho.nsa.gov>,
        Pavel Tikhomirov <ptikhomirov@...tuozzo.com>,
        Frederic Weisbecker <fweisbec@...il.com>,
        Stanislav Kinsburskiy <skinsbursky@...tuozzo.com>,
        Ingo Molnar <mingo@...hat.com>, Paolo Bonzini <pbonzini@...hat.com>,
        Dmitry Safonov <dsafonov@...tuozzo.com>,
        Borislav Petkov <bp@...en8.de>, Josh Poimboeuf <jpoimboe@...hat.com>,
        Brian Gerst <brgerst@...il.com>, Jan Beulich <JBeulich@...e.com>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        Fenghua Yu <fenghua.yu@...el.com>, He Chen <he.chen@...ux.intel.com>,
        Russell King <linux@...linux.org.uk>,
        Vladimir Murzin <vladimir.murzin@....com>,
        Will Deacon
 <will.deacon@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Mark Rutland <mark.rutland@....com>, James Morse <james.morse@....com>,
        "David A . Long" <dave.long@...aro.org>,
        Pratyush Anand <panand@...hat.com>, Laura Abbott <labbott@...hat.com>,
        Andre Przywara <andre.przywara@....com>,
        Chris Metcalf <cmetcalf@...lanox.com>,
        linux-s390 <linux-s390@...r.kernel.org>,
        LKML
 <linux-kernel@...r.kernel.org>,
        Linux API <linux-api@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>,
        Kernel Hardening <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH v3 2/4] x86/syscalls: Specific usage of
 verify_pre_usermode_state

On 03/14/17 08:39, Andy Lutomirski wrote:
>>
>> Ingo: Which approach do you favor? I want to keep the fast path as
>> fast as possible obviously.
> 
> Even though my name isn't Ingo, Linus keeps trying to get me to be the
> actual maintainer of this file.  :)  How about (sorry about whitespace
> damage):
> 
> #ifdef CONFIG_BUG_ON_DATA_CORRUPTION
>        movq    PER_CPU_VAR(current_task), %rax
>        bt $63, TASK_addr_limit(%rax)
>        jc     syscall_return_slowpath
> #endif
> 
> Now the kernel is totally unchanged if the config option is off and
> it's fast and simple if the option is on.
> 

The idea as far as I understand was that the option was about whether or
not to clobber the broken value or BUG on it, not to remove the check.
My point, though, was that we can bail out to the slow path if there is
a discrepancy and worry about BUG or not there; performance doesn't
matter one iota if this triggers regardless of the remediation.

It isn't clear that using bt would be faster, though; although it saves
an instruction that instruction can be hoisted arbitrarily and so is
extremely likely to be hidden in the pipeline.  cmp (which is really a
variant of sub) is one of the basic ALU instructions that are
super-optimized on every CPU, whereas bt is substantially slower on some
implementations.

This version is also "slightly less secure" since it would make it
possible to overwrite the guard page at the end of TASK_SIZE_MAX if one
could figure out a way to put an arbitrary value into this variable, but
I doubt that matters in any way.

	-hpa
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.