Re: [PATCH V2 01/41] x86/entry: Fix swapgs fence

[Thomas Gleixner]

Lai,

On Mon, Sep 27 2021 at 11:27, Lai Jiangshan wrote:
> On 2021/9/27 09:10, Lai Jiangshan wrote:
>
> The commit c75890700455 ("x86/entry/64: Remove unneeded kernel CR3 switching")
> ( https://lore.kernel.org/all/20200419144049.1906-2-laijs@xxxxxxxxxxxxxxxxx/ )
> also made it wrong.

Duh, did not spot that either.

> When the SWITCH_TO_KERNEL_CR3 in the path is removed, FENCE_SWAPGS_USER_ENTRY
> should also be changed to FENCE_SWAPGS_KERNEL_ENTRY. (Or just jmp to
> .Lerror_entry_done_lfence which has FENCE_SWAPGS_KERNEL_ENTRY already.)

Yes.

> And FENCE_SWAPGS_USER_ENTRY could be documented with "it should be followed with
> serializing operations such as SWITCH_TO_KERNEL_CR3".

It does not matter whether the serializing is before or after. The
problem is:

    if (from_user)
    	swapgs();

can take the wrong path speculatively which means the speculation is
then based on the wrong GS.

We have these sequences in the non paranoid entries:

    if (from_user) {
       pti_switch_cr3();
       swapgs();
    }

    if (from_user) {
       swapgs();
       pti_switch_cr3();
    }

and with mitigation these become:

    if (from_user) {
       pti_switch_cr3();
       swapgs();
       lfence_if_not_pti();
    } else {
       lfence();
    }

    if (from_user) {
       swapgs();
       lfence_if_not_pti();
       pti_switch_cr3();
    } else {
       lfence();
    }

When PTI is enabled then the CR3 write is sufficient because it's fully
serializing. If PTI is off the LFENCE is required. On which side the CR3
write is before or after SWAPGS does not matter. 

>  Or we can add a SWAPGS_AND_SWITCH_TO_KERNEL_CR3 to combine them.

No. We really don't want to go there.

Thanks,

        tglx