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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210926150838.197719-2-jiangshanlai@gmail.com>
Date:   Sun, 26 Sep 2021 23:07:58 +0800
From:   Lai Jiangshan <jiangshanlai@...il.com>
To:     linux-kernel@...r.kernel.org
Cc:     Lai Jiangshan <laijs@...ux.alibaba.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        "Chang S . Bae" <chang.seok.bae@...el.com>,
        Sasha Levin <sashal@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>
Subject: [PATCH V2 01/41] x86/entry: Fix swapgs fence

From: Lai Jiangshan <laijs@...ux.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@...hat.com>
Cc: Chang S. Bae <chang.seok.bae@...el.com>
Cc: Sasha Levin <sashal@...nel.org>
Signed-off-by: Lai Jiangshan <laijs@...ux.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