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

Xiaoyao Li
2025-09-23T14:13:01Z

On 9/23/2025 4:04 AM, Sean Christopherson wrote:
> From: Sean Christopherson<seanjc@google.com>
> Date: Fri, 19 Sep 2025 15:32:25 -0700
> Subject: [PATCH] KVM: x86: Don't emulate instructions affected by CET features
>
> 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 JMPs
> and CALLs, all of which are effectively indirect. Explicitly check for
> emulation of IRET, FAR RET (IMM), and SYSEXIT (the ret-like far branches)
> instead of adding another flag, e.g. IsRet, as it's unlikely the emulator
> will ever need to check for return-like instructions outside of this one
> specific flow. Use an allow-list instead of a deny-list because (a) it's
> a shorter list and (b) so that a missed entry gets a false positive, not a
> false negative (i.e. reject emulation instead of clobbering CET state).
>
> 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@intel.com>
> Originally-by: Yang Weijiang<weijiang.yang@intel.com>
> Cc: Mathias Krause<minipli@grsecurity.net>
> Cc: John Allen<john.allen@amd.com>
> Cc: Rick Edgecombe<rick.p.edgecombe@intel.com>
> Reviewed-by: Chao Gao<chao.gao@intel.com>
> Reviewed-by: Binbin Wu<binbin.wu@linux.intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

Two nits besides,
> Link:https://lore.kernel.org/r/20250919223258.1604852-19-seanjc@google.com
> Signed-off-by: Sean Christopherson<seanjc@google.com>
> ---
> arch/x86/kvm/emulate.c | 117 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 103 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 23929151a5b8..a7683dc18405 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -178,6 +178,7 @@
> #define IncSP ((u64)1 << 54) /* SP is incremented before ModRM calc */
> #define TwoMemOp ((u64)1 << 55) /* Instruction has two memory operand */
> #define IsBranch ((u64)1 << 56) /* Instruction is considered a branch. */
> +#define ShadowStack ((u64)1 << 57) /* Instruction affects Shadow Stacks. */
>
> #define DstXacc (DstAccLo | SrcAccHi | SrcWrite)
>
> @@ -4068,9 +4069,9 @@ static const struct opcode group4[] = {
> static const struct opcode group5[] = {
> F(DstMem | SrcNone | Lock, em_inc),
> F(DstMem | SrcNone | Lock, em_dec),
> - I(SrcMem | NearBranch | IsBranch, em_call_near_abs),
> - I(SrcMemFAddr | ImplicitOps | IsBranch, em_call_far),
> - I(SrcMem | NearBranch | IsBranch, em_jmp_abs),
> + I(SrcMem | NearBranch | IsBranch | ShadowStack, em_call_near_abs),
> + I(SrcMemFAddr | ImplicitOps | IsBranch | ShadowStack, em_call_far),
> + I(SrcMem | NearBranch | IsBranch, em_jmp_abs),

The change of this line is unexpected, since it only changes the
indentation of 'em_jmp_abs'
> static unsigned imm_size(struct x86_emulate_ctxt *ctxt)
> {
> unsigned size;
> @@ -4943,6 +4998,40 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len, int
>
> ctxt->execute = opcode.u.execute;
>
> + /*
> + * Reject emulation if KVM might need to emulate shadow stack updates
> + * and/or indirect branch tracking enforcement, which the emulator
> + * doesn't support.
> + */
> + if ((is_ibt_instruction(ctxt) || is_shstk_instruction(ctxt)) &&
> + 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 level, and that is not known at this time. The
> + * the expectation is that the guest will not require emulation

Dobule 'the'

> + * of any CET-affected instructions at any privilege level.
> + */
> + if (!(ctxt->d & 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 && is_shstk_instruction(ctxt))
> + return EMULATION_FAILED;
> +
> + if ((u_cet | s_cet) & CET_ENDBR_EN && is_ibt_instruction(ctxt))
> + return EMULATION_FAILED;
> + }
> +
> if (unlikely(emulation_type & EMULTYPE_TRAP_UD) &&
> likely(!(ctxt->d & EmulateOnUD)))
> return EMULATION_FAILED;
>
> base-commit: 88539a6a25bc7a7ed96952775152e0c3331fdcaf