[PATCH 0/5] KVM: guest_memfd: fix unbind race and NUMA selftests

[PATCH 0/5] KVM: guest_memfd: fix unbind race and NUMA selftests

[Shivank Garg]

Split out the guest_memfd folio migration series [1], where migration part
needs more discussion. These fixes, reported by Sashiko bot [2][3], are
independent of it and can be reviewed on their own.

[1] https://lore.kernel.org/all/20260805-shivank-gmem-migrate-v3-0-00d8bdec4e1d@amd.com 
[2] https://lore.kernel.org/all/20260728092027.225CF1F000E9@smtp.kernel.org
[3] https://lore.kernel.org/all/20260728091945.EA6C31F000E9@smtp.kernel.org

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
Shivank Garg (5):
      KVM: guest_memfd: take the invalidate lock when unbinding a dying file
      KVM: selftests: fix maxnode arguments in xapic_ipi_test
      KVM: selftests: use BITS_PER_TYPE() for NUMA masks
      KVM: selftests: add get_numa_mem_nodes()
      KVM: selftests: use allowed NUMA nodes in guest_memfd_test

 tools/testing/selftests/kvm/guest_memfd_test.c   | 86 ++++++++++++++++--------
 tools/testing/selftests/kvm/include/numaif.h     | 52 ++------------
 tools/testing/selftests/kvm/x86/xapic_ipi_test.c | 14 ++--
 virt/kvm/guest_memfd.c                           | 23 ++++---
 4 files changed, 84 insertions(+), 91 deletions(-)
---
base-commit: 2709dd5ae32f0828f386327c76bba9f39f63a1c6
change-id: 20260822-shivank-gmem-fix-split-b8a9057cf987

Best regards,
-- 
Shivank Garg <shivankg@amd.com>

[PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file

[Shivank Garg]

kvm_gmem_unbind() skips mapping->invalidate_lock when the guest_memfd
file is already dying. All other paths that modify f->bindings hold
that lock.

kvm_gmem_invalidate_{start,end}() checks f->bindings independently to
decide whether to begin or end KVM MMU invalidations. So, the bindings
must remain stable between the two calls. If a binding is removed in that
window, start increments mmu_invalidate_in_progress but end does not
decrement it. Example, unbind race with memory failure:

  CPU 0: memory failure               CPU 1: memslot delete
  ----------------------------------  ---------------------------
                                      (guest_memfd file is dying)
  kvm_gmem_error_folio()
    kvm_gmem_invalidate_start()
      finds binding
      mmu_invalidate_in_progress++
                                      kvm_gmem_unbind()
                                        get_file_active() fails
                                        store NULL in bindings
    kvm_gmem_invalidate_end()
      no binding found
      counter stays elevated

mmu_invalidate_retry() then returns 1 forever, so guest page faults
retry without ever installing a mapping and the guest hangs.

Take the invalidate lock in the dying-file path too. This prevents unbind
from removing a binding and leaking mmu_invalidate_in_progress. This is
safe because any caller that reaches this path holds slots_lock, so
kvm_gmem_release() cannot nullify the slot->gmem.file, until
kvm_gmem_unbind() finishes.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260728092027.225CF1F000E9@smtp.kernel.org
Fixes: ae431059e75d ("KVM: guest_memfd: Remove bindings on memslot deletion when gmem is dying")
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 virt/kvm/guest_memfd.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index f0e5da490866..f848120af84b 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -721,6 +721,8 @@ static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
 
 void kvm_gmem_unbind(struct kvm_memory_slot *slot)
 {
+	struct file *gmem_file;
+
 	/*
 	 * Nothing to do if the underlying file was _already_ closed, as
 	 * kvm_gmem_release() invalidates and nullifies all bindings.
@@ -733,21 +735,24 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
 	/*
 	 * However, if the file is _being_ closed, then the bindings need to be
 	 * removed as kvm_gmem_release() might not run until after the memslot
-	 * is freed.  Note, modifying the bindings is safe even though the file
-	 * is dying as kvm_gmem_release() nullifies slot->gmem.file under
+	 * is freed.  Note, dereferencing the dying file is safe as
+	 * kvm_gmem_release() nullifies slot->gmem.file under
 	 * slots_lock, and only puts its reference to KVM after destroying all
 	 * bindings.  I.e. reaching this point means kvm_gmem_release() hasn't
 	 * yet destroyed the bindings or freed the gmem_file, and can't do so
 	 * until the caller drops slots_lock.
 	 */
-	if (!file) {
-		__kvm_gmem_unbind(slot, slot->gmem.file->private_data);
-		return;
-	}
+	gmem_file = file ?: slot->gmem.file;
 
-	filemap_invalidate_lock(file->f_mapping);
-	__kvm_gmem_unbind(slot, file->private_data);
-	filemap_invalidate_unlock(file->f_mapping);
+	/*
+	 * Take the invalidate lock even for a dying file.  Otherwise,
+	 * kvm_gmem_invalidate_start() can find the binding and increment
+	 * mmu_invalidate_in_progress while kvm_gmem_invalidate_end() misses
+	 * the removed binding and skips decrement.
+	 */
+	filemap_invalidate_lock(gmem_file->f_mapping);
+	__kvm_gmem_unbind(slot, gmem_file->private_data);
+	filemap_invalidate_unlock(gmem_file->f_mapping);
 }
 
 /* Returns a locked folio on success.  */

-- 
2.43.0

[PATCH 2/5] KVM: selftests: fix maxnode arguments in xapic_ipi_test

[Shivank Garg]

migrate_pages() syscall expect maxnode to be one greater than the
number of bits in the nodemask. do_migrations() passes the size of
nodemask in bytes to migrate_pages(). This sets the maxnode to 8,
so kernel only checks node IDs 0-6 even though the nodemask covers
node IDs 0-63.

Pass the nodemask size in bits plus one because get_nodes() in
mempolicy does --maxnode.

Fixes: 678e90a349a4 ("KVM: selftests: Test IPI to halted vCPU in xAPIC while backing page moves")
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 tools/testing/selftests/kvm/x86/xapic_ipi_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
index 39ce9a9369f5..761e47e4cae2 100644
--- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
+++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
@@ -310,7 +310,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
 		 * KVM_CREATE_VCPU ioctl. If that assumption ever changes this
 		 * test may break or give a false positive signal.
 		 */
-		pages_not_moved = migrate_pages(0, sizeof(nodemasks[from]),
+		pages_not_moved = migrate_pages(0, sizeof(nodemasks[from]) * 8 + 1,
 						&nodemasks[from],
 						&nodemasks[to]);
 		if (pages_not_moved < 0)

-- 
2.43.0

[PATCH 3/5] KVM: selftests: use BITS_PER_TYPE() for NUMA masks

[Shivank Garg]

Replace the open-coded sizeof() * 8 calculations in do_migrations() with
BITS_PER_TYPE().

No functional change intended.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 tools/testing/selftests/kvm/x86/xapic_ipi_test.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
index 761e47e4cae2..769d8d95ab2c 100644
--- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
+++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
@@ -252,7 +252,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
 {
 	long pages_not_moved;
 	unsigned long nodemask = 0;
-	unsigned long nodemasks[sizeof(nodemask) * 8];
+	unsigned long nodemasks[BITS_PER_TYPE(nodemask)];
 	int nodes = 0;
 	time_t start_time, last_update, now;
 	time_t interval_secs = 1;
@@ -267,18 +267,18 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
 		delay_usecs);
 
 	/* Get set of first 64 numa nodes available */
-	kvm_get_mempolicy(NULL, &nodemask, sizeof(nodemask) * 8,
+	kvm_get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask),
 			  0, MPOL_F_MEMS_ALLOWED);
 
 	fprintf(stderr, "Numa nodes found amongst first %lu possible nodes "
 		"(each 1-bit indicates node is present): %#lx\n",
-		sizeof(nodemask) * 8, nodemask);
+		BITS_PER_TYPE(nodemask), nodemask);
 
 	/* Init array of masks containing a single-bit in each, one for each
 	 * available node. migrate_pages called below requires specifying nodes
 	 * as bit masks.
 	 */
