| Message ID | 20250109133817.314401-2-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: e500: map readonly host pages for read, and cleanup | expand |
On Thu, Jan 09, 2025, Paolo Bonzini wrote: > Avoid a NULL pointer dereference if the memslot table changes between the > exit and the call to kvmppc_e500_shadow_map(). > > Cc: stable@vger.kernel.org > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/powerpc/kvm/e500_mmu_host.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c > index e5a145b578a4..732335444d68 100644 > --- a/arch/powerpc/kvm/e500_mmu_host.c > +++ b/arch/powerpc/kvm/e500_mmu_host.c > @@ -349,6 +349,11 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500, > * pointer through from the first lookup. > */ > slot = gfn_to_memslot(vcpu_e500->vcpu.kvm, gfn); > + if (!slot) { > + ret = -EAGAIN; > + goto out; > + } This is unnecessary, __gfn_to_hva_many() checks for a NULL @slot. static unsigned long __gfn_to_hva_many(const struct kvm_memory_slot *slot, gfn_t gfn, gfn_t *nr_pages, bool write) { if (!slot || slot->flags & KVM_MEMSLOT_INVALID) return KVM_HVA_ERR_BAD; if (memslot_is_readonly(slot) && write) return KVM_HVA_ERR_RO_BAD; if (nr_pages) *nr_pages = slot->npages - (gfn - slot->base_gfn); return __gfn_to_hva_memslot(slot, gfn); } unsigned long gfn_to_hva_memslot(struct kvm_memory_slot *slot, gfn_t gfn) { return gfn_to_hva_many(slot, gfn, NULL); } Not checking the return value and doing a VMA lookup on hva=-1 when tlbsel==1 is gross, but it should be functionally safe. Returning -EAGAIN is nicer (kvmppc_e500_shadow_map() will inevitably return -EINVAL), but in practice it doesn't matter because all callers ultimately ignore the return value. Since there's a ratelimited printk that yells if there's no slot, it's probably best to let sleeping dogs lie. if (likely(!pfnmap)) { tsize_pages = 1UL << (tsize + 10 - PAGE_SHIFT); pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, NULL, &page); if (is_error_noslot_pfn(pfn)) { if (printk_ratelimit()) pr_err("%s: real page not found for gfn %lx\n", __func__, (long)gfn); return -EINVAL; } > + > hva = gfn_to_hva_memslot(slot, gfn); > > if (tlbsel == 1) { > -- > 2.47.1 >
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c index e5a145b578a4..732335444d68 100644 --- a/arch/powerpc/kvm/e500_mmu_host.c +++ b/arch/powerpc/kvm/e500_mmu_host.c @@ -349,6 +349,11 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500, * pointer through from the first lookup. */ slot = gfn_to_memslot(vcpu_e500->vcpu.kvm, gfn); + if (!slot) { + ret = -EAGAIN; + goto out; + } + hva = gfn_to_hva_memslot(slot, gfn); if (tlbsel == 1) {
Avoid a NULL pointer dereference if the memslot table changes between the exit and the call to kvmppc_e500_shadow_map(). Cc: stable@vger.kernel.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/powerpc/kvm/e500_mmu_host.c | 5 +++++ 1 file changed, 5 insertions(+)