Messages in this thread Patch in this message | | | From | Lai Jiangshan <> | Subject | [PATCH V2 01/41] x86/entry: Fix swapgs fence | Date | Sun, 26 Sep 2021 23:07:58 +0800 |
| |
From: Lai Jiangshan <laijs@linux.alibaba.com>
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@redhat.com> Cc: Chang S. Bae <chang.seok.bae@intel.com> Cc: Sasha Levin <sashal@kernel.org> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com> --- 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
| |