[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Re: [PATCH v1] nSVM: Check injected event consistency



On 15.07.2026 16:15, Teddy Astie wrote:
>+             (vmcb_get_sev_es(vmcb) && vmcb_injected_vector == X86_EXC_VC) );

>I'm not sure that #VC is gated behind SEV-ES guest.

>Regardless, injecting manually #VC is weird (especially for SEV-ES guests 
>where it's strongly discouraged), but it could be somewhat abused to emulate 
>some of the SEV-ES semantics. So I would suggest to threat it like the others.
However, It should be a valid VMCB configuration (#VC vector without SEV-ES 
fields enabled) from the hardware perspective according to the APM stated 
conditions. I think it is better for the hypervisor software to sanitize 
against such configuration. The reason is, to my understanding, the #VC vector 
is architecturally designed exclusively for AMD SEV-ES and SEV-SNP guests. The 
handlers for #VC in the guest's IDT table without SEV-ES support are not 
necessarily required which can still result in further debugging and security 
complications.

>> +    uint8_t vmcb_injected_type = vmcb->event_inj.type & 
>> SVM_EVENT_INJ_TYPE_MASK;
>> +    uint8_t vmcb_injected_vector = vmcb->event_inj.vector & 
>> SVM_EVENT_INJ_VEC_MASK;

>What are these masks for ?

>The definition of vmcb->event_inj suggest that they are not required
> > uint8_t  vector;
> > uint8_t  type:3;
Ah, right. This seems to be a leftover from my first draft implementation.
It is redundant and not really needed. I will get rid of it in the next 
revision.

>>   
>>   #define PRINTF(fmt, args...) do { \
>>       if ( !verbose ) return true; \
>> @@ -392,6 +419,15 @@ bool svm_vmcb_isvalid(
>>           PRINTF("eventinj: MBZ bits are set (%#"PRIx64")\n",
>>                  vmcb->event_inj.raw);
>>   
>> +    if ( vmcb_injected_type > X86_ET_SW_INT )
>> +        PRINTF("eventinj: Invalid Injected Event Type: (%#"PRIx8")\n",
>> +               vmcb_injected_type);

>What about vmcb_injected_type == 1 case ?
That is a good catch. I assumed all the representations up to X86_ET_SW_INT (4) 
are valid. I will
fix in the next revision.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.