On Wed, Jun 28, 2023 at 08:37:45AM -0700, Sean Christopherson wrote:
...
> So I think we should try this:
>
> ---
> arch/x86/kvm/mmu/mmu.c | 19 -------------------
> include/linux/kvm_host.h | 1 -
> virt/kvm/kvm_main.c | 13 ++-----------
> 3 files changed, 2 insertions(+), 31 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 60397a1beda3..e305737edf84 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3671,19 +3671,6 @@ void kvm_mmu_free_guest_mode_roots(struct kvm *kvm, struct kvm_mmu *mmu)
> }
> EXPORT_SYMBOL_GPL(kvm_mmu_free_guest_mode_roots);
>
> -
> -static int mmu_check_root(struct kvm_vcpu *vcpu, gfn_t root_gfn)
> -{
> - int ret = 0;
> -
> - if (!kvm_vcpu_is_visible_gfn(vcpu, root_gfn)) {
> - kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
> - ret = 1;
> - }
> -
> - return ret;
> -}
> -
> static hpa_t mmu_alloc_root(struct kvm_vcpu *vcpu, gfn_t gfn, int quadrant,
> u8 level)
> {
> @@ -3821,9 +3808,6 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu)
> root_pgd = kvm_mmu_get_guest_pgd(vcpu, mmu);
> root_gfn = root_pgd >> PAGE_SHIFT;
>
> - if (mmu_check_root(vcpu, root_gfn))
> - return 1;
> -
Hi Sean,
The checking of existence of memslot is still useful,
Otherwise NULL pointer dereference would be met as in below call trace.
mmu_alloc_shadow_roots
|->mmu_alloc_root
|->mmu_alloc_root(root_gfn)
|->kvm_mmu_get_shadow_page
|->__kvm_mmu_get_shadow_page
|->kvm_mmu_alloc_shadow_page
|->account_shadowed
|->slot = __gfn_to_memslot(slots, gfn); ==>NULL pointer
| kvm_slot_page_track_add_page(kvm, slot, gfn,KVM_PAGE_TRACK_WRITE);
|->update_gfn_track(slot, gfn, mode, 1);
|->index = gfn_to_index(gfn, slot->base_gfn, PG_LEVEL_4K); ==>NULL pointer dereference
Thanks
Yan
> /*
> * On SVM, reading PDPTRs might access guest memory, which might fault
> * and thus might sleep. Grab the PDPTRs before acquiring mmu_lock.
> @@ -3833,9 +3817,6 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu)
> pdptrs[i] = mmu->get_pdptr(vcpu, i);
> if (!(pdptrs[i] & PT_PRESENT_MASK))
> continue;
> -
> - if (mmu_check_root(vcpu, pdptrs[i] >> PAGE_SHIFT))
> - return 1;
> }
> }
>
