linux-kernel - Re: [PATCH 1/2] x86/fpu: Reset MXCSR to default in 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: <20200622185954.GK32200@zn.tnic>
Date:   Mon, 22 Jun 2020 20:59:54 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Dave Hansen <dave.hansen@...el.com>, X86 ML <x86@...nel.org>,
        jpa@...nelbug.mail.kapsi.fi, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] x86/fpu: Reset MXCSR to default in kernel_fpu_begin()

On Mon, Jun 22, 2020 at 11:40:38AM -0700, Andy Lutomirski wrote:
> On Mon, Jun 22, 2020 at 11:38 AM Dave Hansen <dave.hansen@...el.com> wrote:
> >
> > On 6/22/20 11:33 AM, Andy Lutomirski wrote:
> > > Suppose you do:
> > >
> > > double x = 1.0;
> > >
> > > kernel_fpu_begin();
> > >
> > > x += 2.0;
> > >
> > > We want to make sure that GCC puts things in the right order.  I
> > > suppose that even a memory clobber is insufficient here, though.
> >
> > Even with CONFIG_PREEMPT disabled, we still have:
> >
> >         #define preempt_disable()                       barrier()
> >
> > I don't see us supporting preemptible kernel_fpu regions any time soon,
> > so shouldn't this be sufficient now and for a long time?
> 
> That's on the wrong end of the function.  It'sL
> 
> preempt_disable();
> LDMXCSR;
> <-- some kind of barrier here might be nice

For what? LDMXCSR loads the MXCSR *hardware* register with a u32 from
memory.

In any case, I'm still unconvinced we should preemptively fix issues
that might potentially happen when someone uses FPU in the kernel. And
even if, this doesn't belong in this patch but in a separate one with a
commit message explaining what is being fixed.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette