linux-kernel - Re: [PATCH v2 06/39] x86/fpu: Add helper for modifying xstate

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: <e0b3662ac478a7d2ae1991e8c674732592c2ea88.camel@intel.com>
Date:   Mon, 3 Oct 2022 20:05:13 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "keescook@...omium.org" <keescook@...omium.org>
CC:     "bsingharora@...il.com" <bsingharora@...il.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "Syromiatnikov, Eugene" <esyr@...hat.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "Eranian, Stephane" <eranian@...gle.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "fweimer@...hat.com" <fweimer@...hat.com>,
        "nadav.amit@...il.com" <nadav.amit@...il.com>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "dethoma@...rosoft.com" <dethoma@...rosoft.com>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        "kcc@...gle.com" <kcc@...gle.com>, "bp@...en8.de" <bp@...en8.de>,
        "oleg@...hat.com" <oleg@...hat.com>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "Yang, Weijiang" <weijiang.yang@...el.com>,
        "Lutomirski, Andy" <luto@...nel.org>,
        "pavel@....cz" <pavel@....cz>, "arnd@...db.de" <arnd@...db.de>,
        "Moreira, Joao" <joao.moreira@...el.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mike.kravetz@...cle.com" <mike.kravetz@...cle.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "jamorris@...ux.microsoft.com" <jamorris@...ux.microsoft.com>,
        "john.allen@....com" <john.allen@....com>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        "corbet@....net" <corbet@....net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "gorcunov@...il.com" <gorcunov@...il.com>
Subject: Re: [PATCH v2 06/39] x86/fpu: Add helper for modifying xstate

On Mon, 2022-10-03 at 10:48 -0700, Kees Cook wrote:
> > The easiest way to modify supervisor xfeature data is to force
> > restore
> > the registers and write directly to the MSRs. Often times this is
> > just fine
> > anyway as the registers need to be restored before returning to
> > userspace.
> > Do this for now, leaving buffer writing optimizations for the
> > future.
> 
> Just for my own clarity, does this mean lock/load _needs_ to happen
> before MSR access, or is it just a convenient place to do it? From
> later
> patches it seems it's a requirement during MSR access, which might be
> a
> good idea to detail here. It answers the question "when is this
> function
> needed?"

The CET state is xsaves managed. It gets lazily restored before
returning to userspace with the rest of the fpu stuff. This function
will force restore all the fpu state to the registers early and lock
them from being automatically saved/restored. Then the tasks CET state
can be modified in the MSRs, before unlocking the fpregs. Last time I
tried to modify the state directly in the xsave buffer when it was
efficient, but it had issues and Thomas suggested this.

> 
> > 
> > Add a new function fpregs_lock_and_load() that can simultaneously
> > call
> > fpregs_lock() and do this restore. Also perform some extra sanity
> > checks in this function since this will be used in non-fpu focused
> > code.
> 
> Nit: this is called "fpu_lock_and_load" in the patch itself.

Oops, thanks.