On Wed, Jan 10, 2018 at 12:29:17PM -0800, Dave Hansen wrote: > 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.
Just let me know what comment you'd like me to add there. I suppose something like "This code has code has been checked against Spectre/v1/v2 attacks ; it's safe as CR3 is not user-controllable and there's no indirect branches" ? Thanks, Willy
