+Gautam and PPC lists/maintainers
On Fri, Oct 30, 2026, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [Medium] KVM: PPC: Book3S HV: rmap_nested memory leak in
> kvmhv_release_all_nested()
> --
>
> commit 9c25a1b4a76e8526eb8a2b11e1b9e63893b0de80
> Author: Sean Christopherson <[email protected]>
>
> KVM: Destroy memslots immediately after mmu_notifiers are unregistered
>
> This commit replaces active memslots with an empty memslot set and frees all
> memslots immediately after unregistering the MMU notifier in kvm_destroy_vm(),
> prior to calling kvm_arch_destroy_vm().
>
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index fa548b12228ff..2eda12a52bf4b 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
>
> [ ... ]
>
> > @@ -1310,12 +1329,10 @@ static void kvm_destroy_vm(struct kvm *kvm)
> > kvm->mn_active_invalidate_count = 0;
> > else
> > WARN_ON(kvm->mmu_invalidate_in_progress);
> > + kvm_destroy_memslots(kvm);
> > +
> > kvm_arch_destroy_vm(kvm);
> > kvm_destroy_devices(kvm);
> > - for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
> > - kvm_free_memslots(kvm, &kvm->__memslots[i][0]);
> > - kvm_free_memslots(kvm, &kvm->__memslots[i][1]);
> > - }
>
> [Severity: Medium]
> Does hoisting kvm_destroy_memslots() above kvm_arch_destroy_vm() leak
> struct rmap_nested allocations in slot->arch.rmap on PowerPC Book3S HV when
> destroying a VM with nested virtualization enabled?
I don't think so? Patch 3[*], "KVM: PPC: Flush/zap all memslots on
kvm_arch_flush_shadow_all()",
should free the rmaps in response to kvm_arch_flush_shadow_all(). From what I
can tell, the page tables have to be in radix mode, and so the rmaps are freed
via:
kvmppc_radix_flush_memslot()
|
-> kvmppc_unmap_pte()
|
-> kvmhv_remove_nest_rmap_range()
|
-> kvmhv_remove_nest_rmap_list()
And if that doesn't work, it means memslot deletion on an active VM is
completely
broken.
However, it means the memslot code in kvmhv_release_all_nested() is dead code,
so
this as follow-up to patch 3? Or maybe folded in?
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c
b/arch/powerpc/kvm/book3s_hv_nested.c
index a6ff42d7666c..27985210f972 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -802,8 +802,6 @@ void kvmhv_release_all_nested(struct kvm *kvm)
int lpid;
struct kvm_nested_guest *gp;
struct kvm_nested_guest *freelist = NULL;
- struct kvm_memory_slot *memslot;
- int srcu_idx, bkt;
spin_lock(&kvm->mmu_lock);
idr_for_each_entry(&kvm->arch.kvm_nested_guest_idr, gp, lpid) {
@@ -820,11 +818,6 @@ void kvmhv_release_all_nested(struct kvm *kvm)
freelist = gp->next;
kvmhv_release_nested(gp);
}
-
- srcu_idx = srcu_read_lock(&kvm->srcu);
- kvm_for_each_memslot(memslot, bkt, kvm_memslots(kvm))
- kvmhv_free_memslot_nest_rmap(memslot);
- srcu_read_unlock(&kvm->srcu, srcu_idx);
}
/* caller must hold gp->tlb_lock */
[*] https://lore.kernel.org/all/[email protected]
> In virt/kvm/kvm_main.c, kvm_destroy_memslots() replaces kvm->memslots[i]
> with kvm_empty_memslots and frees each memslot via
> kvm_free_memslot() -> kvm_arch_free_memslot() -> kvmppc_core_free_memslot_hv()
> in arch/powerpc/kvm/book3s_hv.c:
>
> static void kvmppc_core_free_memslot_hv(struct kvm_memory_slot *slot)
> {
> vfree(slot->arch.rmap);
> slot->arch.rmap = NULL;
> }
>
> which frees the slot->arch.rmap array without freeing the linked
> struct rmap_nested entries.
>
> Previously, those entries were freed before memslot destruction via
> kvm_arch_destroy_vm() -> kvmppc_core_destroy_vm_hv() ->
> kvmhv_release_all_nested() in arch/powerpc/kvm/book3s_hv_nested.c:
>
> srcu_idx = srcu_read_lock(&kvm->srcu);
> kvm_for_each_memslot(memslot, bkt, kvm_memslots(kvm))
> kvmhv_free_memslot_nest_rmap(memslot);
> srcu_read_unlock(&kvm->srcu, srcu_idx);
>
> With kvm_destroy_memslots() running before kvm_arch_destroy_vm(),
> kvm_memslots(kvm) now returns kvm_empty_memslots and the memslots have
> already been freed when kvmhv_release_all_nested() runs, so
> kvmhv_free_memslot_nest_rmap() is never called. Any struct rmap_nested
> entries remaining in slot->arch.rmap at teardown (for example, when
> KVMPPC_SECURE_INIT_DONE causes kvmppc_radix_flush_memslot() to return early,
> or when 4KB PTEs were replaced by a huge page in kvmppc_unmap_free_pte())
> will be leaked.
>
> --
> Sashiko AI review ยท
> https://sashiko.dev/#/patchset/[email protected]?part=12
>
