linux-kernel - Re: [PATCH v7 04/27] x86/fpu/xstate: Introduce XSAVES system states

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: <2F0417F1-DA1E-4632-AFA2-757C09B3C4B4@amacapital.net>
Date:   Thu, 6 Jun 2019 18:54:56 -0700
From:   Andy Lutomirski <luto@...capital.net>
To:     Dave Hansen <dave.hansen@...el.com>
Cc:     Yu-cheng Yu <yu-cheng.yu@...el.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-mm@...ck.org,
        linux-arch@...r.kernel.org, linux-api@...r.kernel.org,
        Arnd Bergmann <arnd@...db.de>,
        Balbir Singh <bsingharora@...il.com>,
        Borislav Petkov <bp@...en8.de>,
        Cyrill Gorcunov <gorcunov@...il.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Eugene Syromiatnikov <esyr@...hat.com>,
        Florian Weimer <fweimer@...hat.com>,
        "H.J. Lu" <hjl.tools@...il.com>, Jann Horn <jannh@...gle.com>,
        Jonathan Corbet <corbet@....net>,
        Kees Cook <keescook@...omium.org>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Nadav Amit <nadav.amit@...il.com>,
        Oleg Nesterov <oleg@...hat.com>, Pavel Machek <pavel@....cz>,
        Peter Zijlstra <peterz@...radead.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>,
        Vedvyas Shanbhogue <vedvyas.shanbhogue@...el.com>,
        Dave Martin <Dave.Martin@....com>
Subject: Re: [PATCH v7 04/27] x86/fpu/xstate: Introduce XSAVES system states

> On Jun 6, 2019, at 3:08 PM, Dave Hansen <dave.hansen@...el.com> wrote:
> 
> 
> 
> On 6/6/19 3:04 PM, Andy Lutomirski wrote:
>>> But, that seems broken.  If we have supervisor state, we can't 
>>> always defer the load until return to userspace, so we'll never?? 
>>> have TIF_NEED_FPU_LOAD.  That would certainly be true for 
>>> cet_kernel_state.
>> 
>> Ugh. I was sort of imagining that we would treat supervisor state
> completely separately from user state.  But can you maybe give
> examples of exactly what you mean?

I was imagining a completely separate area in memory for supervisor states.  I guess this might defeat the modified optimization and is probably a bad idea.

>> 
>>> It seems like we actually need three classes of XSAVE states: 1. 
>>> User state
>> 
>> This is FPU, XMM, etc, right?
> 
> Yep.
> 
>>> 2. Supervisor state that affects user mode
>> 
>> User CET?
> 
> Yep.
> 
>>> 3. Supervisor state that affects kernel mode
>> 
>> Like supervisor CET?  If we start doing supervisor shadow stack, the 
>> context switches will be real fun.  We may need to handle this in 
>> asm.
> 
> Yeah, that's what I was thinking.
> 
> I have the feeling Yu-cheng's patches don't comprehend this since
> Sebastian's patches went in after he started working on shadow stacks.

Do we need to have TIF_LOAD_FPU mean “we need to load *some* of the xsave state”?  If so, maybe a bunch of the accessors should have their interfaces reviewed to make sure they’re sill sensible.

> 
>> Where does PKRU fit in?  Maybe we can treat it as #3?
> 
> I thought Sebastian added specific PKRU handling to make it always
> eager.  It's actually user state that affect kernel mode. :)

Indeed.  But, if we document a taxonomy of states, we should make sure it fits in. I guess it’s like supervisor CET except that user code can can also read and write it.

We should probably have self tests that make sure that the correct states, and nothing else, show up in ptrace and signal states, and that trying to write supervisor CET via ptrace and sigreturn is properly rejected.

Just to double check my mental model: it’s okay to XSAVES twice to the same buffer with disjoint RFBM as long as we do something intelligent with XSTATE_BV afterwards, right?  Because, unless we split up the buffers, I think we will have to do this when we context switch while TIF_LOAD_FPU is set.

Are there performance numbers for how the time needed to XRSTORS everything versus the time to XRSTORS supervisor CET and then separately XRSTORS the FPU?  This may affect whether we want context switches to have the new task eagerly or lazily restored.

Hmm. I wonder if we need some way for a selftest to reliably trigger TIF_LOAD_FPU.

—Andy