Re: [PATCH v5 08/20] KVM: VMX: Support instruction information extension

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The new EXTENDED_INSTRUCTION_INFO VMCS field is omitted from nested VMX reflection (vmcs12), breaking APX instruction emulation for L1.
- [High] Reading EXTENDED_INSTRUCTION_INFO unconditionally when Enlightened VMCS (eVMCS) is enabled triggers a warning and returns 0, corrupting KVM's instruction emulation on Hyper-V.
- [High] Malicious guests can trigger a host WARN_ON_ONCE by executing APX instructions when KVM is compiled without CONFIG_KVM_APX.
--

--- Patch [8]: [PATCH v5 08/20] KVM: VMX: Support instruction information extension ---
commit 96831841dd3c831246d26ed4875bb970e11f9dce
Author: Chang S. Bae <chang.seok.bae@intel.com>

KVM: VMX: Support instruction information extension

This commit defines VMCS field offsets for extended instruction
information and supports retrieving 5-bit register indices for the APX
extension. It relies on the host CPUID APX enumeration for enabling this
behavior.

Link: https://lore.kernel.org/aakEsXJgO-3m2xca@google.com

> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 5645a84f4fbc9..2334f52071f9f 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -276,6 +276,8 @@ enum vmcs_field {
>  	PID_POINTER_TABLE_HIGH		= 0x00002043,
>  	GUEST_PHYSICAL_ADDRESS          = 0x00002400,
>  	GUEST_PHYSICAL_ADDRESS_HIGH     = 0x00002401,
> +	EXTENDED_INSTRUCTION_INFO       = 0x00002406,
> +	EXTENDED_INSTRUCTION_INFO_HIGH  = 0x00002407,

[Severity: High]
Does this break nested VMX instruction parsing?

It appears EXTENDED_INSTRUCTION_INFO is not mapped in
kvm_supported_vmcs12_field_offsets in arch/x86/kvm/vmx/vmcs12.c, nor is
it copied in prepare_vmcs12() in arch/x86/kvm/vmx/nested.c.

If L2 executes an APX-extended instruction that exits to L1, and L1 attempts
a VMREAD of EXTENDED_INSTRUCTION_INFO to parse the exit, will this fail with
VMXERR_UNSUPPORTED_VMCS_COMPONENT and cause L1 emulation failures?

>  	VMCS_LINK_POINTER               = 0x00002800,
>  	VMCS_LINK_POINTER_HIGH          = 0x00002801,
>  	GUEST_IA32_DEBUGCTL             = 0x00002802,

[ ... ]

> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index f3b0ec7ccb03a..2f53338f84f0b 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -323,9 +323,18 @@ static __always_inline unsigned long vmx_get_exit_qual(struct kvm_vcpu *vcpu)
>  	return vt->exit_qualification;
>  }
>  
> +/*
> + * The APX enumeration guarantees the presence of the extended fields.
> + * The host CPUID bit alone is sufficient to rely on it.
> + */
> +static inline bool vmx_instr_info_extended(void)
> +{
> +	return static_cpu_has(X86_FEATURE_APX);
> +}

[Severity: High]
Can a guest trigger a host-side WARN_ON_ONCE by executing an APX instruction
when KVM is compiled without CONFIG_KVM_APX?

If KVM is built with NR_VCPU_GENERAL_PURPOSE_REGS = 16, but the host CPU
supports APX, this check returns true. A guest could cause a VM-exit with
a 5-bit register index 16 or greater.

When KVM extracts this index from the instruction and passes it along, will it
hit this check in arch/x86/kvm/regs.h:kvm_register_read_raw():

    if (WARN_ON_ONCE((unsigned int)reg >= NR_VCPU_GENERAL_PURPOSE_REGS))
        return 0;

This could panic the host if panic_on_warn is enabled.

>  
>  static inline int vmx_get_exit_qual_reg(unsigned long exit_qualification)
>  {
> -	return (exit_qualification >> 8) & 0xf;
> +	return (exit_qualification >> 8) & (vmx_instr_info_extended() ? 0x1f : 0xf);
>  }

[ ... ]

> @@ -707,20 +716,22 @@ static inline bool vmx_guest_state_valid(struct kvm_vcpu *vcpu)
>  
>  void dump_vmcs(struct kvm_vcpu *vcpu);
>  
> -/* A placeholder to smoothen 64-bit extension */
>  static inline u64 vmx_get_instr_info(void)
>  {
> -	return vmcs_read32(VMX_INSTRUCTION_INFO);
> +	return vmx_instr_info_extended() ? vmcs_read64(EXTENDED_INSTRUCTION_INFO) :
> +					   vmcs_read32(VMX_INSTRUCTION_INFO);
>  }

[Severity: High]
Will this corrupt KVM's instruction emulation when Enlightened VMCS (eVMCS)
is enabled on Hyper-V?

When vmx_instr_info_extended() is true, this calls:

vmcs_read64(EXTENDED_INSTRUCTION_INFO)
  evmcs_read64()
    get_evmcs_offset()

Since EXTENDED_INSTRUCTION_INFO doesn't seem to be added to the EVMCS1_FIELD
mappings in arch/x86/kvm/vmx/hyperv_evmcs.c, get_evmcs_offset() will return
a negative offset.

Will this trigger a warning and cause the read to unconditionally return 0,
leading to incorrect instruction decoding and silent data corruption in the
guest?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720171949.498680-1-chang.seok.bae@intel.com?part=8