[PATCH 0/2] KVM: selftests: guest_memfd close() fix and hardening

[PATCH 0/2] KVM: selftests: guest_memfd close() fix and hardening

[Sean Christopherson]

Fix a bug where freeing a VM attempts to close an invalid guest_memfd instance
due to checking if a u32 value is non-negative.  Then harden closing of fds in
kvm_util by invalidating fds after they are closed, e.g. so that freeing an fd
multiple times will fail instead of silently closing the wrong file.

Sean Christopherson (2):
  KVM: selftests: Cast guest_memfd fd to a signed int when checking for
    >= 0
  KVM: selftests: Add and use kvm_free_fd() to harden against fd goofs

 .../selftests/kvm/include/kvm_syscalls.h      |  6 +++++
 tools/testing/selftests/kvm/lib/kvm_util.c    | 25 +++++++++----------
 2 files changed, 18 insertions(+), 13 deletions(-)


base-commit: 66939c1603bd5579e63278f9dc72cba5b79da9b5
-- 
2.54.0.794.g4f17f83d09-goog

[PATCH 1/2] KVM: selftests: Cast guest_memfd fd to a signed int when checking for >= 0

[Sean Christopherson]

When conditionally closing a memory region's guest_memfd file descriptor,
cast the field to a signed it so that negative values are correctly
detected.  Because selftests reuse "struct kvm_userspace_memory_region2"
instead of providing custom storage, they pick up the kernel uAPI's __u32
definition of the file descriptor, not the more common "int" definition,
e.g. that's used for userspace_mem_region.fd.

Fixes: bb2968ad6c33 ("KVM: selftests: Add support for creating private memslots")
Reported-by: Bibo Mao <maobibo@loongson.cn>
Closes: https://lore.kernel.org/all/20260508015013.4108345-1-maobibo@loongson.cn
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index e08967ef7b7b..4ad015c6c44f 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -817,7 +817,7 @@ static void __vm_mem_region_delete(struct kvm_vm *vm,
 		kvm_munmap(region->mmap_alias, region->mmap_size);
 		close(region->fd);
 	}
-	if (region->region.guest_memfd >= 0)
+	if ((int)region->region.guest_memfd >= 0)
 		close(region->region.guest_memfd);
 
 	free(region);
-- 
2.54.0.794.g4f17f83d09-goog

[PATCH 2/2] KVM: selftests: Add and use kvm_free_fd() to harden against fd goofs

[Sean Christopherson]

Add a kvm_free_fd() macro to close and invalidate a file descriptor, and
use it through the core infrastructure to harden against goofs where a
selftest attempts to reuse a closed file descriptor.

Cc: Bibo Mao <maobibo@loongson.cn>
Cc: Fuad Tabba <tabba@google.com>
Cc: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../selftests/kvm/include/kvm_syscalls.h      |  6 +++++
 tools/testing/selftests/kvm/lib/kvm_util.c    | 23 +++++++++----------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_syscalls.h b/tools/testing/selftests/kvm/include/kvm_syscalls.h
