[PATCH 0/2] KVM: selftests: Fixes for guest_memfd_test and FD double-close

[PATCH 0/2] KVM: selftests: Fixes for guest_memfd_test and FD double-close

[Fuad Tabba]

Hi folks,

Two more patches, standalone KVM selftests fixes that I'd like to
land before posting a pKVM selftests series, a follow-up to Will's
pKVM infrastructure series [1].

The first patch fixes a build failure I ran into on Debian Bookworm
(glibc 2.36): guest_memfd_test.c uses MADV_COLLAPSE unconditionally,
but the constant was only exposed in glibc 2.37. The fix follows the
established selftest idiom of providing a compile-time fallback to
the kernel-ABI value when the libc header does not define it.

The second patch fixes an FD double-close in kvm_vm_release().
kvm_vm_free() calls kvm_vm_release() internally, so a test that
calls kvm_vm_release() and then kvm_vm_free() without reopening the
VM in between double-closes vmp->fd and vmp->kvm_fd. Existing
in-tree callers reopen via vm_recreate_with_one_vcpu() and do not
hit this today. The double-close becomes a hard test failure for
the pKVM selftests I will submit, which release the VM and then
inspect host-mapped memory before freeing.

Based on Linux 7.1-rc3.

Cheers,
/fuad

[1] https://lore.kernel.org/all/20260105154939.11041-1-will@kernel.org/

Fuad Tabba (2):
  KVM: selftests: Fix MADV_COLLAPSE build failure on older toolchains
  KVM: selftests: Fix FD double-close in kvm_vm_release()

 tools/testing/selftests/kvm/guest_memfd_test.c |  4 ++++
 tools/testing/selftests/kvm/lib/kvm_util.c     | 10 ++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.54.0.563.g4f69b47b94-goog

[PATCH 1/2] KVM: selftests: Fix MADV_COLLAPSE build failure on older toolchains

[Fuad Tabba]

guest_memfd_test.c uses MADV_COLLAPSE unconditionally, but the
constant was only exposed in glibc 2.37. Distros still on older glibc
(e.g. Debian Bookworm, glibc 2.36) cannot build the test.

Add a compile-time fallback that defines MADV_COLLAPSE to its
kernel-ABI value when the libc header does not, matching the
established selftest pattern for sometimes-missing libc constants
(see selftests/bpf/uprobe_multi.c, selftests/mm/hugetlb-soft-offline.c).

Fixes: 9830209b4ae8 ("KVM: selftests: Test MADV_COLLAPSE on guest_memfd")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 tools/testing/selftests/kvm/guest_memfd_test.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index d6528c6f5e03..45990ae5f1e5 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -23,6 +23,10 @@
 #include "test_util.h"
 #include "ucall_common.h"
 
+#ifndef MADV_COLLAPSE
+#define MADV_COLLAPSE	25
+#endif
+
 static size_t page_size;
 
 static void test_file_read_write(int fd, size_t total_size)
-- 
2.54.0.563.g4f69b47b94-goog

[PATCH 2/2] KVM: selftests: Fix FD double-close in kvm_vm_release()

[Fuad Tabba]

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.  Since
kvm_close() asserts on close() failure, the second close trips
TEST_ASSERT and aborts the test, or, if the FD was recycled, silently
closes an unrelated file.

Guard the two closes in kvm_vm_release() by checking each FD against
-1 and resetting it to -1 after closing, matching the existing
kvm_stats_release() idiom.  Existing in-tree callers all pass through
vm_recreate_with_one_vcpu() before teardown, so they reassign the FDs
and do not hit the bug today.

Fixes: fa3899add105 ("kvm: selftests: add basic test for state save and restore")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 2a76eca7029d..e44223714fd4 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -793,8 +793,14 @@ void kvm_vm_release(struct kvm_vm *vmp)
 	list_for_each_entry_safe(vcpu, tmp, &vmp->vcpus, list)
 		vm_vcpu_rm(vmp, vcpu);
 
-	kvm_close(vmp->fd);
-	kvm_close(vmp->kvm_fd);
+	if (vmp->fd >= 0) {
+		kvm_close(vmp->fd);
+		vmp->fd = -1;
+	}
+	if (vmp->kvm_fd >= 0) {
+		kvm_close(vmp->kvm_fd);
+		vmp->kvm_fd = -1;
+	}
 
 	/* Free cached stats metadata and close FD */
 	kvm_stats_release(&vmp->stats);
-- 
2.54.0.563.g4f69b47b94-goog

Re: [PATCH 2/2] KVM: selftests: Fix FD double-close in kvm_vm_release()

[Sean Christopherson]

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?

Re: [PATCH 1/2] KVM: selftests: Fix MADV_COLLAPSE build failure on older toolchains

[Sean Christopherson]

On Mon, May 11, 2026, Fuad Tabba wrote:
> guest_memfd_test.c uses MADV_COLLAPSE unconditionally, but the
> constant was only exposed in glibc 2.37. Distros still on older glibc
> (e.g. Debian Bookworm, glibc 2.36) cannot build the test.
> 
> Add a compile-time fallback that defines MADV_COLLAPSE to its
> kernel-ABI value when the libc header does not, matching the
> established selftest pattern for sometimes-missing libc constants
> (see selftests/bpf/uprobe_multi.c, selftests/mm/hugetlb-soft-offline.c).
> 
> Fixes: 9830209b4ae8 ("KVM: selftests: Test MADV_COLLAPSE on guest_memfd")
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  tools/testing/selftests/kvm/guest_memfd_test.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index d6528c6f5e03..45990ae5f1e5 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> @@ -23,6 +23,10 @@
>  #include "test_util.h"
>  #include "ucall_common.h"
>  
> +#ifndef MADV_COLLAPSE
> +#define MADV_COLLAPSE	25
> +#endif

Fixed by:

  https://lore.kernel.org/all/20260428012503.1213654-1-seanjc@google.com

I'll get that applied and on its way to Paolo this week.

Re: [PATCH 2/2] KVM: selftests: Fix FD double-close in kvm_vm_release()

[Fuad Tabba]

On Mon, 11 May 2026 at 15:58, Sean Christopherson <seanjc@google.com> 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.

I lean (a). (b) doesn't really avoid the change, as kvm_vm_free()
still calls kvm_vm_release() internally, so either kvm_vm_release()
gains these guards, or kvm_vm_free() grows new "already half-released"
state, or kvm_vm_destroy_kernel() open-codes the same guarded closes
on every FD it touches.  (a) is the minimal form, and it lines up
vmp->fd / vmp->kvm_fd with vmp->stats.fd, which kvm_vm_release()
already handles this way via kvm_stats_release().  But happy to take
(b) or (c) if you'd rather not broaden kvm_vm_release()'s contract.

Cheers,
/fuad

Re: [PATCH 2/2] KVM: selftests: Fix FD double-close in kvm_vm_release()

[Sean Christopherson]

On Mon, May 11, 2026, Fuad Tabba wrote:
> On Mon, 11 May 2026 at 15:58, Sean Christopherson <seanjc@google.com> 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, ...));