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

Binbin Wu
2025-09-22T05:39:55Z

On 9/20/2025 6:32 AM, 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@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>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>

Two nits below.

> ---
> arch/x86/kvm/emulate.c | 114 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 100 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 23929151a5b8..dc0249929cbf 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)
>
> @@ -660,6 +661,57 @@ static inline bool emul_is_noncanonical_address(u64 la,
> return !ctxt->ops->is_canonical_addr(ctxt, la, flags);
> }
>
> +static bool is_shstk_instruction(u64 flags)
> +{
> + return flags & ShadowStack;
> +}
> +
> +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

> + */
> + 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?

> + }
> +}
> +
> …