Paolo Bonzini <pbonzini@xxxxxxxxxx> writes: > On 11/05/2017 15:56, Bandan Das wrote: >> Paolo Bonzini <pbonzini@xxxxxxxxxx> 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