-	for (i = 0, bit = 1; i < sizeof(nodemask) * 8; i++, bit <<= 1) {
+	for (i = 0, bit = 1; i < BITS_PER_TYPE(nodemask); i++, bit <<= 1) {
 		if (nodemask & bit) {
 			nodemasks[nodes] = nodemask & bit;
 			nodes++;
@@ -310,7 +310,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
 		 * KVM_CREATE_VCPU ioctl. If that assumption ever changes this
 		 * test may break or give a false positive signal.
 		 */
-		pages_not_moved = migrate_pages(0, sizeof(nodemasks[from]) * 8 + 1,
+		pages_not_moved = migrate_pages(0, BITS_PER_TYPE(nodemasks[from]) + 1,
 						&nodemasks[from],
 						&nodemasks[to]);
 		if (pages_not_moved < 0)

-- 
2.43.0

[PATCH 4/5] KVM: selftests: add get_numa_mem_nodes()

[Shivank Garg]

The xAPIC IPI test uses MPOL_F_MEMS_ALLOWED to get the memory nodes
available to the current process. Move the query to numaif.h as
get_numa_mem_nodes() so other KVM selftests can use it.

Call get_mempolicy() directly instead of using the assert-on-failure
kvm_get_mempolicy() wrapper. The xAPIC test already asserts its two
node requirement.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 tools/testing/selftests/kvm/include/numaif.h     | 12 ++++++++++++
 tools/testing/selftests/kvm/x86/xapic_ipi_test.c |  6 ++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/numaif.h b/tools/testing/selftests/kvm/include/numaif.h
index 29572a6d789c..55124e2330ab 100644
--- a/tools/testing/selftests/kvm/include/numaif.h
+++ b/tools/testing/selftests/kvm/include/numaif.h
@@ -75,6 +75,18 @@ static bool is_numa_available(void)
 		(errno != ENOSYS && errno != EPERM);
 }
 
+static inline unsigned long get_numa_mem_nodes(void)
+{
+	unsigned long nodemask = 0;
+
+	/* Get set of first 64 numa nodes available */
+	if (get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask), NULL,
+			  MPOL_F_MEMS_ALLOWED))
+		return 0;
+
+	return nodemask;
+}
+
 static inline bool is_multi_numa_node_system(void)
 {
 	return is_numa_available() && get_max_numa_node() >= 1;
diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
index 769d8d95ab2c..66dcf36398aa 100644
--- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
+++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
@@ -251,7 +251,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
 		   u64 *pipis_rcvd)
 {
 	long pages_not_moved;
-	unsigned long nodemask = 0;
+	unsigned long nodemask;
 	unsigned long nodemasks[BITS_PER_TYPE(nodemask)];
 	int nodes = 0;
 	time_t start_time, last_update, now;
@@ -266,9 +266,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
 	fprintf(stderr, "Calling migrate_pages every %d microseconds\n",
 		delay_usecs);
 
-	/* Get set of first 64 numa nodes available */
-	kvm_get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask),
-			  0, MPOL_F_MEMS_ALLOWED);
+	nodemask = get_numa_mem_nodes();
 
 	fprintf(stderr, "Numa nodes found amongst first %lu possible nodes "
 		"(each 1-bit indicates node is present): %#lx\n",

-- 
2.43.0

[PATCH 5/5] KVM: selftests: use allowed NUMA nodes in guest_memfd_test

[Shivank Garg]

guest_memfd_test assumes that nodes 0 and 1 exist and have memory.
is_multi_numa_node_system() only checks that the maximum node ID is
nonzero, which is not enough for sparse or memoryless nodes.

Select the required nodes from MPOL_F_MEMS_ALLOWED instead. Use the full
nodemask width plus one to mbind(), and let test_mbind() run when only
one memory node is available.

The sysfs helpers for finding maxnode are no longer needed.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 tools/testing/selftests/kvm/guest_memfd_test.c | 86 +++++++++++++++++---------
 tools/testing/selftests/kvm/include/numaif.h   | 52 ----------------
 2 files changed, 58 insertions(+), 80 deletions(-)

diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index 2233d871a38f..aee80dda6229 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -76,33 +76,53 @@ static void test_mmap_supported(int fd, size_t total_size)
 	kvm_munmap(mem, total_size);
 }
 
