linux-kernel - Re: [PATCH v2] x86/cpu: Use SERIALIZE in sync

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: <CALCETrWByBugaunKPz52sdOGJpEdNNMK2kcp-wXgjFpFZuoOmQ@mail.gmail.com>
Date:   Wed, 5 Aug 2020 11:28:31 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Cc:     Borislav Petkov <bp@...en8.de>, "H. Peter Anvin" <hpa@...or.com>,
        Borislav Petkov <bp@...e.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Dave Hansen <dave.hansen@...el.com>,
        Tony Luck <tony.luck@...el.com>,
        Cathy Zhang <cathy.zhang@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Kyung Min Park <kyung.min.park@...el.com>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Ricardo Neri <ricardo.neri@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        linux-edac <linux-edac@...r.kernel.org>
Subject: Re: [PATCH v2] x86/cpu: Use SERIALIZE in sync_core() when available

On Wed, Aug 5, 2020 at 10:07 AM Ricardo Neri
<ricardo.neri-calderon@...ux.intel.com> wrote:
>
> On Wed, Aug 05, 2020 at 07:08:08AM +0200, Borislav Petkov wrote:
> > On Tue, Aug 04, 2020 at 09:58:25PM -0700, hpa@...or.com wrote:
> > > Because why use an alternative to jump over one instruction?
> > >
> > > I personally would prefer to have the IRET put out of line
> >
> > Can't yet - SERIALIZE CPUs are a minority at the moment.
> >
> > > and have the call/jmp replaced by SERIALIZE inline.
> >
> > Well, we could do:
> >
> >       alternative_io("... IRET bunch", __ASM_SERIALIZE, X86_FEATURE_SERIALIZE, ...);
> >
> > and avoid all kinds of jumping. Alternatives get padded so there
> > would be a couple of NOPs following when SERIALIZE gets patched in
> > but it shouldn't be a problem. I guess one needs to look at what gcc
> > generates...
>
> But the IRET-TO-SELF code has instruction which modify the stack. This
> would violate stack invariance in alternatives as enforced in commit
> 7117f16bf460 ("objtool: Fix ORC vs alternatives"). As a result, objtool
> gives warnings as follows:
>
> arch/x86/kernel/alternative.o: warning: objtool: do_sync_core()+0xe:
> alternative modifies stack
>
> Perhaps in this specific case it does not matter as the changes in the
> stack will be undone by IRET. However, using alternative_io would require
> adding the macro STACK_FRAME_NON_STANDARD to functions using sync_core().
> IMHO, it wouldn't look good.
>
> So maybe the best approach is to implement as you suggested using
> static_cpu_has()?

I agree.  Let's keep it simple.

Honestly, I think the right solution is to have iret_to_self() in
actual asm and invoke it from C as needed.  IRET is *slow* -- trying
to optimize it at all is silly.  The big optimization was switching
from CPUID to IRET, since CPUID is slooooooooooooooooooow in virtual
environments, whereas IRET is merely sloooooooow and SERIALIZE is
probably just sloooow.

(I once benchmarked it.  IIRC the winning version on my laptop is MOV
to CR2 on bare metal and IRET in a Xen PV guest.  This optimization
was not obviously worthwhile.)

>
> Thanks and BR,
> Ricardo