linux-kernel - [PATCH] x86/apic: Fix the issues in x2apic

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20240812100819.870513-1-yuntao.wang@linux.dev>
Date: Mon, 12 Aug 2024 18:08:19 +0800
From: Yuntao Wang <yuntao.wang@...ux.dev>
To: linux-kernel@...r.kernel.org,
	x86@...nel.org
Cc: Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Peter Zijlstra (Intel)" <peterz@...radead.org>,
	Thorsten Blum <thorsten.blum@...lux.com>,
	Tony Luck <tony.luck@...el.com>,
	Daniel Sneddon <daniel.sneddon@...ux.intel.com>,
	Yuntao Wang <yuntao.wang@...ux.dev>
Subject: [PATCH] x86/apic: Fix the issues in x2apic_disable()

There are two issues in x2apic_disable().

The first issue is that the 'pr_warn' in 'if (x2apic_hw_locked())' will
never be executed, because when x2apic_hw_locked() evaluates to true,
x2apic_state should be X2APIC_ON_LOCKED. However, the current logic in
x2apic_disable() is that if x2apic_state is not X2APIC_ON, it returns
early, so the subsequent logic will not be executed.

Therefore, 'if (state != X2APIC_ON)' should be changed to
'if (state < X2APIC_ON)', so that when x2apic_state is X2APIC_ON_LOCKED,
the corresponding warning log can be printed.

The second issue is that the current logic of x2apic_disable() first sets
x2apic_mode and x2apic_state to 0 and X2APIC_DISABLED, respectively, and
then tries to disable x2APIC. However, when the APIC is locked in x2APIC
mode, that is, x2apic_state is X2APIC_ON_LOCKED, x2APIC cannot be disabled,
which causes the final values of the x2apic_mode/x2apic_state variables to
be inconsistent with the actual state of x2APIC.

Let's fix these issues.

Signed-off-by: Yuntao Wang <yuntao.wang@...ux.dev>
---
 arch/x86/kernel/apic/apic.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 66fd4b2a37a3..fce8d0214069 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1775,13 +1775,10 @@ static __init void apic_set_fixmap(bool read_apic);

 static __init void x2apic_disable(void)
 {
-	u32 x2apic_id, state = x2apic_state;
+	u32 x2apic_id;

-	x2apic_mode = 0;
-	x2apic_state = X2APIC_DISABLED;
-
-	if (state != X2APIC_ON)
-		return;
+	if (x2apic_state < X2APIC_ON)
+		goto out;

 	x2apic_id = read_apic_id();
 	if (x2apic_id >= 255)
@@ -1799,6 +1796,10 @@ static __init void x2apic_disable(void)
 	 * which fails to do the read after x2APIC was disabled.
 	 */
 	apic_set_fixmap(false);
+
+out:
+	x2apic_mode = 0;
+	x2apic_state = X2APIC_DISABLED;
 }

 static __init void x2apic_enable(void)
-- 
2.46.0