linux-kernel - Re: [PATCH 4/7] x86/idle: Disable IBRS entering idle and enable it on wakeup

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: <20180104230029.GZ13348@redhat.com>
Date:   Fri, 5 Jan 2018 00:00:29 +0100
From:   Andrea Arcangeli <aarcange@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Tim Chen <tim.c.chen@...ux.intel.com>,
        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>,
        Andi Kleen <ak@...ux.intel.com>,
        Arjan Van De Ven <arjan.van.de.ven@...el.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/7] x86/idle: Disable IBRS entering idle and enable it
 on wakeup

On Thu, Jan 04, 2018 at 11:47:31PM +0100, Peter Zijlstra wrote:
> Argh.. no. Who is calling this with IRQs enabled? And why can't we frob
> the MSR with IRQs enabled? That comment doesn't seem to explain
> anything.

Why we can't is easy to explain, the irq handler would run in such
case and that isn't using save paranoid, it relies on KERNEL_CS and it
assumes IBRS already set.

The irqs_disabled() check can be dropped if you do enough verification
that it never happens. Initially it wasn't obvious the irq disabled
invariant would be always enforced from the multitude of callers it
has (and that varies on different codebases). I didn't want to deal
with such an occurrence and risk even more trouble. Later I did the
verifications and I dropped the irqs_disabled() too.

It should be possible to drop it but it generally doesn't hurt to
start more obviously safe and optimize it later.