linux-kernel - Re: [PATCH v1 1/1] x86/fred: Fix the FRED RSP0 MSR out of sync with its per CPU cache

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1282A5DA-1D5F-49F6-B780-FC8520C20266@zytor.com>
Date: Tue, 07 Jan 2025 19:27:53 -0800
From: "H. Peter Anvin" <hpa@...or.com>
To: "Xin Li (Intel)" <xin@...or.com>, linux-kernel@...r.kernel.org
CC: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...ux.intel.com, x86@...nel.org, andrew.cooper3@...rix.com
Subject: Re: [PATCH v1 1/1] x86/fred: Fix the FRED RSP0 MSR out of sync with its per CPU cache

On January 7, 2025 6:36:41 PM PST, "Xin Li (Intel)" <xin@...or.com> wrote:
>The FRED RSP0 MSR (pointing to the top of the kernel stack for user
>level event delivery) and its per CPU cache should be kept in sync to
>avoid redundant writes in the exit to user space path, as a result,
>a write to the FRED RSP0 MSR is paired with a write to its per CPU
>cache as fred_update_rsp0() does.
>
>However as the FRED RSP0 MSR is set to 0 in cpu_init_fred_exceptions(),
>it gets out of sync with its per CPU cache during a CPU offline/online
>cycle, which causes #DF exceptions if no context switch happens after
>a CPU offline/online cycle and before exit to user space.
>
>Fix the bug through resynchronizing the FRED RSP0 MSR with its per CPU
>cache value in cpu_init_fred_exceptions().
>
>Fixes: fe85ee391966 ("x86/entry: Set FRED RSP0 on return to userspace instead of context switch")
>Signed-off-by: Xin Li (Intel) <xin@...or.com>
>Cc: stable@...r.kernel.org
>---
> arch/x86/kernel/fred.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
>diff --git a/arch/x86/kernel/fred.c b/arch/x86/kernel/fred.c
>index 8d32c3f48abc..9524ace96bfa 100644
>--- a/arch/x86/kernel/fred.c
>+++ b/arch/x86/kernel/fred.c
>@@ -50,7 +50,18 @@ void cpu_init_fred_exceptions(void)
> 	       FRED_CONFIG_ENTRYPOINT(asm_fred_entrypoint_user));
> 
> 	wrmsrl(MSR_IA32_FRED_STKLVLS, 0);
>-	wrmsrl(MSR_IA32_FRED_RSP0, 0);
>+
>+	/*
>+	 * Resynchronize the FRED RSP0 MSR with its per CPU cache value.
>+	 *
>+	 * Another option is to leave the FRED RSP0 MSR as-is, because the RESET
>+	 * state of FRED MSRs is zero and INIT does not change the value of the
>+	 * FRED MSRs in a CPU offline/online cycle.  But it doesn't seem safe to
>+	 * depend on the properties of INIT as that's way too many things that
>+	 * could cause bugs.
>+	 */
>+	wrmsrl(MSR_IA32_FRED_RSP0, __this_cpu_read(fred_rsp0));
>+
> 	wrmsrl(MSR_IA32_FRED_RSP1, 0);
> 	wrmsrl(MSR_IA32_FRED_RSP2, 0);
> 	wrmsrl(MSR_IA32_FRED_RSP3, 0);
>
>base-commit: cf6b067860f69fe01b08ebae9138fe0f89854398

Looks straightforward enough.

Reviewed-by: H. Peter Anvin (Intel) <hpa@...or.com>