index 067a4c9cf452..6cb3bed29b81 100644
--- a/tools/testing/selftests/kvm/include/kvm_syscalls.h
+++ b/tools/testing/selftests/kvm/include/kvm_syscalls.h
@@ -89,4 +89,10 @@ __KVM_SYSCALL_DEFINE(fallocate, 4, int, fd, int, mode, loff_t, offset, loff_t, l
 __KVM_SYSCALL_DEFINE(ftruncate, 2, unsigned int, fd, off_t, length);
 __KVM_SYSCALL_DEFINE(madvise, 3, void *, addr, size_t, length, int, advice);
 
+#define kvm_free_fd(fd)		\
+do {				\
+	kvm_close(fd);		\
+	(fd) = -1;		\
+} while (0)
+
 #endif /* SELFTEST_KVM_SYSCALLS_H */
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 4ad015c6c44f..195f3fdae1e3 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -77,7 +77,8 @@ static ssize_t get_module_param(const char *module_name, const char *param,
 	int fd, r;
 
 	/* Verify KVM is loaded, to provide a more helpful SKIP message. */
-	close(open_kvm_dev_path_or_exit());
+	fd = open_kvm_dev_path_or_exit();
+	kvm_free_fd(fd);
 
 	r = snprintf(path, path_size, "/sys/module/%s/parameters/%s",
 		     module_name, param);
@@ -90,8 +91,7 @@ static ssize_t get_module_param(const char *module_name, const char *param,
 	TEST_ASSERT(bytes_read > 0, "read(%s) returned %ld, wanted %ld bytes",
 		    path, bytes_read, buffer_size);
 
-	r = close(fd);
-	TEST_ASSERT(!r, "close(%s) failed", path);
+	kvm_free_fd(fd);
 	return bytes_read;
 }
 
@@ -160,7 +160,7 @@ unsigned int kvm_check_cap(long cap)
 	ret = __kvm_ioctl(kvm_fd, KVM_CHECK_EXTENSION, (void *)cap);
 	TEST_ASSERT(ret >= 0, KVM_IOCTL_ERROR(KVM_CHECK_EXTENSION, ret));
 
-	close(kvm_fd);
+	kvm_free_fd(kvm_fd);
 
 	return (unsigned int)ret;
 }
@@ -747,8 +747,7 @@ static void kvm_stats_release(struct kvm_binary_stats *stats)
 		stats->desc = NULL;
 	}
 
-	kvm_close(stats->fd);
-	stats->fd = -1;
+	kvm_free_fd(stats->fd);
 }
 
 __weak void vcpu_arch_free(struct kvm_vcpu *vcpu)
@@ -777,7 +776,7 @@ static void vm_vcpu_rm(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
 
 	kvm_munmap(vcpu->run, vcpu_mmap_sz());
 
-	kvm_close(vcpu->fd);
+	kvm_free_fd(vcpu->fd);
 	kvm_stats_release(&vcpu->stats);
 
 	list_del(&vcpu->list);
@@ -793,8 +792,8 @@ 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);
+	kvm_free_fd(vmp->fd);
+	kvm_free_fd(vmp->kvm_fd);
 
 	/* Free cached stats metadata and close FD */
 	kvm_stats_release(&vmp->stats);
@@ -815,10 +814,10 @@ static void __vm_mem_region_delete(struct kvm_vm *vm,
 	if (region->fd >= 0) {
 		/* There's an extra map when using shared memory. */
 		kvm_munmap(region->mmap_alias, region->mmap_size);
-		close(region->fd);
+		kvm_free_fd(region->fd);
 	}
 	if ((int)region->region.guest_memfd >= 0)
-		close(region->region.guest_memfd);
+		kvm_free_fd(region->region.guest_memfd);
 
 	free(region);
 }
@@ -1311,7 +1310,7 @@ static size_t vcpu_mmap_sz(void)
 	TEST_ASSERT(ret >= 0 && ret >= sizeof(struct kvm_run),
 		    KVM_IOCTL_ERROR(KVM_GET_VCPU_MMAP_SIZE, ret));
 
-	close(dev_fd);
+	kvm_free_fd(dev_fd);
 
 	return ret;
 }
-- 
2.54.0.794.g4f17f83d09-goog

Re: [PATCH 1/2] KVM: selftests: Cast guest_memfd fd to a signed int when checking for >= 0

[Ackerley Tng]

Sean Christopherson <seanjc@google.com> writes:

> When conditionally closing a memory region's guest_memfd file descriptor,
> cast the field to a signed it so that negative values are correctly
> detected.  Because selftests reuse "struct kvm_userspace_memory_region2"
> instead of providing custom storage, they pick up the kernel uAPI's __u32
> definition of the file descriptor, not the more common "int" definition,
> e.g. that's used for userspace_mem_region.fd.
>

I wonder if the better way to handle this would be to have a clearer
userspace_mem_region vs memslot separation, where only the final
function actually making the SET_MEMORY_ATTRIBUTES ioctl builds the
struct and passes it to the ioctl. userspace_mem_region should store
userspace-facing or userspace-specific parameters.

