Messages in this thread |  | | Subject | Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature | From | David Woodhouse <> | Date | Sat, 27 Jan 2018 14:26:25 +0000 |
| |
On Sat, 2018-01-27 at 08:59 -0500, Konrad Rzeszutek Wilk wrote: > On Mon, Jan 08, 2018 at 05:14:37PM +0100, Peter Zijlstra wrote: > > > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > > > > > > > > > > a good suggestion, but we encountered some issues with it either > > > > crashing the kernel at boot or not properly turning on/off. > > The below boots, but I lack stuff to test the enabling. > ..snip.. > > > > --- a/arch/x86/entry/calling.h > > +++ b/arch/x86/entry/calling.h > > @@ -373,22 +373,17 @@ For 32-bit we have the following convent > > .endm > > > > .macro ENABLE_IBRS > > - testl $1, dynamic_ibrs > > - jz .Lskip_\@ > > + STATIC_JUMP_IF_FALSE .Lskip_\@, ibrs_key, def=0 > > > > PUSH_MSR_REGS > > WRMSR_ASM $MSR_IA32_SPEC_CTRL, $SPEC_CTRL_FEATURE_ENABLE_IBRS > > POP_MSR_REGS > > - jmp .Ldone_\@ > > > > .Lskip_\@: > > - lfence > > -.Ldone_\@: > > .endm > I know that this particular patchset is now obsolete as the retpoline > along with stuffing the RSB half or full is the preferred way. > > But I am wondering - why was the 'lfence' added in the first place > if dynamic_ibrs was zero?
This was in the system call path, just before the jmp *sys_call_table() indirect jump. If an attacker could cause that 'jz .Lskip_\@' over the wrmsr to be predicted *taken*, speculative execution would happily proceed all the way to the indirect jump. Oops :)
Hence the 'else lfence' so that the branch-taken code path still stops to wait. Hence also the subsequent insistence on using ALTERNATIVE for it, and my paranoia about relying on GCC not missing optimisations if we're using static_cpu_has(). [unhandled content-type:application/x-pkcs7-signature] |  |