linux-kernel - Re: [PATCH] x86: Disable kexec for TDX guests

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: <20230327105518.zuswpklvyvz72u6i@box.shutemov.name>
Date:   Mon, 27 Mar 2023 13:55:18 +0300
From:   "Kirill A. Shutemov" <kirill@...temov.name>
To:     Dave Hansen <dave.hansen@...el.com>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Borislav Petkov <bp@...en8.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
        Eric Biederman <ebiederm@...ssion.com>,
        kexec@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Andrew Cooper <andrew.cooper3@...rix.com>
Subject: Re: [PATCH] x86: Disable kexec for TDX guests

On Sun, Mar 26, 2023 at 10:01:23AM -0700, Dave Hansen wrote:
> > I guess we can go this path if you are fine with taking CR4.MCE and shared
> > memory reverting patches (they require some rework, but I can get them
> > into shape quickly). After that we can forbid kexec on machines with MADT
> > if nr_cpus > 1.
> 
> This goes back to what I asked before: is anyone actually going to *use*
> a single-processor system that wants to kexec()?  If not, let's not
> waste the time to introduce code that is just going to bitrot.  Just
> mark it broken and move on with life.
> 
> I'm also a _bit_ curious what the implications of the CR4.MCE
> preservation are.  IIRC, systems are quite a bit less stable when
> CR4.MCE==0. So, maybe there are some benefits to leaving it set during
> kexec() for everyone.

Hm. I thought the opposite: keeping MCE set brings more risks.

Andrew had feedback on the patch:

	Async events, including NMIs, cannot be taken between this point and the
	target having set itself up into it's intended operating mode.  During
	this period you get all kinds of fun with type confusion in the IDT/TSS
	and/or not having a safe stack to service the event.

I tend to agree with him, but maybe I miss bigger picture.

Based on that I adjusted the patch to only affect TDX guests:

>From edbef5f1e6c31929ae1249c58b29c38f86e676c0 Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Date: Fri, 10 Feb 2023 12:53:11 +0300
Subject: [PATCH] x86/kexec: Keep CR4.MCE during kexec for TDX guest

TDX guests are not allowed to clear CR4.MCE. Attempt to clear it leads
to #VE.

Use alternatives to keep the flag during kexec for TDX guests.

The change doesn't affect non-TDX environments.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
---
 arch/x86/kernel/relocate_kernel_64.S | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
index 4a73351f87f8..9e83a638a2b8 100644
--- a/arch/x86/kernel/relocate_kernel_64.S
+++ b/arch/x86/kernel/relocate_kernel_64.S
@@ -145,8 +145,11 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
 	 * Set cr4 to a known state:
 	 *  - physical address extension enabled
 	 *  - 5-level paging, if it was enabled before
+	 *  - Machine check exception on TDX guest. Clearing MCE is not allowed
+	 *    in TDX guests.
 	 */
-	movl	$X86_CR4_PAE, %eax
+	ALTERNATIVE "movl "$X86_CR4_PAE", %eax", \
+		    "movl "$(X86_CR4_PAE | X86_CR4_MCE)", %eax", X86_FEATURE_TDX_GUEST
 	testq	$X86_CR4_LA57, %r13
 	jz	1f
 	orl	$X86_CR4_LA57, %eax
-- 
  Kiryl Shutsemau / Kirill A. Shutemov