linux-kernel - Re: [PATCH v16 18/51] KVM: x86: Don't emulate instructions affected by CET features

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: <aNF9vnXrRnKjC1DD@google.com>
Date: Mon, 22 Sep 2025 09:47:58 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Binbin Wu <binbin.wu@...ux.intel.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Tom Lendacky <thomas.lendacky@....com>, Mathias Krause <minipli@...ecurity.net>, 
	John Allen <john.allen@....com>, Rick Edgecombe <rick.p.edgecombe@...el.com>, 
	Chao Gao <chao.gao@...el.com>, Xiaoyao Li <xiaoyao.li@...el.com>, 
	Maxim Levitsky <mlevitsk@...hat.com>, Zhang Yi Z <yi.z.zhang@...ux.intel.com>, Xin Li <xin@...or.com>
Subject: Re: [PATCH v16 18/51] KVM: x86: Don't emulate instructions affected
 by CET features

On Mon, Sep 22, 2025, Binbin Wu wrote:
> > +static bool is_ibt_instruction(u64 flags)
> > +{
> > +	if (!(flags & IsBranch))
> > +		return false;
> > +
> > +	/*
> > +	 * Far transfers can affect IBT state even if the branch itself is
> > +	 * direct, e.g. when changing privilege levels and loading a conforming
> > +	 * code segment.  For simplicity, treat all far branches as affecting
> > +	 * IBT.  False positives are acceptable (emulating far branches on an
> > +	 * IBT-capable CPU won't happen in practice), while false negatives
> > +	 * could impact guest security.
> > +	 *
> > +	 * Note, this also handles SYCALL and SYSENTER.
> 
> SYCALL -> SYSCALL

Fixed.

> > +	 */
> > +	if (!(flags & NearBranch))
> > +		return true;
> > +
> > +	switch (flags & (OpMask << SrcShift)) {
> > +	case SrcReg:
> > +	case SrcMem:
> > +	case SrcMem16:
> > +	case SrcMem32:
> > +		return true;
> > +	case SrcMemFAddr:
> > +	case SrcImmFAddr:
> > +		/* Far branches should be handled above. */
> > +		WARN_ON_ONCE(1);
> > +		return true;
> > +	case SrcNone:
> > +	case SrcImm:
> > +	case SrcImmByte:
> > +	/*
> > +	 * Note, ImmU16 is used only for the stack adjustment operand on ENTER
> > +	 * and RET instructions.  ENTER isn't a branch and RET FAR is handled
> > +	 * by the NearBranch check above.  RET itself isn't an indirect branch.
> > +	 */
> > +	case SrcImmU16:
> > +		return false;
> > +	default:
> > +		WARN_ONCE(1, "Unexpected Src operand '%llx' on branch",
> > +			  (flags & (OpMask << SrcShift)));
> > +		return false;
> 
> Is it safer to reject the emulation if it has unexpected src operand?

Not really?  Maybe?  Honestly, we've failed miserably if this escapes initial
development and testing, to the point where I don't think there's a "good"
answer as to whether KVM should treat the instruction as affecting IBT.  I think
I'd prefer to let the guest limp along and hope for the best?