Re: [PATCH 2/2] KVM: selftests: Test guest_memfd binding overlap without GPA overlap

[Sean Christopherson <seanjc@google.com>]

On Mon, May 18, 2026, Ackerley Tng wrote:
> ZongYao.Chen@linux.alibaba.com writes:
> 
> > From: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>
> >
> > The guest_memfd binding overlap test recreates the deleted slot with GPA
> > ranges that overlap the still-live slot.  KVM rejects those attempts from
> > the generic memslot overlap check before reaching kvm_gmem_bind(), so the
> > test can pass even if guest_memfd binding overlap detection is broken.
> >
> > Recreate the slot at its original, non-overlapping GPA and use guest_memfd
> > offsets that overlap the front and back halves of the other slot's binding.
> > Expand the guest_memfd so the back-half case remains within the file size.
> >
> > Fixes: 2feabb855df8 ("KVM: selftests: Expand set_memory_region_test to validate guest_memfd()")
> 
> Thanks for fixing this!
> 
> > Signed-off-by: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>
> > ---
> >  .../testing/selftests/kvm/set_memory_region_test.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
> > index 9b919a231c93..15607e0bec90 100644
> > --- a/tools/testing/selftests/kvm/set_memory_region_test.c
> > +++ b/tools/testing/selftests/kvm/set_memory_region_test.c
> > @@ -510,7 +510,7 @@ static void test_add_overlapping_private_memory_regions(void)
> 
> Shall we rename this to test_bind_overlapping_guest_memfd_offsets to
> make it clearer?

Hmm, not if we make the change additive (see blelow).
 
> Perhaps also update the pr_info() to "Testing binding to overlapping
> guest_memfd offsets\n".
> 
> >
> >  	vm = vm_create_barebones_type(KVM_X86_SW_PROTECTED_VM);
> >
> > -	memfd = vm_create_guest_memfd(vm, MEM_REGION_SIZE * 4, 0);
> > +	memfd = vm_create_guest_memfd(vm, MEM_REGION_SIZE * 6, 0);
> 
> I think this technically only needs to be MEM_REGION_SIZE * 5 for this
> test to work.
> 
> >
> >  	vm_set_user_memory_region2(vm, MEM_REGION_SLOT, KVM_MEM_GUEST_MEMFD,
> >  				   MEM_REGION_GPA, MEM_REGION_SIZE * 2, 0, memfd, 0);
> > @@ -526,19 +526,19 @@ static void test_add_overlapping_private_memory_regions(void)
> >  	vm_set_user_memory_region2(vm, MEM_REGION_SLOT, KVM_MEM_GUEST_MEMFD,
> >  				   MEM_REGION_GPA, 0, NULL, -1, 0);
> 
> When I re-read this I was wondering why we created and removed the first
> memslot. Was it meant as a confidence check that set_memory_region works
> with the given MEM_REGION_GPA? Perhaps we could add a comment/pr_info()
> to check that.

Rather than "fix" the check, why not have both?

> > -	/* Overlap the front half of the other slot. */
> > +	/* Overlap the front half of the other slot's guest_memfd binding. */
> >  	r = __vm_set_user_memory_region2(vm, MEM_REGION_SLOT, KVM_MEM_GUEST_MEMFD,
> > -					 MEM_REGION_GPA * 2 - MEM_REGION_SIZE,
> > +					 MEM_REGION_GPA,
> >  					 MEM_REGION_SIZE * 2,
> > -					 0, memfd, 0);
> > +					 0, memfd, MEM_REGION_SIZE);
> >  	TEST_ASSERT(r == -1 && errno == EEXIST, "%s",
> >  		    "Overlapping guest_memfd() bindings should fail with EEXIST");
> >
> > -	/* And now the back half of the other slot. */
> > +	/* And now the back half of the other slot's guest_memfd binding. */
> >  	r = __vm_set_user_memory_region2(vm, MEM_REGION_SLOT, KVM_MEM_GUEST_MEMFD,
> > -					 MEM_REGION_GPA * 2 + MEM_REGION_SIZE,
> > +					 MEM_REGION_GPA,
> >  					 MEM_REGION_SIZE * 2,
> > -					 0, memfd, 0);
> > +					 0, memfd, MEM_REGION_SIZE * 3);
> >  	TEST_ASSERT(r == -1 && errno == EEXIST, "%s",
> >  		    "Overlapping guest_memfd() bindings should fail with EEXIST");
> >
> 
> Since this test program is meant to test set_memory_region, should we be
> retaining the original test? The original test is wrong in that it
> doesn't test guest_memfd's binding, but it does test that
> set_memory_region returns -EEXIST on overlapping GPAs.
> 
> Perhaps to just test overlapping GPAs we can use anonymous memory
> instead of guest_memfd.

Eh, I see no harm in having both.  E.g. if we do this, then we don't have to
explain why we're not testing the other case :-)

diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
index 9b919a231c93..283392bcc3a0 100644
--- a/tools/testing/selftests/kvm/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/set_memory_region_test.c
@@ -510,7 +510,7 @@ static void test_add_overlapping_private_memory_regions(void)
 
        vm = vm_create_barebones_type(KVM_X86_SW_PROTECTED_VM);
 
-       memfd = vm_create_guest_memfd(vm, MEM_REGION_SIZE * 4, 0);
+       memfd = vm_create_guest_memfd(vm, MEM_REGION_SIZE * 5, 0);
 
        vm_set_user_memory_region2(vm, MEM_REGION_SLOT, KVM_MEM_GUEST_MEMFD,
                                   MEM_REGION_GPA, MEM_REGION_SIZE * 2, 0, memfd, 0);
@@ -542,6 +542,26 @@ static void test_add_overlapping_private_memory_regions(void)
        TEST_ASSERT(r == -1 && errno == EEXIST, "%s",
                    "Overlapping guest_memfd() bindings should fail with EEXIST");
 
+       /*
+        * Repeat the overlap tests, but so that there is overlap in the
+        * guest_memfd bindings (i.e. in guest_memfd file offsets), but _not_
+        * in the GPA space.  Regardless of where there's overlap, KVM should
+        * return -EEXIST.
+        */
+       r = __vm_set_user_memory_region2(vm, MEM_REGION_SLOT, KVM_MEM_GUEST_MEMFD,
+                                        MEM_REGION_GPA,
+                                        MEM_REGION_SIZE * 2,
+                                        0, memfd, MEM_REGION_SIZE);
+       TEST_ASSERT(r == -1 && errno == EEXIST, "%s",
+                   "Overlapping guest_memfd() bindings should fail with EEXIST");
+
+       /* And now the back half of the other slot's guest_memfd binding. */
+       r = __vm_set_user_memory_region2(vm, MEM_REGION_SLOT, KVM_MEM_GUEST_MEMFD,
+                                        MEM_REGION_GPA,
+                                        MEM_REGION_SIZE * 2,
+                                        0, memfd, MEM_REGION_SIZE * 3);
+       TEST_ASSERT(r == -1 && errno == EEXIST, "%s",
+                   "Overlapping guest_memfd() bindings should fail with EEXIST");
        close(memfd);
        kvm_vm_free(vm);
 }