linux-kernel - Re: [PATCH v14 15/22] KVM: x86: Don't emulate instructions guarded by CET

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: <aMQ4L8id7f1fK16J@google.com>
Date: Fri, 12 Sep 2025 08:11:43 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Xiaoyao Li <xiaoyao.li@...el.com>
Cc: Chao Gao <chao.gao@...el.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	acme@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com, hpa@...or.com, 
	john.allen@....com, mingo@...nel.org, mingo@...hat.com, 
	minipli@...ecurity.net, mlevitsk@...hat.com, namhyung@...nel.org, 
	pbonzini@...hat.com, prsampat@....com, rick.p.edgecombe@...el.com, 
	shuah@...nel.org, tglx@...utronix.de, weijiang.yang@...el.com, x86@...nel.org, 
	xin@...or.com
Subject: Re: [PATCH v14 15/22] KVM: x86: Don't emulate instructions guarded by CET

On Fri, Sep 12, 2025, Sean Christopherson wrote:
> On Fri, Sep 12, 2025, Xiaoyao Li wrote:
> > On 9/11/2025 6:42 PM, Chao Gao wrote:
> > > (and thus complex). The reason is that no one had a strong opinion on whether
> > > to do the CPL check or not. I asked the same question before [*], but I don't
> > > have a strong opinion on this either.
> > 
> > I'm OK with it.
> 
> I have a strong opinion.  :-)
> 
> KVM must NOT check CPL, because inter-privilege level transfers could trigger
> CET emulation and both levels.  E.g. a FAR CALL will be affected by both shadow
> stacks and IBT at the target privilege level.
> 
> So this need more than just a changelog blurb, it needs a comment.  The code
> can also be cleaned up and optimized.  Reading CR4 and two MSRs (via indirect
> calls, i.e. potential retpolines) is wasteful for the vast majority of instructions,
> and gathering "stop emulation" into a local variable when a positive test is fatal
> is pointless.
> 
> 	/*
> 	 * Reject emulation if KVM might need to emulate shadow stack updates
> 	 * and/or indirect branch tracking enforcement, which the emulator
> 	 * doesn't support.  Deliberately don't check CPL as inter-privilege
> 	 * level transfers can trigger emulation at both privilege levels, and
> 	 * the expectation is that the guest will not require emulation of any
> 	 * CET-affected instructions at any privilege level.
> 	 */
> 	if (opcode.flags & (ShadowStack | IndirBrnTrk) &&
> 	    ctxt->ops->get_cr(ctxt, 4) & X86_CR4_CET) {
> 		u64 u_cet, s_cet;
> 
> 		if (ctxt->ops->get_msr(ctxt, MSR_IA32_U_CET, &u_cet) ||
> 		    ctxt->ops->get_msr(ctxt, MSR_IA32_S_CET, &s_cet))
> 			return EMULATION_FAILED;
> 
> 		if ((u_cet | s_cet) & CET_SHSTK_EN && opcode.flags & ShadowStack)
> 			  return EMULATION_FAILED;
> 
> 		if ((u_cet | s_cet) & CET_ENDBR_EN && opcode.flags & IndirBrnTrk)
> 			  return EMULATION_FAILED;
> 	}

On second thought, I think it's worth doing the CPL checks.  Explaining why KVM
doesn't bother with checking privilege level is more work than just writing the
code.

	/*
	 * Reject emulation if KVM might need to emulate shadow stack updates
	 * and/or indirect branch tracking enforcement, which the emulator
	 * doesn't support.
	 */
	if (opcode.flags & (ShadowStack | IndirBrnTrk) &&
	    ctxt->ops->get_cr(ctxt, 4) & X86_CR4_CET) {
		u64 u_cet = 0, s_cet = 0;

		/*
		 * Check both User and Supervisor on far transfers as inter-
		 * privilege level transfers are impacted by CET at the target
		 * privilege levels, and that is not known at this time.  The
	 	 * the expectation is that the guest will not require emulation
		 * of any CET-affected instructions at any privilege level.
		 */
		if (!(opcode.flags & NearBranch)) {
			u_cet = s_cet = CET_SHSTK_EN | CET_ENDBR_EN;
		} else if (ctxt->ops->cpl(ctxt) == 3) {
			u_cet = CET_SHSTK_EN | CET_ENDBR_EN;
		} else {
			s_cet = CET_SHSTK_EN | CET_ENDBR_EN;
		}

		if ((u_cet && ctxt->ops->get_msr(ctxt, MSR_IA32_U_CET, &u_cet)) ||
		    (s_cet && ctxt->ops->get_msr(ctxt, MSR_IA32_S_CET, &s_cet)))
			return EMULATION_FAILED;

		if ((u_cet | s_cet) & CET_SHSTK_EN && opcode.flags & ShadowStack)
			  return EMULATION_FAILED;

		if ((u_cet | s_cet) & CET_ENDBR_EN && opcode.flags & IndirBrnTrk)
			  return EMULATION_FAILED;
	}

Side topic, has anyone actually tested that this works?  I.e. that attempts to
emulate CET-affected instructions result in emulation failure?  I'd love to have
a selftest for this (hint, hint), but presumably writing one is non-trivial due
to the need to get the selftest compiled with the necessary annotations, setup,
and whatnot.