linux-kernel - Re: [PATCH v15 04/13] x86/sev: Change TSC MSR behavior for Secure TSC enabled 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: <20241209155718.GBZ1cTXp2XsgtvUzHm@fat_crate.local>
Date: Mon, 9 Dec 2024 16:57:18 +0100
From: Borislav Petkov <bp@...en8.de>
To: Nikunj A Dadhania <nikunj@....com>
Cc: linux-kernel@...r.kernel.org, thomas.lendacky@....com, x86@...nel.org,
	kvm@...r.kernel.org, mingo@...hat.com, tglx@...utronix.de,
	dave.hansen@...ux.intel.com, pgonda@...gle.com, seanjc@...gle.com,
	pbonzini@...hat.com
Subject: Re: [PATCH v15 04/13] x86/sev: Change TSC MSR behavior for Secure
 TSC enabled guests

On Tue, Dec 03, 2024 at 02:30:36PM +0530, Nikunj A Dadhania wrote:
> Secure TSC enabled guests should not write to MSR_IA32_TSC(10H) register as
> the subsequent TSC value reads are undefined.

What does that mean exactly?

I'd prefer if we issued a WARN_ONCE() there on the write to catch any
offenders.

*NO ONE* should be writing the TSC MSR but that's a different story.

IOW, something like this ontop of yours?

---

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index c22cb2ea4b99..050170eb28e6 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -1443,9 +1443,15 @@ static enum es_result __vc_handle_msr_tsc(struct pt_regs *regs, bool write)
 {
 	u64 tsc;
 
-	if (write)
-		return ES_OK;
+	if (!(sev_status & MSR_AMD64_SNP_SECURE_TSC))
+		goto read_tsc;
+
+	if (write) {
+		WARN_ONCE(1, "TSC MSR writes are verboten!\n");
+		return ES_UNSUPPORTED;
+	}
 
+read_tsc:
 	tsc = rdtsc_ordered();
 	regs->ax = lower_32_bits(tsc);
 	regs->dx = upper_32_bits(tsc);
@@ -1462,11 +1468,14 @@ static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
 	/* Is it a WRMSR? */
 	write = ctxt->insn.opcode.bytes[1] == 0x30;
 
-	if (regs->cx == MSR_SVSM_CAA)
+	switch(regs->cx) {
+	case MSR_SVSM_CAA:
 		return __vc_handle_msr_caa(regs, write);
-
-	if (regs->cx == MSR_IA32_TSC && (sev_status & MSR_AMD64_SNP_SECURE_TSC))
+	case MSR_IA32_TSC:
 		return __vc_handle_msr_tsc(regs, write);
+	default:
+		break;
+	}
 
 	ghcb_set_rcx(ghcb, regs->cx);
 	if (write) {

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette