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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The new EXTENDED_INSTRUCTION_INFO (0x2406) field is read unconditionally based on host CPU capability but is not added to nested VMX (vmcs12) or Enlightened VMCS (eVMCS) tracking structures, breaking instruction parsing for L1 hypervisors.
- [High] Guest-triggerable WARN_ON_ONCE Denial of Service via APX extended register parsing when KVM is compiled without CONFIG_KVM_APX.
--

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

KVM: VMX: Support instruction information extension
    
Define the VMCS field offset for the extended instruction information.
Then, support 5-bit register indices retrieval from VMCS fields.

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

> 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);
> +}
> +
>  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);
>  }

[Severity: High]
Does this expose the host to a guest-triggerable warning if KVM is
compiled without APX support?

If the host hardware supports APX but the KVM software configuration does
not, NR_VCPU_GENERAL_PURPOSE_REGS remains 16. A malicious guest could
execute a trapping APX instruction using an extended register, such as
r16.

This would cause vmx_get_exit_qual_reg to extract a 5-bit register index
greater than or equal to 16. When passed to kvm_register_read, it would
trigger the bounds check:

arch/x86/kvm/regs.h:kvm_register_read_raw() {
    if (WARN_ON_ONCE((unsigned int)reg >= NR_VCPU_GENERAL_PURPOSE_REGS))
        return 0;
    ...
}

This allows an unprivileged guest to spam the host kernel log or cause a
host panic if panic_on_warn is set. Should this check also verify KVM's
software capability for APX?

[ ... ]

> @@ -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]
Does reading EXTENDED_INSTRUCTION_INFO unconditionally on APX-capable hosts
break nested virtualization and enlightened VMCS?

When KVM runs a nested L1 hypervisor on an APX host, L0's prepare_vmcs12
only copies the legacy VMX_INSTRUCTION_INFO. The new
EXTENDED_INSTRUCTION_INFO is missing from vmcs12.

Additionally, if L1 attempts to read EXTENDED_INSTRUCTION_INFO via VMREAD,
L0 will fail the lookup in handle_vmread and inject a VM-fail because the
field is entirely missing from KVM's supported vmcs12 fields:

arch/x86/kvm/vmx/nested.c:handle_vmread() {
    offset = get_vmcs12_field_offset(field);
    if (offset < 0)
        return nested_vmx_fail(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
    ...
}

Similarly, if KVM itself runs as L1 on Hyper-V, reading this field
fails to find the eVMCS offset mapping in get_evmcs_offset, triggering a
WARN_ONCE and returning 0.

Do we need to add EXTENDED_INSTRUCTION_INFO to the nested VMX and eVMCS
tracking structures?

>  
>  static inline int vmx_get_instr_info_reg(u64 instr_info)
>  {
> -	return (instr_info >> 3) & 0xf;
> +	return vmx_instr_info_extended() ? (instr_info >> 16) & 0x1f :
> +					   (instr_info >> 3) & 0xf;
>  }

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