lkml.org 
[lkml]   [2021]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH V5 01/50] x86/entry: Add fence for kernel entry swapgs in paranoid_entry()
From


On 2021/11/18 23:54, Peter Zijlstra wrote:

>
> I'm confused, shouldn't the LFENCE be between SWAPGS and future uses of
> GS prefix?

I'm wrong a again.

I once thought "it should be followed with serializing operations such
as SWITCH_TO_KERNEL_CR3", and tglx corrected me:

https://lore.kernel.org/lkml/875yumbgox.ffs@tglx/

> It does not matter whether the *serializing* is before or after

And in my brain, it was incorrectly stored as:
It does not matter whether the *fence* is before or after.

I will update patch1 and the corresponding C code in later patches.

Patch 1 in V4 is correct, but not as good as Borislav Petkov pointed out
that it has duplicated FENCE_SWAPGS_KERNEL_ENTRY.

I will change it as

rdmsr
if (need_swapgs) {
swapgs
set ebx/return value
}
FENCE_SWAPGS_KERNEL_ENTRY

>
> In the old code, before 96b2371413e8f, we had:
>
> swapgs
> SAVE_AND_SWITCH_TO_KERNEL_CR3
> FENCE_SWAPGS_KERNEL_ENTRY
>
> // %gs user comes here..
>
> And the comment made sense, since if SAVE_AND_SWITCH_TO_KERNEL_CR3 would
> imply an unconditional CR3 write, the LFENCE would not be needed.
>
> Then along gomes 96b2371413e8f and changes the order to:
>
> SAVE_AND_SWITCH_TO_KERNEL_CR3
> swapgs
> FENCE_SWAPGS_KERNEL_ENTRY
> // %gs user comes here..
>
> But now the comment is crazy talk, because even if the CR3 write were
> unconditional, it'd be pointless, since it's not after SWAPGS, but we
> still have the LFENCE in the right place.

I think the comments also make sense.

If CR3 write were unconditional before swapgs, no fence is needed after
swapgs since cr3 write is serializing.

>
> But now you want to make it:
>
> SAVE_AND_SWITCH_TO_KERNEL_CR3
> FENCE_SWAPGS_KERNEL_ENTRY
> swapgs
> // %gs user comes here..
>
> And there's nothing left and speculation can use the old %gs for our
> user and things go sideways. Hmm?
>
>
> (on a completely unrelated note, I find KERNEL_ENTRY and USER_ENTRY
> utterly confusing)
>

\
 
 \ /
  Last update: 2021-11-18 18:28    [W:0.614 / U:2.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site