linux-kernel - Re: [regression in -rc1] Re: [PATCH v6 2/8] x86/fsgsbase/64: Introduce FS/GS base helper functions

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] [day] [month] [year] [list]

Message-ID: <CALCETrUGqfpcu6gf2931f9kfkzirTCojANt3BncTY7h9usQYtA@mail.gmail.com>
Date:   Thu, 25 Oct 2018 15:37:19 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Andrew Lutomirski <luto@...nel.org>
Cc:     "Bae, Chang Seok" <chang.seok.bae@...el.com>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>, Andi Kleen <ak@...ux.intel.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "Metzger, Markus T" <markus.t.metzger@...el.com>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [regression in -rc1] Re: [PATCH v6 2/8] x86/fsgsbase/64:
 Introduce FS/GS base helper functions

On Wed, Oct 24, 2018 at 12:01 PM Andy Lutomirski <luto@...nel.org> wrote:
>
> On Tue, Sep 18, 2018 at 4:09 PM Chang S. Bae <chang.seok.bae@...el.com> wrote:
> >
> > With new helpers, FS/GS base access is centralized.
> > Eventually, when FSGSBASE instruction enabled, it will
> > be faster.
>
> Sorry for not catching this during review, but:
>
> > +void x86_fsbase_write_cpu(unsigned long fsbase)
> > +{
> > +       /*
> > +        * Set the selector to 0 as a notion, that the segment base is
> > +        * overwritten, which will be checked for skipping the segment load
> > +        * during context switch.
> > +        */
> > +       loadseg(FS, 0);
>
> ^^^
>
> what?
>
> > +       wrmsrl(MSR_FS_BASE, fsbase);
> > +}
>
> I don't understand what the comment is trying to say, but the sole
> caller so far of this function is x86_gsbase_write_task(), and the
> code looks incorrect.
>
> Ingo, I think we need to address this during this merge window,
> probably by removing the comment and the loadseg() call (and the same
> for gsbase...inactive).  But first, Chang, can you explain what
> exactly your intent is here?

It might not be a problem for the current merge window, since the one
and only caller (I think) that hits this code is okay with it.  But it
might still be nice to have it cleaned up in Linus' tree.