On Mon, May 11, 2026, Fuad Tabba wrote:
> On Mon, 11 May 2026 at 15:58, Sean Christopherson <[email protected]> wrote:
> >
> > On Mon, May 11, 2026, Fuad Tabba wrote:
> > > kvm_vm_release() closes vmp->fd and vmp->kvm_fd unconditionally, and
> > > kvm_vm_free() calls kvm_vm_release() at teardown. A test that calls
> > > kvm_vm_release() and then kvm_vm_free() without a
> > > vm_recreate_with_one_vcpu() in between double-closes both FDs.
> >
> > That's a test bug, no?
>
> Fair point. Let me describe the actual use case and then I'd like
> your guidance on which way to fix it.
>
> The pKVM selftests I'm preparing need to verify that protected guest
> memory is zeroed when the VM is destroyed before the host regains
> visibility of those pages. The test pattern is:
>
> vm_userspace_mem_region_add(vm, ..., GPA_BASE, 1, GPA_PAGES, 0);
> /* run protected guest, which writes/shares/unshares its memory */
> ...
> TEST_ASSERT(get_proc_locked_vm_size() > GPA_SIZE); /* still pinned */
>
> kvm_vm_release(vm); /* trigger destroy */
>
> /* host_mem is still mmap'd in the test process; pKVM should have
> * zeroed it before unpinning */
> TEST_ASSERT(is_zero(region->host_mem, GPA_PAGES * PAGE_SIZE));
>
> kvm_vm_free(vm); /* tidy up */
> TEST_ASSERT(get_proc_locked_vm_size() == 0);
>
> 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, ...));
