Re: [PATCH v3] x86/nSVM: Check injected event consistency

[Teddy Astie <teddy.astie@xxxxxxxxxx>]

Le 06/08/2026 à 19:26, Abdelkareem Abdelsaamad a écrit :
> On the AMD platforms, allowing a VMRUN instruction with a malformed VMCB has
> debugging complications, security and performance implications. The APM
> volume #2 15.20 (40332-Rev. 4.10-July 2026) states the two possibilities that
> result in a VMRUN exit with VMEXIT_INVALID due to the injected event. These are
> • Reserved values of TYPE have been specified.
> • TYPE = 3 (exception) has been specified with a vector that does not
>    correspond to an exception (this includes vector 2, which is an NMI, not
>    an exception).
>
> Extend the VMCB validation to check for such inconsistency.
>
> The collection of the invalid exception vectors are ported from the upstream
> KVM commit
> ("7e79f71bca5c" KVM: nSVM: Add missing consistency check for EVENTINJ). Adjust
> the checks from the commit to align with the APM Volume #2 and Volume #3
> (40332—Rev. 4.40—July 2026) for the X86_EXC_OF and X86_EXC_BR vectors which
> should not be valid on the x86 64-bit (long mode) platforms. The adjustment is
> posted to the KVM mailing commit patch thread
> https://lore.kernel.org/all/20260803225402.2324595-1-abdelkareem.abdelsaamad@xxxxxxxxxx/
>
> Signed-off-by: Abdelkareem Abdelsaamad <abdelkareem.abdelsaamad@xxxxxxxxxx>
> ---
> Changes in v3:
> - Restricted X86_EXC_OF (4) and X86_EXC_BR (5) vector injections to
>    non-64-bit guests to prevent impossible guest-mode state injections
>    per AMD APM Volumes 2 & 3.
> - Refactored exception vector validation from if-conditions to a switch
>    statement to improve readability and extensibility.
> - Restricted X86_EXC_CP (21) vector injection to hosts with enabled CET
>    to prevent VMRUN failures on hardware without CET support.
>
> Changes in v2:
> - Remove the redundant SVM_EVENT_INJ_TYPE_MASK and SVM_EVENT_INJ_VEC_MASK
>    constants.
> - Correct the Injected Event Type consistency check to disallow the injection
>    of reserved type 1 events.
> ---
(...)

> https://gitlab.com/xen-project/people/aabdelsa/xen/-/pipelines/2734283788
> ---
>   xen/arch/x86/hvm/svm/vmcb.c | 51 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
I would add this newly introduced function in svm_vmexit_handler(), when 
encountering VMEXIT_INVALID to attempt giving more information of the 
problem (nobody likes to debug VMEXIT_INVALID).
> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
> index 975a1eaef8..4379bbef09 100644
> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -320,6 +320,41 @@ void svm_vmcb_dump(const char *from, const struct 
> vmcb_struct *vmcb)
>       svm_dump_sel("  TR", &vmcb->tr);
>   }
>   
> +static bool is_valid_svm_vmcb_injected_exception_vector(
> +    const struct vmcb_struct *vmcb, uint8_t vmcb_injected_vector)
> +{
> +    switch ( vmcb_injected_vector )
> +    {
> +    case X86_EXC_DE:
> +    case X86_EXC_DB:
> +    case X86_EXC_BP:
> +    case X86_EXC_UD:
> +    case X86_EXC_NM:
> +    case X86_EXC_DF:
> +    case X86_EXC_TS:
> +    case X86_EXC_NP:
> +    case X86_EXC_SS:
> +    case X86_EXC_GP:
> +    case X86_EXC_PF:
> +    case X86_EXC_MF:
> +    case X86_EXC_AC:
> +    case X86_EXC_MC:
> +    case X86_EXC_XM:
> +    case X86_EXC_HV:
As you plan to drop #HV (due to being SEV-SNP specific), could it be at 
least commented out; which would hint the need for a appropriate check 
when implementing SEV-SNP restricted injections.
> +    case X86_EXC_SX:
> +        return true;
> +    case X86_EXC_OF:
> +    case X86_EXC_BR:
> +        return !(vmcb_get_efer(vmcb) & EFER_LMA) || !(vmcb->cs.l);
> +    case X86_EXC_VC:
> +        return vmcb_get_sev_es(vmcb);
> +    case X86_EXC_CP:
> +        return !!(vmcb_get_cr4(vmcb) & X86_CR4_CET);
> +    default:
> +        return false;
> +    }
> +}
> +

Teddy

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature