linux-kernel - Re: [PATCH 11/11] x86/fpu: defer FPU state load until return to userspace

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: <20181012202518.2gnmpgwxesvkna6i@linutronix.de>
Date:   Fri, 12 Oct 2018 22:25:18 +0200
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krcmar <rkrcmar@...hat.com>,
        kvm list <kvm@...r.kernel.org>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Rik van Riel <riel@...riel.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH 11/11] x86/fpu: defer FPU state load until return to
 userspace

On 2018-10-04 09:14:33 [-0700], Andy Lutomirski wrote:
> > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> > index 3b2490b819181..3dad5c3b335eb 100644
> > --- a/arch/x86/entry/common.c
> > +++ b/arch/x86/entry/common.c
> > @@ -196,6 +197,14 @@ __visible inline void prepare_exit_to_usermode(struct pt_regs *regs)
> >         if (unlikely(cached_flags & EXIT_TO_USERMODE_LOOP_FLAGS))
> >                 exit_to_usermode_loop(regs, cached_flags);
> >
> > +       /* Reload ti->flags; we may have rescheduled above. */
> > +       cached_flags = READ_ONCE(ti->flags);
> > +
> > +       if (unlikely(cached_flags & _TIF_LOAD_FPU))
> > +               switch_fpu_return();
> > +       else
> > +               fpregs_is_state_consistent();
> 
> Shouldn't this be:
> 
> fpregs_assert_state_consistent();  /* see below */
> 
> if (unlikely(cached_flags & _TIF_LOAD_FPU))
>   switch_fpu_return();

hmm. This should work. 

> > diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
> > index a9caac9d4a729..e3077860f7333 100644
> > --- a/arch/x86/include/asm/fpu/api.h
> > +++ b/arch/x86/include/asm/fpu/api.h
> > @@ -27,6 +27,17 @@ extern void kernel_fpu_begin(void);
> >  extern void kernel_fpu_end(void);
> >  extern bool irq_fpu_usable(void);
> >
> > +#ifdef CONFIG_X86_DEBUG_FPU
> > +extern void fpregs_is_state_consistent(void);
> > +#else
> > +static inline void fpregs_is_state_consistent(void) { }
> > +#endif
> 
> Can you name this something like fpregs_assert_state_consistent()?
sure.

Sebastian