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

From: Lai Jiangshan
Date: Sun Sep 26 2021 - 11:08:48 EST


From: Lai Jiangshan <laijs@xxxxxxxxxxxxxxxxx>

Commit 18ec54fdd6d18 ("x86/speculation: Prepare entry code for Spectre
v1 swapgs mitigations") (commit1) added FENCE_SWAPGS_{KERNEL|USER}_ENTRY
for conditional swapgs. And in paranoid_entry(), it uses only
FENCE_SWAPGS_KERNEL_ENTRY for both conditions/branches. It is totally
correct because FENCE_SWAPGS_KERNEL_ENTRY implies FENCE_SWAPGS_USER_ENTRY
which can be seen in spectre_v1_select_mitigation() that if
X86_FEATURE_FENCE_SWAPGS_USER is set, X86_FEATURE_FENCE_SWAPGS_KERNEL
must be also set.

The commit1 also has a piece of comment saying why
FENCE_SWAPGS_KERNEL_ENTRY is needed since writing cr3 implies lfence:
writing cr3 is also conditionally.

But commit 96b2371413e8f ("x86/entry/64: Switch CR3 before SWAPGS in
paranoid entry") (commit2) switches the code order and at least this piece
of comments is useless because there is no "writing cr3" in between the
conditional swaps and the fence.

Even worse, the commit2 does not use FENCE_SWAPGS_{KERNEL|USER}_ENTRY
in the corresponding branches. It just uses FENCE_SWAPGS_KERNEL_ENTRY
in the user path and no FENCE_SWAPGS_KERNEL_ENTRY in the kernel path.
Possibly, it was because the commit1 which uses FENCE_SWAPGS_KERNEL_ENTRY
in both paths and shadowed the lfence requirement.

Fix it and remove the unneeded comment.

Fixes: Commit 96b2371413e8f ("x86/entry/64: Switch CR3 before SWAPGS in paranoid entry")
Cc: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
Cc: Chang S. Bae <chang.seok.bae@xxxxxxxxx>
Cc: Sasha Levin <sashal@xxxxxxxxxx>
Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxxxxx>
---
arch/x86/entry/entry_64.S | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index e38a4cf795d9..95d85b16710b 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -898,17 +898,12 @@ SYM_CODE_START_LOCAL(paranoid_entry)
rdmsr
testl %edx, %edx
jns .Lparanoid_entry_swapgs
+ FENCE_SWAPGS_KERNEL_ENTRY
ret

.Lparanoid_entry_swapgs:
swapgs
-
- /*
- * The above SAVE_AND_SWITCH_TO_KERNEL_CR3 macro doesn't do an
- * unconditional CR3 write, even in the PTI case. So do an lfence
- * to prevent GS speculation, regardless of whether PTI is enabled.
- */
- FENCE_SWAPGS_KERNEL_ENTRY
+ FENCE_SWAPGS_USER_ENTRY

/* EBX = 0 -> SWAPGS required on exit */
xorl %ebx, %ebx
--
2.19.1.6.gb485710b