On Tue, 12 May 2026 at 14:30, Sean Christopherson <[email protected]> wrote: > > On Tue, May 12, 2026, Fuad Tabba wrote: > > On Mon, 11 May 2026 at 21:25, Sean Christopherson <[email protected]> wrote: > > > > I used kvm_vm_release() because it's the only public API that closes > > > > vm->fd to trigger kernel-side destruction. But the existing callers > > > > follow it with vm_recreate_with_one_vcpu(), so the "release + later > > > > kvm_vm_free()" path isn't exercised today. > > > > > > > > I see three ways to make this clean: > > > > a) This patch: kvm_vm_release() becomes idempotent for its three > > > > FDs, matching the kvm_stats_release() idiom it already invokes. > > > > b) Leave kvm_vm_release() as-is and add a dedicated helper, e.g. > > > > kvm_vm_destroy_kernel(), that closes vm->fd to trigger kernel > > > > destruction while leaving the kvm_vm struct intact for > > > > post-destruction inspection. kvm_vm_free() learns to handle the > > > > half-released state. > > > > c) Something else entirely, e.g., the test should manage vm->fd > > > > directly and not rely on library helpers for this pattern. > > > > > > d) Fully kill the VM; validate the semantics with an explict mmap(). > > > > > > The entire point of the test you are writing is to verfiy that a > > > guest_memfd VMA > > > doesn't somehow cause KVM to leak state. So, make that obvious instead > > > of abusing > > > APIs that kinda sorta do what you want, but not really. > > > > > > mem = kvm_mmap(region->mmap_size, PROT_READ | PROT_WRITE, > > > MAP_SHARED, > > > region->guest_memfd); > > > > > > ... > > > > > > kvm_vm_free(vm); > > > > > > TEST_ASSERT(is_zero(mem, ...)); > > > > The test isn't about guest_memfd. The pKVM support that just landed > > via Will's series [1] > > Landed where? Is pKVM actually going upstream with anonymous memory? I > thought > the inability to protect against page faults in the untrusted kernel was a > non-starter? > > > kvm_mmap() + kvm_vm_free() + is_zero() doesn't translate here. The only > > host view of the donated pages is the memslot mmap, and kvm_vm_free() > > munmaps it on the way out, so inspection has to happen between > > kernel-side destruction and userspace free. kvm_vm_release() is the > > only library primitive that does that today. > > > > What do you suggest? > > Manually allocate the memory and expose it to the guest via > vm_set_user_memory_region2() vm_set_user_memory_region(). >
That works for this test and is cleaner. I'll restructure: mmap the backing in the test, vm_set_user_memory_region2() it in, then kvm_vm_free() -> is_zero() through the test-owned VMA -> munmap(). That drops the motivation for the kvm_vm_release() fd-guard patch in this series; happy to drop it, or to keep it as a standalone "match the kvm_stats_release() idiom" cleanup, whichever you prefer. Thanks, /fuad
