KVM: MMU: bail out pagewalk on kvm_read_guest error
Exit the guest pagetable walk loop if reading gpte failed. Otherwise its possible to enter an endless loop processing the previous present pte.
arch/x86/kvm/paging_tmpl.h +3 -1
@@ -150,7 +150,9 @@ walk: walker->table_gfn[walker->level -1 ] = table_gfn; walker->pte_gpa[walker->level -1 ] = pte_gpa; -kvm_read_guest (vcpu->kvm, pte_gpa, &pte,sizeof (pte)); +if (kvm_read_guest (vcpu->kvm, pte_gpa, &pte,sizeof (pte))) + goto not_present; +trace_kvm_mmu_paging_element (pte, walker->level);if (!is_present_gpte (pte))
Quoted 11 lines · Marcelo Tosatti
Exit the guest pagetable walk loop if reading gpte failed. Otherwise its possible to enter an endless loop processing the previous present pte.
Signed-off-by Marcelo Tosatti<[email protected]>arch/x86/kvm/paging_tmpl.h +0
-0
arch/x86/kvm/paging_tmpl.h walk: · line 156
Quoted 8 lines
arch/x86/kvm/paging_tmpl.h +3
-1@@ -150,7 +150,9 @@ walk: walker->table_gfn[walker->level -1 ] = table_gfn; walker->pte_gpa[walker->level -1 ] = pte_gpa; -kvm_read_guest (vcpu->kvm, pte_gpa,&pte,sizeof (pte)); +if (kvm_read_guest (vcpu->kvm, pte_gpa,&pte,sizeof (pte))) + goto not_present; +
On real hardware, if you place a pte at non-existing memory, you aren't guaranteed to get the present bit clear, so why is this necessary?
We should be able to survive any garbage the pte previously contained.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 24 lines · Avi Kivity
Quoted 11 lines · Marcelo Tosatti
Exit the guest pagetable walk loop if reading gpte failed. Otherwise its possible to enter an endless loop processing the previous present pte.
Signed-off-by Marcelo Tosatti<[email protected]>arch/x86/kvm/paging_tmpl.h +0
-0arch/x86/kvm/paging_tmpl.h walk: · line 155
Quoted 7 lines
arch/x86/kvm/paging_tmpl.h +3
-1@@ -150,7 +150,9 @@ walk: walker->table_gfn[walker->level -1 ] = table_gfn; walker->pte_gpa[walker->level -1 ] = pte_gpa; -kvm_read_guest (vcpu->kvm, pte_gpa,&pte,sizeof (pte)); +if (kvm_read_guest (vcpu->kvm, pte_gpa,&pte,sizeof (pte))) + goto not_present; +On real hardware, if you place a pte at non-existing memory, you aren't guaranteed to get the present bit clear, so why is this necessary?
Quoted 1 line
We should be able to survive any garbage the pte previously contained.
The problem is the content of the previous pte is processed (which is valid), but the cmpxchg fails (see the loop), without level decreasing.
Quoted 27 lines · Marcelo Tosatti
Quoted 25 lines · Avi Kivity
Quoted 11 lines · Marcelo Tosatti
Exit the guest pagetable walk loop if reading gpte failed. Otherwise its possible to enter an endless loop processing the previous present pte.
Signed-off-by Marcelo Tosatti<[email protected]>arch/x86/kvm/paging_tmpl.h +0
-0arch/x86/kvm/paging_tmpl.h walk: · line 156
Quoted 8 lines
arch/x86/kvm/paging_tmpl.h +3
-1@@ -150,7 +150,9 @@ walk: walker->table_gfn[walker->level -1 ] = table_gfn; walker->pte_gpa[walker->level -1 ] = pte_gpa; -kvm_read_guest (vcpu->kvm, pte_gpa,&pte,sizeof (pte)); +if (kvm_read_guest (vcpu->kvm, pte_gpa,&pte,sizeof (pte))) + goto not_present; +On real hardware, if you place a pte at non-existing memory, you aren't guaranteed to get the present bit clear, so why is this necessary?
Quoted 6 lines
Quoted 2 lines
We should be able to survive any garbage the pte previously contained.
The problem is the content of the previous pte is processed (which is valid), but the cmpxchg fails (see the loop), without level decreasing.
Yes, you're right. Patch is fine, then (we could triple-fault instead of #PF, but I don't think it matters).
Signature · 1 line
error compiling committee.c: too many arguments to function