linux-kernel - Re: [PATCH v2 3/8] x86/enter: Use IBRS on syscall and interrupts

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]

Message-ID: <20180107192721.tyefgor22426vvfm@pd.tnic>
Date:   Sun, 7 Jan 2018 20:27:21 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Tim Chen <tim.c.chen@...ux.intel.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Andy Lutomirski <luto@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Dave Hansen <dave.hansen@...el.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Arjan Van De Ven <arjan.van.de.ven@...el.com>,
        David Woodhouse <dwmw@...zon.co.uk>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/8] x86/enter: Use IBRS on syscall and interrupts

On Fri, Jan 05, 2018 at 06:12:18PM -0800, Tim Chen wrote:
> From: Andrea Arcangeli <aarcange@...hat.com>
> 
> Set IBRS upon kernel entrance via syscall and interrupts. Clear it
> upon exit.  IBRS protects against unsafe indirect branching predictions
> in the kernel.
> 
> The NMI interrupt save/restore of IBRS state was based on Andrea
> Arcangeli's implementation.
> Here's an explanation by Dave Hansen on why we save IBRS state for NMI.
> 
> The normal interrupt code uses the 'error_entry' path which uses the
> Code Segment (CS) of the instruction that was interrupted to tell
> whether it interrupted the kernel or userspace and thus has to switch
> IBRS, or leave it alone.
> 
> The NMI code is different.  It uses 'paranoid_entry' because it can
> interrupt the kernel while it is running with a userspace IBRS (and %GS
> and CR3) value, but has a kernel CS.  If we used the same approach as
> the normal interrupt code, we might do the following;
> 
> 	SYSENTER_entry
> <-------------- NMI HERE
> 	IBRS=1
> 		do_something()
> 	IBRS=0
> 	SYSRET
> 
> The NMI code might notice that we are running in the kernel and decide
> that it is OK to skip the IBRS=1.  This would leave it running
> unprotected with IBRS=0, which is bad.
> 
> However, if we unconditionally set IBRS=1, in the NMI, we might get the
> following case:
> 
> 	SYSENTER_entry
> 	IBRS=1
> 		do_something()
> 	IBRS=0
> <-------------- NMI HERE (set IBRS=1)
> 	SYSRET
> 
> and we would return to userspace with IBRS=1.  Userspace would run
> slowly until we entered and exited the kernel again.
> 
> Instead of those two approaches, we chose a third one where we simply
> save the IBRS value in a scratch register (%r13) and then restore that
> value, verbatim.

That's one helluva commit message. This is how you write commit
messages!

> Signed-off-by: Andrea Arcangeli <aarcange@...hat.com>
> Signed-off-by: Tim Chen <tim.c.chen@...ux.intel.com>
> ---
>  arch/x86/entry/entry_64.S        | 23 +++++++++++++++++++++++
>  arch/x86/entry/entry_64_compat.S |  8 ++++++++
>  2 files changed, 31 insertions(+)

Reviewed-by: Borislav Petkov <bp@...e.de>

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.