linux-kernel - Re: [PATCH REBASED RESEND] x86/cpu: Move early cpu initialization into a separate translation unit

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]

Date:   Fri, 22 Jun 2018 18:50:56 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Peter Zijlstra <peterz@...radead.org>
cc:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH REBASED RESEND] x86/cpu: Move early cpu initialization
 into a separate translation unit

On Fri, 22 Jun 2018, Peter Zijlstra wrote:
> On Fri, Jun 22, 2018 at 06:16:05PM +0200, Thomas Gleixner wrote:
> > On Fri, 22 Jun 2018, Kirill A. Shutemov wrote:
> > > On Fri, Jun 22, 2018 at 03:35:18PM +0000, Thomas Gleixner wrote:
> 
> > > > How is that supposed to work correctly?
> > > > 
> > > > start_kernel()
> > > >   ....
> > > >   trap_init()
> > > >     cpu_init()
> > > > 
> > > >   ....
> > > >   check_bugs()
> > > >     alternative_instructions()
> > > > 
> > > > So the first invocation of cpu_init() on the boot CPU will then use
> > > > static_cpu_has() which is not yet initialized proper.
> > > 
> > > Ouch.
> > > 
> > > Is there a way to catch such improper static_cpu_has() users?
> > > Silent misbehaviour is risky.
> > 
> > Yes, it is. I don't think we have something in place right now, but we
> > should add it definitely. PeterZ ????
> 
> So static_cpu_has() _should_ work. That thing is mightily convoluted,
> but behold:
> 
> | static __always_inline __pure bool _static_cpu_has(u16 bit)
> | {
> |         asm_volatile_goto("1: jmp 6f\n"
> |                  "2:\n"
> |                  ".skip -(((5f-4f) - (2b-1b)) > 0) * "
> |                          "((5f-4f) - (2b-1b)),0x90\n"
> 
> <snip magic shite>
> 
> |                  ".section .altinstr_aux,\"ax\"\n"
> |                  "6:\n"
> |                  " testb %[bitnum],%[cap_byte]\n"
> |                  " jnz %l[t_yes]\n"
> |                  " jmp %l[t_no]\n"
> |                  ".previous\n"
> |                  : : [feature]  "i" (bit),
> |                      [always]   "i" (X86_FEATURE_ALWAYS),
> |                      [bitnum]   "i" (1 << (bit & 7)),
> |                      [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
> |                  : : t_yes, t_no);
> | t_yes:
> |         return true;
> | t_no:
> |         return false;
> | }
> 
> So by default that emits, before patching:
> 
> 	jmp 6f
> 	'however many single byte NOPs are needed'
> 
> 	.section.altinstr_aux
> 	6: testb %[bitnum],%[cap_byte]
> 	   jnz %l[t_yes]
> 	   jmp %l[t_no]
> 	.previous
> 
> Which is a dynamic test for the bit in the bitmask. Which always works,
> irrespective of the alternative patching.
> 
> The magic, which I cut out, will rewrite the "jmp 6f, nops" thing to
> "jmp %l[y_{yes,no}]" at the alternative patching and we'll loose the
> dynamic test, pinning the condition forever more.

Hrm. Memory seems have to tricked me. So yes, it should work then.

Though I still prefer the two liners fixup of the cpu_init() section
mismatch thingy for now over the whole code move. Especially since Borislav
and I have plans to rework that insanity completely once the speculative
distractions are subsiding.

Thanks,

	tglx