Anyway this is the faster fix :)

Reviewed-by: Ackerley Tng <ackerleytng@google.com>

> Fixes: bb2968ad6c33 ("KVM: selftests: Add support for creating private memslots")
> Reported-by: Bibo Mao <maobibo@loongson.cn>
> Closes: https://lore.kernel.org/all/20260508015013.4108345-1-maobibo@loongson.cn
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index e08967ef7b7b..4ad015c6c44f 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -817,7 +817,7 @@ static void __vm_mem_region_delete(struct kvm_vm *vm,
>  		kvm_munmap(region->mmap_alias, region->mmap_size);
>  		close(region->fd);
>  	}
> -	if (region->region.guest_memfd >= 0)
> +	if ((int)region->region.guest_memfd >= 0)
>  		close(region->region.guest_memfd);
>
>  	free(region);
> --
> 2.54.0.794.g4f17f83d09-goog

Re: [PATCH 2/2] KVM: selftests: Add and use kvm_free_fd() to harden against fd goofs

[Ackerley Tng]

Sean Christopherson <seanjc@google.com> writes:

> Add a kvm_free_fd() macro to close and invalidate a file descriptor, and
> use it through the core infrastructure to harden against goofs where a
> selftest attempts to reuse a closed file descriptor.
>
>
> [...snip...]
>

Thanks!

Reviewed-by: Ackerley Tng <ackerleytng@google.com>

Re: [PATCH 1/2] KVM: selftests: Cast guest_memfd fd to a signed int when checking for >= 0

[Bibo Mao]

On 2026/5/23 上午1:15, Sean Christopherson wrote:
> When conditionally closing a memory region's guest_memfd file descriptor,
> cast the field to a signed it so that negative values are correctly
> detected.  Because selftests reuse "struct kvm_userspace_memory_region2"
> instead of providing custom storage, they pick up the kernel uAPI's __u32
> definition of the file descriptor, not the more common "int" definition,
> e.g. that's used for userspace_mem_region.fd.
> 
> Fixes: bb2968ad6c33 ("KVM: selftests: Add support for creating private memslots")
> Reported-by: Bibo Mao <maobibo@loongson.cn>
> Closes: https://lore.kernel.org/all/20260508015013.4108345-1-maobibo@loongson.cn
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   tools/testing/selftests/kvm/lib/kvm_util.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index e08967ef7b7b..4ad015c6c44f 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -817,7 +817,7 @@ static void __vm_mem_region_delete(struct kvm_vm *vm,
>   		kvm_munmap(region->mmap_alias, region->mmap_size);
>   		close(region->fd);
>   	}
> -	if (region->region.guest_memfd >= 0)
> +	if ((int)region->region.guest_memfd >= 0)
>   		close(region->region.guest_memfd);
>   
>   	free(region);
> 

Reviewed-by: Bibo Mao <maobibo@loongson.cn>

Re: [PATCH 0/2] KVM: selftests: guest_memfd close() fix and hardening

[Sean Christopherson]

On Fri, 22 May 2026 10:15:33 -0700, Sean Christopherson wrote:
> Fix a bug where freeing a VM attempts to close an invalid guest_memfd instance
> due to checking if a u32 value is non-negative.  Then harden closing of fds in
> kvm_util by invalidating fds after they are closed, e.g. so that freeing an fd
> multiple times will fail instead of silently closing the wrong file.
> 
> Sean Christopherson (2):
>   KVM: selftests: Cast guest_memfd fd to a signed int when checking for
>     >= 0
>   KVM: selftests: Add and use kvm_free_fd() to harden against fd goofs
> 
> [...]

Applied to kvm-x86 selftests, thanks!

[1/2] KVM: selftests: Cast guest_memfd fd to a signed int when checking for >= 0
      https://github.com/kvm-x86/linux/commit/849d65e27bd6
[2/2] KVM: selftests: Add and use kvm_free_fd() to harden against fd goofs
      https://github.com/kvm-x86/linux/commit/3e8a0b991223

--
https://github.com/kvm-x86/linux/tree/next