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 11/05/2017 15:56, Bandan Das wrote:
>> 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.
>
> If vmcs12->guest_pml_index is 512 it will write beyond the end without
> Dan's patch.

Oops, sorry! I misread, the assignment is taking place here as well. v1 is fine.

Thanks,
Bandan

>>>> 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 ?
>
> L0 writing 8 bytes in kernel memory outside the bounds of L1's memory.
>
> Paolo