Re: [PATCH v2] kvm: nVMX: off by one in vmx_write_pml_buffer()

[Bandan Das <bsd@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 10/05/2017 22:43, Dan Carpenter wrote:
>> There are PML_ENTITY_NUM elements in the pml_address[] array so the >
>> should be >= or we write beyond the end of the array when we do:
>> 
>> 	pml_address[vmcs12->guest_pml_index--] = gpa;

Actually, we can never write beyond the end when we do
pml_address[vmcs12->guest_pml_index--] = gpa (which happens in the
host hypervisor btw). I think this should be changed.

>> This causes a static checker warning but the runtime impact is minimal.
>> The ->guest_pml_index variable can only be set to PML_ENTITY_NUM by a
>> buggy hypervisor.
>
> The v1 commit message is better actually.  You can always replace
> "buggy" with "malicious".

I agree, they are interchangeable but what's the worst that can happen ?
L1 killing itself ?

Bandan

> It's a 8 byte write and bits 12-45 of the datum are controlled by the
> attacker.  It's pretty bad (and embarrassing - I'm not sure why I was
> super-sure that PML_ENTITY_NUM was 511 rather than 512).
>
> Paolo