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: <aNEkrlv1bdoRitoU@intel.com>
Date: Mon, 22 Sep 2025 18:27:58 +0800
From: Chao Gao <chao.gao@...el.com>
To: Sean Christopherson <seanjc@...gle.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>, Binbin Wu
	<binbin.wu@...ux.intel.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 Fri, Sep 19, 2025 at 03:32:25PM -0700, Sean Christopherson wrote:
>Don't emulate branch instructions, e.g. CALL/RET/JMP etc., that are
>affected by Shadow Stacks and/or Indirect Branch Tracking when said
>features are enabled in the guest, as fully emulating CET would require
>significant complexity for no practical benefit (KVM shouldn't need to
>emulate branch instructions on modern hosts).  Simply doing nothing isn't
>an option as that would allow a malicious entity to subvert CET
>protections via the emulator.
>
>To detect instructions that are subject to IBT or affect IBT state, use
>the existing IsBranch flag along with the source operand type to detect
>indirect branches, and the existing NearBranch flag to detect far branches
>(which can affect IBT state even if the branch itself is direct).
>
>For Shadow Stacks, explicitly track instructions that directly affect the
>current SSP, as KVM's emulator doesn't have existing flags that can be
>used to precisely detect such instructions.  Alternatively, the em_xxx()
>helpers could directly check for ShadowStack interactions, but using a
>dedicated flag is arguably easier to audit, and allows for handling both
>IBT and SHSTK in one fell swoop.
>
>Note!  On far transfers, do NOT consult the current privilege level and
>instead treat SHSTK/IBT as being enabled if they're enabled for User *or*
>Supervisor mode.  On inter-privilege level far transfers, SHSTK and IBT
>can be in play for the target privilege level, i.e. checking the current
>privilege could get a false negative, and KVM doesn't know the target
>privilege level until emulation gets under way.
>
>Note #2, FAR JMP from 64-bit mode to compatibility mode interacts with
>the current SSP, but only to ensure SSP[63:32] == 0.  Don't tag FAR JMP
>as SHSTK, which would be rather confusing and would result in FAR JMP
>being rejected unnecessarily the vast majority of the time (ignoring that
>it's unlikely to ever be emulated).  A future commit will add the #GP(0)
>check for the specific FAR JMP scenario.
>
>Note #3, task switches also modify SSP and so need to be rejected.  That
>too will be addressed in a future commit.
>
>Suggested-by: Chao Gao <chao.gao@...el.com>
>Originally-by: Yang Weijiang <weijiang.yang@...el.com>
>Cc: Mathias Krause <minipli@...ecurity.net>
>Cc: John Allen <john.allen@....com>
>Cc: Rick Edgecombe <rick.p.edgecombe@...el.com>
>Signed-off-by: Sean Christopherson <seanjc@...gle.com>

Reviewed-by: Chao Gao <chao.gao@...el.com>

<snip>
>+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.
>+	 */
>+	if (!(flags & NearBranch))
>+		return true;
>+
>+	switch (flags & (OpMask << SrcShift)) {

nit: maybe use SrcMask here.

#define SrcMask     (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.
>+	 */

RET FAR isn't affected by IBT, right? So it is a false positive in the above
NearBranch check. I am not asking you to fix it - just want to ensure it is
intended.

>+	case SrcImmU16:
>+		return false;
>+	default:
>+		WARN_ONCE(1, "Unexpected Src operand '%llx' on branch",
>+			  (flags & (OpMask << SrcShift)));

Ditto. use SrcMask here.