On 01/09/2018 04:56 AM, Willy Tarreau wrote: > --- a/arch/x86/entry/calling.h > +++ b/arch/x86/entry/calling.h > @@ -214,6 +214,11 @@ > .macro SWITCH_TO_KERNEL_CR3 scratch_reg:req > ALTERNATIVE "jmp .Lend_\@", "", X86_FEATURE_PTI > mov %cr3, \scratch_reg > + > + /* if we're already on the kernel PGD, we don't switch */ > + testq $(PTI_SWITCH_PGTABLES_MASK), \scratch_reg > + jz .Lend_\@ > + > ADJUST_KERNEL_CR3 \scratch_reg
Willy, this is not specific to your patch, but it is one of the first *changes* to this code since Spectre, thus I'm bringing it up in this thread. The code already has some, but new conditional branches give me the willies. None of them take the form that can be used to exploit Spectre/Variant1 that I can see, but I do think we need to start talking about why this is not vulnerable and probably documenting it in the entry code. Spectre/V1 (conditional branches) * Data reads in entry/exit when you have the user CR3 must be in cpu_entry_area and readable to a Meltdown exploit, anyway. That implies that there is no data to be leaked in the address space at all at this point. * Conditional branches in the entry/exit code with a kernel CR3 value are the only concern. It is safe, however, if the data being checked is not user-controllable. Spectre/V2 (indirect branches) * Indirect Branches in the entry code are forbidden because of Spectre/V2. Retpolines or other mitigations (IBRS) must be used instead. Anybody disagree? In this case, the data being checked (CR3) is not user-controllable and there are no indirect branches. So this code is OK.