+/*
+ * Fill @nids with the first @nr_nids nodes in the allowed mask.
+ * Return false if the mask contains fewer than @nr_nids nodes.
+ */
+static bool get_numa_node_ids(int *nids, int nr_nids)
+{
+	unsigned long nodemask = get_numa_mem_nodes();
+	unsigned long nid;
+	int nr_found = 0;
+
+	for_each_set_bit(nid, &nodemask, BITS_PER_TYPE(nodemask)) {
+		nids[nr_found++] = nid;
+		if (nr_found == nr_nids)
+			return true;
+	}
+
+	return false;
+}
+
 static void test_mbind(int fd, size_t total_size)
 {
-	const unsigned long nodemask_0 = 1; /* nid: 0 */
-	unsigned long nodemask = 0;
-	unsigned long maxnode = BITS_PER_TYPE(nodemask);
+	unsigned long nodemask, bind_nodemask;
+	unsigned long maxnode = BITS_PER_TYPE(nodemask) + 1;
 	int policy;
 	char *mem;
+	int nid;
 	int ret;
 
-	if (!is_multi_numa_node_system())
+	if (!get_numa_node_ids(&nid, 1))
 		return;
 
+	bind_nodemask = 1UL << nid;
 	mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
 
 	/* Test MPOL_INTERLEAVE policy */
-	kvm_mbind(mem, page_size * 2, MPOL_INTERLEAVE, &nodemask_0, maxnode, 0);
+	kvm_mbind(mem, page_size * 2, MPOL_INTERLEAVE, &bind_nodemask, maxnode, 0);
 	kvm_get_mempolicy(&policy, &nodemask, maxnode, mem, MPOL_F_ADDR);
-	TEST_ASSERT(policy == MPOL_INTERLEAVE && nodemask == nodemask_0,
+	TEST_ASSERT(policy == MPOL_INTERLEAVE && nodemask == bind_nodemask,
 		    "Wanted MPOL_INTERLEAVE (%u) and nodemask 0x%lx, got %u and 0x%lx",
-		    MPOL_INTERLEAVE, nodemask_0, policy, nodemask);
+		    MPOL_INTERLEAVE, bind_nodemask, policy, nodemask);
 
 	/* Test basic MPOL_BIND policy */
-	kvm_mbind(mem + page_size * 2, page_size * 2, MPOL_BIND, &nodemask_0, maxnode, 0);
+	kvm_mbind(mem + page_size * 2, page_size * 2, MPOL_BIND, &bind_nodemask, maxnode, 0);
 	kvm_get_mempolicy(&policy, &nodemask, maxnode, mem + page_size * 2, MPOL_F_ADDR);
-	TEST_ASSERT(policy == MPOL_BIND && nodemask == nodemask_0,
+	TEST_ASSERT(policy == MPOL_BIND && nodemask == bind_nodemask,
 		    "Wanted MPOL_BIND (%u) and nodemask 0x%lx, got %u and 0x%lx",
-		    MPOL_BIND, nodemask_0, policy, nodemask);
+		    MPOL_BIND, bind_nodemask, policy, nodemask);
 
 	/* Test MPOL_DEFAULT policy */
 	kvm_mbind(mem, total_size, MPOL_DEFAULT, NULL, 0, 0);
@@ -112,7 +132,7 @@ static void test_mbind(int fd, size_t total_size)
 		    MPOL_DEFAULT, policy, nodemask);
 
 	/* Test with invalid policy */
-	ret = mbind(mem, page_size, 999, &nodemask_0, maxnode, 0);
+	ret = mbind(mem, page_size, 999, &bind_nodemask, maxnode, 0);
 	TEST_ASSERT(ret == -1 && errno == EINVAL,
 		    "mbind with invalid policy should fail with EINVAL");
 
@@ -121,17 +141,19 @@ static void test_mbind(int fd, size_t total_size)
 
 static void test_numa_allocation(int fd, size_t total_size)
 {
-	unsigned long node0_mask = 1;  /* Node 0 */
-	unsigned long node1_mask = 2;  /* Node 1 */
-	unsigned long maxnode = 8;
+	unsigned long bind_nodemasks[2];
+	unsigned long maxnode = BITS_PER_TYPE(bind_nodemasks[0]) + 1;
 	void *pages[4];
+	int nids[2];
 	int status[4];
 	char *mem;
 	int i;
 
-	if (!is_multi_numa_node_system())
+	if (!get_numa_node_ids(nids, ARRAY_SIZE(nids)))
 		return;
 
+	bind_nodemasks[0] = 1UL << nids[0];
+	bind_nodemasks[1] = 1UL << nids[1];
 	mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
 
 	for (i = 0; i < 4; i++)
@@ -139,34 +161,42 @@ static void test_numa_allocation(int fd, size_t total_size)
 
 	/* Set NUMA policy after allocation */
 	memset(mem, 0xaa, page_size);
-	kvm_mbind(pages[0], page_size, MPOL_BIND, &node0_mask, maxnode, 0);
+	kvm_mbind(pages[0], page_size, MPOL_BIND, &bind_nodemasks[0], maxnode, 0);
 	kvm_fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0, page_size);
 
 	/* Set NUMA policy before allocation */
-	kvm_mbind(pages[0], page_size * 2, MPOL_BIND, &node1_mask, maxnode, 0);
-	kvm_mbind(pages[2], page_size * 2, MPOL_BIND, &node0_mask, maxnode, 0);
+	kvm_mbind(pages[0], page_size * 2, MPOL_BIND, &bind_nodemasks[1], maxnode, 0);
+	kvm_mbind(pages[2], page_size * 2, MPOL_BIND, &bind_nodemasks[0], maxnode, 0);
 	memset(mem, 0xaa, total_size);
 
 	/* Validate if pages are allocated on specified NUMA nodes */
 	kvm_move_pages(0, 4, pages, NULL, status, 0);
