Messages in this thread | | | Subject | Re: [PATCH 2/6] KVM: nSVM: call nested_svm_load_cr3 on nested state load | From | Paolo Bonzini <> | Date | Thu, 1 Apr 2021 16:31:35 +0200 |
| |
On 01/04/21 16:18, Maxim Levitsky wrote: > While KVM's MMU should be fully reset by loading of nested CR0/CR3/CR4 > by KVM_SET_SREGS, we are not in nested mode yet when we do it and therefore > only root_mmu is reset. > > On regular nested entries we call nested_svm_load_cr3 which both updates > the guest's CR3 in the MMU when it is needed, and it also initializes > the mmu again which makes it initialize the walk_mmu as well when nested > paging is enabled in both host and guest. > > Since we don't call nested_svm_load_cr3 on nested state load, > the walk_mmu can be left uninitialized, which can lead to a NULL pointer > dereference while accessing it if we happen to get a nested page fault > right after entering the nested guest first time after the migration and > we decide to emulate it, which leads to the emulator trying to access > walk_mmu->gva_to_gpa which is NULL. > > Therefore we should call this function on nested state load as well. > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/kvm/svm/nested.c | 40 +++++++++++++++++++++------------------ > 1 file changed, 22 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c > index 8523f60adb92..ac5e3e17bda4 100644 > --- a/arch/x86/kvm/svm/nested.c > +++ b/arch/x86/kvm/svm/nested.c > @@ -215,24 +215,6 @@ static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm) > return true; > } > > -static bool svm_get_nested_state_pages(struct kvm_vcpu *vcpu) > -{ > - struct vcpu_svm *svm = to_svm(vcpu); > - > - if (WARN_ON(!is_guest_mode(vcpu))) > - return true; > - > - if (!nested_svm_vmrun_msrpm(svm)) { > - vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; > - vcpu->run->internal.suberror = > - KVM_INTERNAL_ERROR_EMULATION; > - vcpu->run->internal.ndata = 0; > - return false; > - } > - > - return true; > -} > - > static bool nested_vmcb_check_controls(struct vmcb_control_area *control) > { > if (CC(!vmcb_is_intercept(control, INTERCEPT_VMRUN))) > @@ -1312,6 +1294,28 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu, > return ret; > } > > +static bool svm_get_nested_state_pages(struct kvm_vcpu *vcpu) > +{ > + struct vcpu_svm *svm = to_svm(vcpu); > + > + if (WARN_ON(!is_guest_mode(vcpu))) > + return true; > + > + if (nested_svm_load_cr3(&svm->vcpu, vcpu->arch.cr3, > + nested_npt_enabled(svm))) > + return false; > + > + if (!nested_svm_vmrun_msrpm(svm)) { > + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; > + vcpu->run->internal.suberror = > + KVM_INTERNAL_ERROR_EMULATION; > + vcpu->run->internal.ndata = 0; > + return false; > + } > + > + return true; > +} > + > struct kvm_x86_nested_ops svm_nested_ops = { > .check_events = svm_check_nested_events, > .triple_fault = nested_svm_triple_fault, >
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
| |