linux-kernel - Re: Lazy FPU restoration / moving kernel_fpu

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: <63A4C722-A37E-49A8-96AC-57BFD0B2DAAB@amacapital.net>
Date:   Fri, 15 Jun 2018 13:52:25 -0700
From:   Andy Lutomirski <luto@...capital.net>
To:     Dave Hansen <dave.hansen@...ux.intel.com>
Cc:     "Jason A. Donenfeld" <Jason@...c4.com>,
        Andrew Lutomirski <luto@...nel.org>, riel@...riel.com,
        LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>
Subject: Re: Lazy FPU restoration / moving kernel_fpu_end() to context switch



> On Jun 15, 2018, at 1:42 PM, Dave Hansen <dave.hansen@...ux.intel.com> wrote:
> 
>> On 06/15/2018 01:33 PM, Jason A. Donenfeld wrote:
>>> On Fri, Jun 15, 2018 at 8:32 PM Andy Lutomirski <luto@...nel.org> wrote:
>>> quite in the form you imagined.  The idea that we've tossed around is
>>> to restore FPU state on return to user mode.  Roughly, we'd introduce
>>> a new thread flag TIF_FPU_UNLOADED (name TBD).
>>> prepare_exit_to_usermode() would notice this flag, copy the fpstate to
>>> fpregs, and clear the flag.  (Or maybe exit_to_usermode_loop() -- No
>>> one has quite thought it through, but I think it should be outside the
>>> loop.)  We'd update all the FPU accessors to understand the flag.
>> Yes! This is exactly what I was thinking. Then those calls to begin()
>> and end() could be placed as close to the actual FPU usage as
>> possible.
> 
> Andy, what was the specific concern about PKRU?  That we might do:
> 
>    kernel_fpu_begin(); <- Saves the first time
>    something()
>    kernel_fpu_end(); <- Does not XRSTOR
> 
>    copy_from_user(); <- Sees old PKRU, does the wrong thing
> 
>    prepare_exit_to_usermode(); <- Does the XRSTOR
>    // only now does PKRU have the right value
>    SYSRET/IRET
> 
> ?
> 
> Does that *matter* unless something() modified PKRU?  We could just make
> the rule that nobody is supposed to mess with it and that it's not
> covered by kernel_fpu_begin/end() semantics.  We could even
> theoretically enforce that in a debug environment if we watch its value.

Indeed, this case seems highly unlikely. I was imagining we have two tasks. Task A enters the kernel and sleeps. Task B runs and gets its PKRU loaded. Then it enters the kernel and we switch back to A. Now we have A’s FPU state in memory and B’s PKRU in the register. If A does copy_to_user, we lose.