-	TEST_ASSERT(status[0] == 1, "Expected page 0 on node 1, got it on node %d", status[0]);
-	TEST_ASSERT(status[1] == 1, "Expected page 1 on node 1, got it on node %d", status[1]);
-	TEST_ASSERT(status[2] == 0, "Expected page 2 on node 0, got it on node %d", status[2]);
-	TEST_ASSERT(status[3] == 0, "Expected page 3 on node 0, got it on node %d", status[3]);
+	TEST_ASSERT(status[0] == nids[1],
+		    "Expected page 0 on node %d, got it on node %d", nids[1], status[0]);
+	TEST_ASSERT(status[1] == nids[1],
+		    "Expected page 1 on node %d, got it on node %d", nids[1], status[1]);
+	TEST_ASSERT(status[2] == nids[0],
+		    "Expected page 2 on node %d, got it on node %d", nids[0], status[2]);
+	TEST_ASSERT(status[3] == nids[0],
+		    "Expected page 3 on node %d, got it on node %d", nids[0], status[3]);
 
 	/* Punch hole for all pages */
 	kvm_fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0, total_size);
 
 	/* Change NUMA policy nodes and reallocate */
-	kvm_mbind(pages[0], page_size * 2, MPOL_BIND, &node0_mask, maxnode, 0);
-	kvm_mbind(pages[2], page_size * 2, MPOL_BIND, &node1_mask, maxnode, 0);
+	kvm_mbind(pages[0], page_size * 2, MPOL_BIND, &bind_nodemasks[0], maxnode, 0);
+	kvm_mbind(pages[2], page_size * 2, MPOL_BIND, &bind_nodemasks[1], maxnode, 0);
 	memset(mem, 0xaa, total_size);
 
 	kvm_move_pages(0, 4, pages, NULL, status, 0);
-	TEST_ASSERT(status[0] == 0, "Expected page 0 on node 0, got it on node %d", status[0]);
-	TEST_ASSERT(status[1] == 0, "Expected page 1 on node 0, got it on node %d", status[1]);
-	TEST_ASSERT(status[2] == 1, "Expected page 2 on node 1, got it on node %d", status[2]);
-	TEST_ASSERT(status[3] == 1, "Expected page 3 on node 1, got it on node %d", status[3]);
+	TEST_ASSERT(status[0] == nids[0],
+		    "Expected page 0 on node %d, got it on node %d", nids[0], status[0]);
+	TEST_ASSERT(status[1] == nids[0],
+		    "Expected page 1 on node %d, got it on node %d", nids[0], status[1]);
+	TEST_ASSERT(status[2] == nids[1],
+		    "Expected page 2 on node %d, got it on node %d", nids[1], status[2]);
+	TEST_ASSERT(status[3] == nids[1],
+		    "Expected page 3 on node %d, got it on node %d", nids[1], status[3]);
 
 	kvm_munmap(mem, total_size);
 }
diff --git a/tools/testing/selftests/kvm/include/numaif.h b/tools/testing/selftests/kvm/include/numaif.h
index 55124e2330ab..4bbbf314e9d7 100644
--- a/tools/testing/selftests/kvm/include/numaif.h
+++ b/tools/testing/selftests/kvm/include/numaif.h
@@ -4,8 +4,6 @@
 #ifndef SELFTEST_KVM_NUMAIF_H
 #define SELFTEST_KVM_NUMAIF_H
 
-#include <dirent.h>
-
 #include <linux/mempolicy.h>
 
 #include "kvm_syscalls.h"
@@ -30,51 +28,6 @@ KVM_SYSCALL_DEFINE(mbind, 6, void *, addr, unsigned long, size, int, mode,
 		   const unsigned long *, nodemask, unsigned long, maxnode,
 		   unsigned int, flags);
 
-static inline int get_max_numa_node(void)
-{
-	struct dirent *de;
-	int max_node = 0;
-	DIR *d;
-
-	/*
-	 * Assume there's a single node if the kernel doesn't support NUMA,
-	 * or if no nodes are found.
-	 */
-	d = opendir("/sys/devices/system/node");
-	if (!d)
-		return 0;
-
-	while ((de = readdir(d)) != NULL) {
-		int node_id;
-		char *endptr;
-
-		if (strncmp(de->d_name, "node", 4) != 0)
-			continue;
-
-		node_id = strtol(de->d_name + 4, &endptr, 10);
-		if (*endptr != '\0')
-			continue;
-
-		if (node_id > max_node)
-			max_node = node_id;
-	}
-	closedir(d);
-
-	return max_node;
-}
-
-static bool is_numa_available(void)
-{
-	/*
-	 * Probe for NUMA by doing a dummy get_mempolicy().  If the syscall
-	 * fails with ENOSYS, then the kernel was built without NUMA support.
-	 * if the syscall fails with EPERM, then the process/user lacks the
-	 * necessary capabilities (CAP_SYS_NICE).
-	 */
-	return !get_mempolicy(NULL, NULL, 0, NULL, 0) ||
-		(errno != ENOSYS && errno != EPERM);
-}
-
 static inline unsigned long get_numa_mem_nodes(void)
 {
 	unsigned long nodemask = 0;
@@ -87,9 +40,4 @@ static inline unsigned long get_numa_mem_nodes(void)
 	return nodemask;
 }
 
-static inline bool is_multi_numa_node_system(void)
-{
-	return is_numa_available() && get_max_numa_node() >= 1;
-}
-
 #endif /* SELFTEST_KVM_NUMAIF_H */

-- 
2.43.0

Re: [PATCH 4/5] KVM: selftests: add get_numa_mem_nodes()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] `numaif.h` uses the `BITS_PER_TYPE` macro but does not include the header that defines it.
--

--- Patch [4]: [PATCH 4/5] KVM: selftests: add get_numa_mem_nodes() ---
commit 307e268608f2261ad345e9a86e81fdd51a53d40a
Author: Shivank Garg <shivankg@amd.com>
KVM: selftests: add get_numa_mem_nodes()

This patch moves the memory node query using MPOL_F_MEMS_ALLOWED from the
xAPIC IPI test into numaif.h as get_numa_mem_nodes(). This allows other
KVM selftests to use it directly instead of duplicating the logic.

> diff --git a/tools/testing/selftests/kvm/include/numaif.h b/tools/testing/selftests/kvm/include/numaif.h
> index 29572a6d789cd..55124e2330ab6 100644
> --- a/tools/testing/selftests/kvm/include/numaif.h
> +++ b/tools/testing/selftests/kvm/include/numaif.h
> @@ -75,6 +75,18 @@ static bool is_numa_available(void)
[ ... ]
> +static inline unsigned long get_numa_mem_nodes(void)
> +{
> +	unsigned long nodemask = 0;
> +
> +	/* Get set of first 64 numa nodes available */
> +	if (get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask), NULL,
> +			  MPOL_F_MEMS_ALLOWED))

[Severity: Low]
Does numaif.h need to include the header that defines BITS_PER_TYPE?
If a test includes numaif.h without previously including the header defining
this macro, it might fail to compile due to a missing definition, breaking
header self-containment.

> +		return 0;
> +
> +	return nodemask;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260823-shivank-gmem-fix-split-v1-0-512a29fb8e86@amd.com?part=4

Re: [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Ignoring the return value of xa_store_range() in guest_memfd binding paths leads to a critical Use-After-Free vulnerability.
--

--- Patch [1]: [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file ---
commit d844dab82ec050e986f6764e412dddfb4f90c309
Author: Shivank Garg <shivankg@amd.com>

KVM: guest_memfd: take the invalidate lock when unbinding a dying file

kvm_gmem_unbind() skips mapping->invalidate_lock when the guest_memfd
file is already dying. All other paths that modify f->bindings hold
that lock. Take the invalidate lock in the dying-file path too to prevent
unbind from removing a binding and leaking mmu_invalidate_in_progress.

Closes: https://lore.kernel.org/all/20260728092027.225CF1F000E9@smtp.kernel.org

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index f0e5da4908660..f848120af84be 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -721,6 +721,8 @@ static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)

[Severity: High]
This isn't a bug introduced by this patch, but does ignoring the return value
of xa_store_range() here and in kvm_gmem_bind() lead to a use-after-free?

Looking at kvm_gmem_bind() in virt/kvm/guest_memfd.c, xa_store_range() is
called with GFP_KERNEL:

virt/kvm/guest_memfd.c:kvm_gmem_bind() {
	...
	xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
	filemap_invalidate_unlock(inode->i_mapping);

	/*
	 * Drop the reference to the file, even on success.  The file pins KVM,
	 * not the other way 'round.  Active bindings are invalidated if the
	 * file is closed before memslots are destroyed.
	 */
	r = 0;
	...
}

If memory allocation fails during this operation, xa_store_range() returns an
error pointer, but the return value is ignored. This masks ENOMEM errors and
leads to an inconsistent state where the slot points to the file but the file
doesn't track the slot.

Because the slot is missing from the file's XArray, when the file is later
closed, kvm_gmem_release() iterates over f->bindings but fails to find this
slot, leaving slot->gmem.file as a dangling pointer.

When the VM destroys the memslot, kvm_gmem_unbind() accesses this dangling
pointer and calls get_file_active() on it. Can this cause a use-after-free on
the freed struct file, or improperly increment the refcount of an unrelated,
reused memory object?

Similarly, in __kvm_gmem_unbind():

virt/kvm/guest_memfd.c:__kvm_gmem_unbind() {
	...
	xa_store_range(&f->bindings, start, end - 1, NULL, GFP_KERNEL);
	...
}

The return value is ignored here as well, masking errors when removing from
the XArray. Should the return value of xa_store_range() be checked and
handled to prevent these vulnerabilities?

>  
>  void kvm_gmem_unbind(struct kvm_memory_slot *slot)
>  {
> +	struct file *gmem_file;
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260823-shivank-gmem-fix-split-v1-0-512a29fb8e86@amd.com?part=1

Re: [PATCH 4/5] KVM: selftests: add get_numa_mem_nodes()

[Garg, Shivank]

On Sun, 2026-08-23 at 13:36 +0000, Shivank Garg wrote:
> The xAPIC IPI test uses MPOL_F_MEMS_ALLOWED to get the memory nodes
> available to the current process. Move the query to numaif.h as
> get_numa_mem_nodes() so other KVM selftests can use it.
> 
> Call get_mempolicy() directly instead of using the assert-on-failure
> kvm_get_mempolicy() wrapper. The xAPIC test already asserts its two
> node requirement.
> 
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  tools/testing/selftests/kvm/include/numaif.h     | 12 ++++++++++++
>  tools/testing/selftests/kvm/x86/xapic_ipi_test.c |  6 ++----
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/numaif.h b/tools/testing/selftests/kvm/include/numaif.h
> index 29572a6d789c..55124e2330ab 100644
> --- a/tools/testing/selftests/kvm/include/numaif.h
> +++ b/tools/testing/selftests/kvm/include/numaif.h
> @@ -75,6 +75,18 @@ static bool is_numa_available(void)
>  		(errno != ENOSYS && errno != EPERM);
>  }
>  
> +static inline unsigned long get_numa_mem_nodes(void)
> +{
> +	unsigned long nodemask = 0;
> +
> +	/* Get set of first 64 numa nodes available */
> +	if (get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask), NULL,
> +			  MPOL_F_MEMS_ALLOWED))
> +		return 0;
> +
> +	return nodemask;
> +}
> +
>  static inline bool is_multi_numa_node_system(void)
>  {
>  	return is_numa_available() && get_max_numa_node() >= 1;
> diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> index 769d8d95ab2c..66dcf36398aa 100644
> --- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> +++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> @@ -251,7 +251,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
>  		   u64 *pipis_rcvd)
>  {
>  	long pages_not_moved;
> -	unsigned long nodemask = 0;
> +	unsigned long nodemask;
>  	unsigned long nodemasks[BITS_PER_TYPE(nodemask)];
>  	int nodes = 0;
>  	time_t start_time, last_update, now;
> @@ -266,9 +266,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
>  	fprintf(stderr, "Calling migrate_pages every %d microseconds\n",
>  		delay_usecs);
>  
> -	/* Get set of first 64 numa nodes available */
> -	kvm_get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask),
> -			  0, MPOL_F_MEMS_ALLOWED);
> +	nodemask = get_numa_mem_nodes();
>  
>  	fprintf(stderr, "Numa nodes found amongst first %lu possible nodes "
>  		"(each 1-bit indicates node is present): %#lx\n",
> 


From fd118b48e046c87d9b645b4ce7fc4ac5c0656431 Mon Sep 17 00:00:00 2001
From: Shivank Garg <shivankg@amd.com>
Date: Sun, 23 Aug 2026 14:11:42 +0000
Subject: [PATCH] Fix potential compilation issues

Sashiko reported that numaif.h does not include header that defines
BITS_PER_TYPE. If a test includes numaif.h without previously including
the header defining this macro, it might fail to compile due to a missing
definition, breaking header self-containment.

Link: https://lore.kernel.org/kvm/20260823134800.596C51F000E9@smtp.kernel.org
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 tools/testing/selftests/kvm/include/numaif.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/kvm/include/numaif.h b/tools/testing/selftests/kvm/include/numaif.h
index 4bbbf314e9d7..b89559b6ea3c 100644
--- a/tools/testing/selftests/kvm/include/numaif.h
+++ b/tools/testing/selftests/kvm/include/numaif.h
@@ -4,6 +4,7 @@
 #ifndef SELFTEST_KVM_NUMAIF_H
 #define SELFTEST_KVM_NUMAIF_H
 
+#include <linux/bits.h>
 #include <linux/mempolicy.h>
 
 #include "kvm_syscalls.h"
-- 
2.43.0

Re: [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file

[Sean Christopherson]

On Sun, Aug 23, 2026, sashiko-bot@kernel.org wrote:
> [Severity: High]
> This isn't a bug introduced by this patch, but does ignoring the return value
> of xa_store_range() here and in kvm_gmem_bind() lead to a use-after-free?
> 
> Looking at kvm_gmem_bind() in virt/kvm/guest_memfd.c, xa_store_range() is
> called with GFP_KERNEL:
> 
> virt/kvm/guest_memfd.c:kvm_gmem_bind() {
> 	...
> 	xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
> 	filemap_invalidate_unlock(inode->i_mapping);
> 
> 	/*
> 	 * Drop the reference to the file, even on success.  The file pins KVM,
> 	 * not the other way 'round.  Active bindings are invalidated if the
> 	 * file is closed before memslots are destroyed.
> 	 */
> 	r = 0;
> 	...
> }
> 
> If memory allocation fails during this operation, xa_store_range() returns an
> error pointer, but the return value is ignored. This masks ENOMEM errors and
> leads to an inconsistent state where the slot points to the file but the file
> doesn't track the slot.
> 
> Because the slot is missing from the file's XArray, when the file is later
> closed, kvm_gmem_release() iterates over f->bindings but fails to find this
> slot, leaving slot->gmem.file as a dangling pointer.
> 
> When the VM destroys the memslot, kvm_gmem_unbind() accesses this dangling
> pointer and calls get_file_active() on it. Can this cause a use-after-free on
> the freed struct file, or improperly increment the refcount of an unrelated,
> reused memory object?

Thanks Sashiko, for spilling the beans.  I've been sitting on a patch for about
a week (this got reported/found by multiple sources internally), I'll get it posted
today.

Re: [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file

[Sean Christopherson]

On Sun, Aug 23, 2026, Shivank Garg wrote:
> kvm_gmem_unbind() skips mapping->invalidate_lock when the guest_memfd
> file is already dying. All other paths that modify f->bindings hold
> that lock.
> 
> kvm_gmem_invalidate_{start,end}() checks f->bindings independently to
> decide whether to begin or end KVM MMU invalidations. So, the bindings
> must remain stable between the two calls. If a binding is removed in that
> window, start increments mmu_invalidate_in_progress but end does not
> decrement it. Example, unbind race with memory failure:
> 
>   CPU 0: memory failure               CPU 1: memslot delete
>   ----------------------------------  ---------------------------
>                                       (guest_memfd file is dying)
>   kvm_gmem_error_folio()
>     kvm_gmem_invalidate_start()
>       finds binding
>       mmu_invalidate_in_progress++
>                                       kvm_gmem_unbind()
>                                         get_file_active() fails
>                                         store NULL in bindings
>     kvm_gmem_invalidate_end()
>       no binding found
>       counter stays elevated
> 
> mmu_invalidate_retry() then returns 1 forever, so guest page faults
> retry without ever installing a mapping and the guest hangs.
> 
> Take the invalidate lock in the dying-file path too. This prevents unbind
> from removing a binding and leaking mmu_invalidate_in_progress. This is
> safe because any caller that reaches this path holds slots_lock, so
> kvm_gmem_release() cannot nullify the slot->gmem.file, until
> kvm_gmem_unbind() finishes.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/20260728092027.225CF1F000E9@smtp.kernel.org
> Fixes: ae431059e75d ("KVM: guest_memfd: Remove bindings on memslot deletion when gmem is dying")
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  virt/kvm/guest_memfd.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index f0e5da490866..f848120af84b 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -721,6 +721,8 @@ static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
>  
>  void kvm_gmem_unbind(struct kvm_memory_slot *slot)
>  {
> +	struct file *gmem_file;
> +
>  	/*
>  	 * Nothing to do if the underlying file was _already_ closed, as
>  	 * kvm_gmem_release() invalidates and nullifies all bindings.
> @@ -733,21 +735,24 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
>  	/*
>  	 * However, if the file is _being_ closed, then the bindings need to be
>  	 * removed as kvm_gmem_release() might not run until after the memslot
> -	 * is freed.  Note, modifying the bindings is safe even though the file
> -	 * is dying as kvm_gmem_release() nullifies slot->gmem.file under
> +	 * is freed.  Note, dereferencing the dying file is safe as
> +	 * kvm_gmem_release() nullifies slot->gmem.file under
>  	 * slots_lock, and only puts its reference to KVM after destroying all
>  	 * bindings.  I.e. reaching this point means kvm_gmem_release() hasn't
>  	 * yet destroyed the bindings or freed the gmem_file, and can't do so
>  	 * until the caller drops slots_lock.
>  	 */
> -	if (!file) {
> -		__kvm_gmem_unbind(slot, slot->gmem.file->private_data);
> -		return;
> -	}
> +	gmem_file = file ?: slot->gmem.file;
>  
> -	filemap_invalidate_lock(file->f_mapping);
> -	__kvm_gmem_unbind(slot, file->private_data);
> -	filemap_invalidate_unlock(file->f_mapping);
> +	/*
> +	 * Take the invalidate lock even for a dying file.  Otherwise,
> +	 * kvm_gmem_invalidate_start() can find the binding and increment
> +	 * mmu_invalidate_in_progress while kvm_gmem_invalidate_end() misses
> +	 * the removed binding and skips decrement.

Hmm, so as called out in commit ae431059e75d ("KVM: guest_memfd: Remove bindings
on memslot deletion when gmem is dying"), this assumes that file->f_mapping and
everything "underneath" remains valid for dying files, which makes me a bit
uncomfortable.

    Deliberately don't acquire filemap invalid lock when the file is dying as
    the lifecycle of f_mapping is outside the purview of KVM.  Dereferencing
    the mapping is *probably* fine, but there's no need to invalidate anything
    as memslot deletion is responsible for zapping SPTEs, and the only code
    that can access the dying file is kvm_gmem_release(), whose core code is
    mutually exclusive with unbinding.

Oh, but kvm_gmem_release() takes the same filemap_invalidate_unlock() and
holding slots_lock guarantees that this code would run before release() if it
sees a non-null slot->gmem.file, i.e. past me's concern is completely unfounded.

Rather than make this seem like something special, IMO we should treat this as
a more normal thing.  kvm->slots_lock is already load bearing, might as well
double down on that.  I.e. the exceptional part is doing all the work even though
the file is dying, but the flows themselves should be identical.

E.g.

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index b596486d184c..5cc043466c89 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -668,48 +668,40 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
 	return r;
 }
 
-static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
+void kvm_gmem_unbind(struct kvm_memory_slot *slot)
 {
+	struct file *file = slot->gmem.file;
 	unsigned long start = slot->gmem.pgoff;
 	unsigned long end = start + slot->npages;
+	struct gmem_file *f;
 
-	xa_store_range(&f->bindings, start, end - 1, NULL, GFP_KERNEL);
-
-	/*
-	 * synchronize_srcu(&kvm->srcu) ensured that kvm_gmem_get_pfn()
-	 * cannot see this memslot.
-	 */
-	WRITE_ONCE(slot->gmem.file, NULL);
-}
-
-void kvm_gmem_unbind(struct kvm_memory_slot *slot)
-{
 	/*
 	 * Nothing to do if the underlying file was _already_ closed, as
 	 * kvm_gmem_release() invalidates and nullifies all bindings.
 	 */
-	if (!slot->gmem.file)
+	if (!file)
 		return;
 
-	CLASS(gmem_get_file, file)(slot);
-
 	/*
 	 * However, if the file is _being_ closed, then the bindings need to be
 	 * removed as kvm_gmem_release() might not run until after the memslot
-	 * is freed.  Note, modifying the bindings is safe even though the file
-	 * is dying as kvm_gmem_release() nullifies slot->gmem.file under
-	 * slots_lock, and only puts its reference to KVM after destroying all
-	 * bindings.  I.e. reaching this point means kvm_gmem_release() hasn't
-	 * yet destroyed the bindings or freed the gmem_file, and can't do so
-	 * until the caller drops slots_lock.
+	 * is freed.  Modifying the bindings is safe even if the file is dying
+	 * as kvm_gmem_release() nullifies slot->gmem.file under slots_lock,
+	 * and only puts its reference to KVM after destroying all bindings.
+	 * I.e. reaching this point means kvm_gmem_release() hasn't destroyed
+	 * the bindings or freed the gmem_file and can't do so until the caller
+	 * drops slots_lock, so there's no need to verify the file is live.
 	 */
-	if (!file) {
-		__kvm_gmem_unbind(slot, slot->gmem.file->private_data);
-		return;
-	}
+	f = file->private_data;
 
 	filemap_invalidate_lock(file->f_mapping);
-	__kvm_gmem_unbind(slot, file->private_data);
+	xa_store_range(&f->bindings, start, end - 1, NULL, GFP_KERNEL);
+
+	/*
+	 * synchronize_srcu(&kvm->srcu) ensured that kvm_gmem_get_pfn()
+	 * cannot see this memslot.
+	 */
+	WRITE_ONCE(slot->gmem.file, NULL);
 	filemap_invalidate_unlock(file->f_mapping);
 }

Re: [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file

[Garg, Shivank]

On Tue, 2026-08-25 at 15:13 -0700, Sean Christopherson wrote:
> On Sun, Aug 23, 2026, Shivank Garg wrote:
> > kvm_gmem_unbind() skips mapping->invalidate_lock when the guest_memfd
> > file is already dying. All other paths that modify f->bindings hold
> > that lock.
> > 
> > kvm_gmem_invalidate_{start,end}() checks f->bindings independently to
> > decide whether to begin or end KVM MMU invalidations. So, the bindings
> > must remain stable between the two calls. If a binding is removed in that
> > window, start increments mmu_invalidate_in_progress but end does not
> > decrement it. Example, unbind race with memory failure:
> > 
> >   CPU 0: memory failure               CPU 1: memslot delete
> >   ----------------------------------  ---------------------------
> >                                       (guest_memfd file is dying)
> >   kvm_gmem_error_folio()
> >     kvm_gmem_invalidate_start()
> >       finds binding
> >       mmu_invalidate_in_progress++
> >                                       kvm_gmem_unbind()
> >                                         get_file_active() fails
> >                                         store NULL in bindings
> >     kvm_gmem_invalidate_end()
> >       no binding found
> >       counter stays elevated
> > 
> > mmu_invalidate_retry() then returns 1 forever, so guest page faults
> > retry without ever installing a mapping and the guest hangs.
> > 
> > Take the invalidate lock in the dying-file path too. This prevents unbind
> > from removing a binding and leaking mmu_invalidate_in_progress. This is
> > safe because any caller that reaches this path holds slots_lock, so
> > kvm_gmem_release() cannot nullify the slot->gmem.file, until
> > kvm_gmem_unbind() finishes.
> > 
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/all/20260728092027.225CF1F000E9@smtp.kernel.org
> > Fixes: ae431059e75d ("KVM: guest_memfd: Remove bindings on memslot deletion when gmem is dying")
> > Signed-off-by: Shivank Garg <shivankg@amd.com>
> > ---
> >  virt/kvm/guest_memfd.c | 23 ++++++++++++++---------
> >  1 file changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index f0e5da490866..f848120af84b 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -721,6 +721,8 @@ static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
> >  
> >  void kvm_gmem_unbind(struct kvm_memory_slot *slot)
> >  {
> > +	struct file *gmem_file;
> > +
> >  	/*
> >  	 * Nothing to do if the underlying file was _already_ closed, as
> >  	 * kvm_gmem_release() invalidates and nullifies all bindings.
> > @@ -733,21 +735,24 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
> >  	/*
> >  	 * However, if the file is _being_ closed, then the bindings need to be
> >  	 * removed as kvm_gmem_release() might not run until after the memslot
> > -	 * is freed.  Note, modifying the bindings is safe even though the file
> > -	 * is dying as kvm_gmem_release() nullifies slot->gmem.file under
> > +	 * is freed.  Note, dereferencing the dying file is safe as
> > +	 * kvm_gmem_release() nullifies slot->gmem.file under
> >  	 * slots_lock, and only puts its reference to KVM after destroying all
> >  	 * bindings.  I.e. reaching this point means kvm_gmem_release() hasn't
> >  	 * yet destroyed the bindings or freed the gmem_file, and can't do so
> >  	 * until the caller drops slots_lock.
> >  	 */
> > -	if (!file) {
> > -		__kvm_gmem_unbind(slot, slot->gmem.file->private_data);
> > -		return;
> > -	}
> > +	gmem_file = file ?: slot->gmem.file;
> >  
> > -	filemap_invalidate_lock(file->f_mapping);
> > -	__kvm_gmem_unbind(slot, file->private_data);
> > -	filemap_invalidate_unlock(file->f_mapping);
> > +	/*
> > +	 * Take the invalidate lock even for a dying file.  Otherwise,
> > +	 * kvm_gmem_invalidate_start() can find the binding and increment
> > +	 * mmu_invalidate_in_progress while kvm_gmem_invalidate_end() misses
> > +	 * the removed binding and skips decrement.
> 
> Hmm, so as called out in commit ae431059e75d ("KVM: guest_memfd: Remove bindings
> on memslot deletion when gmem is dying"), this assumes that file->f_mapping and
> everything "underneath" remains valid for dying files, which makes me a bit
> uncomfortable.
> 
>     Deliberately don't acquire filemap invalid lock when the file is dying as
>     the lifecycle of f_mapping is outside the purview of KVM.  Dereferencing
>     the mapping is *probably* fine, but there's no need to invalidate anything
>     as memslot deletion is responsible for zapping SPTEs, and the only code
>     that can access the dying file is kvm_gmem_release(), whose core code is
>     mutually exclusive with unbinding.
> 
> Oh, but kvm_gmem_release() takes the same filemap_invalidate_unlock() and
> holding slots_lock guarantees that this code would run before release() if it
> sees a non-null slot->gmem.file, i.e. past me's concern is completely unfounded.
> 
> Rather than make this seem like something special, IMO we should treat this as
> a more normal thing.  kvm->slots_lock is already load bearing, might as well
> double down on that.  I.e. the exceptional part is doing all the work even though
> the file is dying, but the flows themselves should be identical.
> 
> E.g.
> 
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index b596486d184c..5cc043466c89 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -668,48 +668,40 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
>  	return r;
>  }
>  
> -static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
> +void kvm_gmem_unbind(struct kvm_memory_slot *slot)
>  {
> +	struct file *file = slot->gmem.file;
>  	unsigned long start = slot->gmem.pgoff;
>  	unsigned long end = start + slot->npages;
> +	struct gmem_file *f;
>  
> -	xa_store_range(&f->bindings, start, end - 1, NULL, GFP_KERNEL);
> -
> -	/*
> -	 * synchronize_srcu(&kvm->srcu) ensured that kvm_gmem_get_pfn()
> -	 * cannot see this memslot.
> -	 */
> -	WRITE_ONCE(slot->gmem.file, NULL);
> -}
> -
> -void kvm_gmem_unbind(struct kvm_memory_slot *slot)
> -{
>  	/*
>  	 * Nothing to do if the underlying file was _already_ closed, as
>  	 * kvm_gmem_release() invalidates and nullifies all bindings.
>  	 */
> -	if (!slot->gmem.file)
> +	if (!file)
>  		return;
>  
> -	CLASS(gmem_get_file, file)(slot);
> -
>  	/*
>  	 * However, if the file is _being_ closed, then the bindings need to be
>  	 * removed as kvm_gmem_release() might not run until after the memslot
> -	 * is freed.  Note, modifying the bindings is safe even though the file
> -	 * is dying as kvm_gmem_release() nullifies slot->gmem.file under
> -	 * slots_lock, and only puts its reference to KVM after destroying all
> -	 * bindings.  I.e. reaching this point means kvm_gmem_release() hasn't
> -	 * yet destroyed the bindings or freed the gmem_file, and can't do so
> -	 * until the caller drops slots_lock.
> +	 * is freed.  Modifying the bindings is safe even if the file is dying
> +	 * as kvm_gmem_release() nullifies slot->gmem.file under slots_lock,
> +	 * and only puts its reference to KVM after destroying all bindings.
> +	 * I.e. reaching this point means kvm_gmem_release() hasn't destroyed
> +	 * the bindings or freed the gmem_file and can't do so until the caller
> +	 * drops slots_lock, so there's no need to verify the file is live.
>  	 */
> -	if (!file) {
> -		__kvm_gmem_unbind(slot, slot->gmem.file->private_data);
> -		return;
> -	}
> +	f = file->private_data;
>  
>  	filemap_invalidate_lock(file->f_mapping);
> -	__kvm_gmem_unbind(slot, file->private_data);
> +	xa_store_range(&f->bindings, start, end - 1, NULL, GFP_KERNEL);
> +
> +	/*
> +	 * synchronize_srcu(&kvm->srcu) ensured that kvm_gmem_get_pfn()
> +	 * cannot see this memslot.
> +	 */
> +	WRITE_ONCE(slot->gmem.file, NULL);
>  	filemap_invalidate_unlock(file->f_mapping);
>  }


Thanks Sean.
This version is more simple and clear.

Best regards,
Shivank