1. Patchew
  2. linux
  3. [PATCH 0/6] KVM: Avoid a lurking guest_memfd ABI mess
  4. View patch

[PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails

Sean Christopherson posted 6 patches 5 days, 6 hours ago
[PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails
Posted by Sean Christopherson 5 days, 6 hours ago 
Add a guest_memfd testcase to verify that faulting in private memory gets
a SIGBUS.  For now, test only the case where memory is private by default
since KVM doesn't yet support in-place conversion.

Cc: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../testing/selftests/kvm/guest_memfd_test.c  | 62 ++++++++++++++-----
 1 file changed, 46 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index 5dd40b77dc07..b5a631aca933 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -40,17 +40,26 @@ static void test_file_read_write(int fd, size_t total_size)
 		    "pwrite on a guest_mem fd should fail");
 }
 
-static void test_mmap_supported(int fd, size_t total_size)
+static void *test_mmap_common(int fd, size_t size)
 {
-	const char val = 0xaa;
-	char *mem;
-	size_t i;
-	int ret;
+	void *mem;
 
-	mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
 	TEST_ASSERT(mem == MAP_FAILED, "Copy-on-write not allowed by guest_memfd.");
 
-	mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
+	mem = kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
+
+	return mem;
+}
+
+static void test_mmap_supported(int fd, size_t total_size)
+{
+	const char val = 0xaa;
+	char *mem;
+	size_t i;
+	int ret;
+
+	mem = test_mmap_common(fd, total_size);
 
 	memset(mem, val, total_size);
 	for (i = 0; i < total_size; i++)
@@ -78,31 +87,47 @@ void fault_sigbus_handler(int signum)
 	siglongjmp(jmpbuf, 1);
 }
 
-static void test_fault_overflow(int fd, size_t total_size)
+static void *test_fault_sigbus(int fd, size_t size)
 {
 	struct sigaction sa_old, sa_new = {
 		.sa_handler = fault_sigbus_handler,
 	};
-	size_t map_size = total_size * 4;
-	const char val = 0xaa;
-	char *mem;
-	size_t i;
+	void *mem;
 
-	mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
+	mem = test_mmap_common(fd, size);
 
 	sigaction(SIGBUS, &sa_new, &sa_old);
 	if (sigsetjmp(jmpbuf, 1) == 0) {
-		memset(mem, 0xaa, map_size);
+		memset(mem, 0xaa, size);
 		TEST_ASSERT(false, "memset() should have triggered SIGBUS.");
 	}
 	sigaction(SIGBUS, &sa_old, NULL);
 
+	return mem;
+}
+
+static void test_fault_overflow(int fd, size_t total_size)
+{
+	size_t map_size = total_size * 4;
+	const char val = 0xaa;
+	char *mem;
+	size_t i;
+
+	mem = test_fault_sigbus(fd, map_size);
+
 	for (i = 0; i < total_size; i++)
 		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
 
 	kvm_munmap(mem, map_size);
 }
 
+static void test_fault_private(int fd, size_t total_size)
+{
+	void *mem = test_fault_sigbus(fd, total_size);
+
+	kvm_munmap(mem, total_size);
+}
+
 static void test_mmap_not_supported(int fd, size_t total_size)
 {
 	char *mem;
@@ -274,9 +299,12 @@ static void __test_guest_memfd(struct kvm_vm *vm, uint64_t flags)
 
 	gmem_test(file_read_write, vm, flags);
 
-	if (flags & GUEST_MEMFD_FLAG_MMAP) {
+	if (flags & GUEST_MEMFD_FLAG_MMAP &&
+	    flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
 		gmem_test(mmap_supported, vm, flags);
 		gmem_test(fault_overflow, vm, flags);
+	} else if (flags & GUEST_MEMFD_FLAG_MMAP) {
+		gmem_test(fault_private, vm, flags);
 	} else {
 		gmem_test(mmap_not_supported, vm, flags);
 	}
@@ -294,9 +322,11 @@ static void test_guest_memfd(unsigned long vm_type)
 
 	__test_guest_memfd(vm, 0);
 
-	if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP))
+	if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP)) {
+		__test_guest_memfd(vm, GUEST_MEMFD_FLAG_MMAP);
 		__test_guest_memfd(vm, GUEST_MEMFD_FLAG_MMAP |
 				       GUEST_MEMFD_FLAG_DEFAULT_SHARED);
+	}
 
 	kvm_vm_free(vm);
 }
-- 
2.51.0.536.g15c5d4f767-goog
Re: [PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails
Posted by Ackerley Tng 2 days, 7 hours ago 
Sean Christopherson <seanjc@google.com> writes:

> Add a guest_memfd testcase to verify that faulting in private memory gets
> a SIGBUS.  For now, test only the case where memory is private by default
> since KVM doesn't yet support in-place conversion.
>
> Cc: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  .../testing/selftests/kvm/guest_memfd_test.c  | 62 ++++++++++++++-----
>  1 file changed, 46 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 5dd40b77dc07..b5a631aca933 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> @@ -40,17 +40,26 @@ static void test_file_read_write(int fd, size_t total_size)
>  		    "pwrite on a guest_mem fd should fail");
>  }
>  

I feel that the tests should be grouped by concepts being tested

+ test_cow_not_supported()
    + mmap() should fail
+ test_mmap_supported()
    + kvm_mmap()
    + regular, successful accesses to offsets within the size of the fd
    + kvm_munmap()
+ test_fault_overflow()
    + kvm_mmap()
    + a helper (perhaps "assert_fault_sigbus(char *mem)"?) that purely
      tries to access beyond the size of the fd and catches SIGBUS
    + regular, successful accesses to offsets within the size of the fd
    + kvm_munmap()
+ test_fault_private()
    + kvm_mmap()
    + a helper (perhaps "assert_fault_sigbus(char *mem)"?) that purely
      tries to access within the size of the fd and catches SIGBUS
    + kvm_munmap()

I think some code duplication in tests is okay if it makes the test flow
more obvious.

> -static void test_mmap_supported(int fd, size_t total_size)
> +static void *test_mmap_common(int fd, size_t size)
>  {
> -	const char val = 0xaa;
> -	char *mem;
> -	size_t i;
> -	int ret;
> +	void *mem;
>  
> -	mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> +	mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
>  	TEST_ASSERT(mem == MAP_FAILED, "Copy-on-write not allowed by guest_memfd.");
>

When grouped this way, test_mmap_common() tests that MAP_PRIVATE or COW
is not allowed twice, once in test_mmap_supported() and once in
test_fault_sigbus(). Is that intentional?

> -	mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> +	mem = kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> +
> +	return mem;

I feel that returning (and using) the userspace address from a test
(test_mmap_common()) is a little hard to follow.

> +}
> +
> +static void test_mmap_supported(int fd, size_t total_size)
> +{
> +	const char val = 0xaa;
> +	char *mem;
> +	size_t i;
> +	int ret;
> +
> +	mem = test_mmap_common(fd, total_size);
>  
>  	memset(mem, val, total_size);
>  	for (i = 0; i < total_size; i++)
> @@ -78,31 +87,47 @@ void fault_sigbus_handler(int signum)
>  	siglongjmp(jmpbuf, 1);
>  }
>  
> -static void test_fault_overflow(int fd, size_t total_size)
> +static void *test_fault_sigbus(int fd, size_t size)
>  {
>  	struct sigaction sa_old, sa_new = {
>  		.sa_handler = fault_sigbus_handler,
>  	};
> -	size_t map_size = total_size * 4;
> -	const char val = 0xaa;
> -	char *mem;
> -	size_t i;
> +	void *mem;
>  
> -	mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> +	mem = test_mmap_common(fd, size);
>  
>  	sigaction(SIGBUS, &sa_new, &sa_old);
>  	if (sigsetjmp(jmpbuf, 1) == 0) {
> -		memset(mem, 0xaa, map_size);
> +		memset(mem, 0xaa, size);
>  		TEST_ASSERT(false, "memset() should have triggered SIGBUS.");
>  	}
>  	sigaction(SIGBUS, &sa_old, NULL);
>  
> +	return mem;

I think returning the userspace address from a test is a little hard to
follow. This one feels even more unexpected because a valid address is
being returned (and used) from a test that has sigbus in its name.

> +}
> +
> +static void test_fault_overflow(int fd, size_t total_size)
> +{
> +	size_t map_size = total_size * 4;
> +	const char val = 0xaa;
> +	char *mem;
> +	size_t i;
> +
> +	mem = test_fault_sigbus(fd, map_size);
> +
>  	for (i = 0; i < total_size; i++)
>  		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
>  
>  	kvm_munmap(mem, map_size);
>  }
>  
> +static void test_fault_private(int fd, size_t total_size)
> +{
> +	void *mem = test_fault_sigbus(fd, total_size);
> +
> +	kvm_munmap(mem, total_size);
> +}
> +

Testing that faults fail when GUEST_MEMFD_FLAG_DEFAULT_SHARED is not set
is a good idea. Perhaps it could be even clearer if further split up:

+ test_mmap_supported()
    + kvm_mmap()
    + kvm_munmap()
+ test_mmap_supported_fault_supported()
    + kvm_mmap()
    + successful accesses to offsets within the size of the fd
    + kvm_munmap()
+ test_mmap_supported_fault_sigbus()
    + kvm_mmap()
    + expect SIGBUS from accesses to offsets within the size of the fd
    + kvm_munmap()

>  static void test_mmap_not_supported(int fd, size_t total_size)
>  {
>  	char *mem;
> @@ -274,9 +299,12 @@ static void __test_guest_memfd(struct kvm_vm *vm, uint64_t flags)
>  
>  	gmem_test(file_read_write, vm, flags);
>  
> -	if (flags & GUEST_MEMFD_FLAG_MMAP) {
> +	if (flags & GUEST_MEMFD_FLAG_MMAP &&
> +	    flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
>  		gmem_test(mmap_supported, vm, flags);
>  		gmem_test(fault_overflow, vm, flags);
> +	} else if (flags & GUEST_MEMFD_FLAG_MMAP) {
> +		gmem_test(fault_private, vm, flags);

test_fault_private() makes me think the test is testing for private
faults, but there's nothing private about this fault, and the fault
doesn't even come from the guest.

>  	} else {
>  		gmem_test(mmap_not_supported, vm, flags);
>  	}

If split up as described above, this could be

	if (flags & GUEST_MEMFD_FLAG_MMAP &&
	    flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
		gmem_test(mmap_supported_fault_supported, vm, flags);
		gmem_test(fault_overflow, vm, flags);
	} else if (flags & GUEST_MEMFD_FLAG_MMAP) {
		gmem_test(mmap_supported_fault_sigbus, vm, flags);
	} else {
		gmem_test(mmap_not_supported, vm, flags);
	}

> @@ -294,9 +322,11 @@ static void test_guest_memfd(unsigned long vm_type)
>  
>  	__test_guest_memfd(vm, 0);
>  
> -	if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP))
> +	if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP)) {
> +		__test_guest_memfd(vm, GUEST_MEMFD_FLAG_MMAP);
>  		__test_guest_memfd(vm, GUEST_MEMFD_FLAG_MMAP |
>  				       GUEST_MEMFD_FLAG_DEFAULT_SHARED);
> +	}
>  
>  	kvm_vm_free(vm);
>  }

I could send a revision, if you agree/prefer!

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

> -- 
> 2.51.0.536.g15c5d4f767-goog
Re: [PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails
Posted by Sean Christopherson 2 days, 4 hours ago 
On Mon, Sep 29, 2025, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
> > Add a guest_memfd testcase to verify that faulting in private memory gets
> > a SIGBUS.  For now, test only the case where memory is private by default
> > since KVM doesn't yet support in-place conversion.
> >
> > Cc: Ackerley Tng <ackerleytng@google.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  .../testing/selftests/kvm/guest_memfd_test.c  | 62 ++++++++++++++-----
> >  1 file changed, 46 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> > index 5dd40b77dc07..b5a631aca933 100644
> > --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> > +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> > @@ -40,17 +40,26 @@ static void test_file_read_write(int fd, size_t total_size)
> >  		    "pwrite on a guest_mem fd should fail");
> >  }
> >  
> 
> I feel that the tests should be grouped by concepts being tested
> 
> + test_cow_not_supported()
>     + mmap() should fail
> + test_mmap_supported()
>     + kvm_mmap()
>     + regular, successful accesses to offsets within the size of the fd
>     + kvm_munmap()
> + test_fault_overflow()
>     + kvm_mmap()
>     + a helper (perhaps "assert_fault_sigbus(char *mem)"?) that purely
>       tries to access beyond the size of the fd and catches SIGBUS
>     + regular, successful accesses to offsets within the size of the fd
>     + kvm_munmap()
> + test_fault_private()
>     + kvm_mmap()
>     + a helper (perhaps "assert_fault_sigbus(char *mem)"?) that purely
>       tries to access within the size of the fd and catches SIGBUS
>     + kvm_munmap()
> 
> I think some code duplication in tests is okay if it makes the test flow
> more obvious.

Yeah, depends on what is being duplicated, and how much.

> > -static void test_mmap_supported(int fd, size_t total_size)
> > +static void *test_mmap_common(int fd, size_t size)
> >  {
> > -	const char val = 0xaa;
> > -	char *mem;
> > -	size_t i;
> > -	int ret;
> > +	void *mem;
> >  
> > -	mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> > +	mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> >  	TEST_ASSERT(mem == MAP_FAILED, "Copy-on-write not allowed by guest_memfd.");
> >
> 
> When grouped this way, test_mmap_common() tests that MAP_PRIVATE or COW
> is not allowed twice, once in test_mmap_supported() and once in
> test_fault_sigbus(). Is that intentional?

Hmm, no?  I suspect I just lost track of what was being tested.

> > -	mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> > +	mem = kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> > +
> > +	return mem;
> 
> I feel that returning (and using) the userspace address from a test
> (test_mmap_common()) is a little hard to follow.

Agreed.  Should be easy enough to eliminate this helper.

> > -static void test_fault_overflow(int fd, size_t total_size)
> > +static void *test_fault_sigbus(int fd, size_t size)
> >  {
> >  	struct sigaction sa_old, sa_new = {
> >  		.sa_handler = fault_sigbus_handler,
> >  	};
> > -	size_t map_size = total_size * 4;
> > -	const char val = 0xaa;
> > -	char *mem;
> > -	size_t i;
> > +	void *mem;
> >  
> > -	mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> > +	mem = test_mmap_common(fd, size);
> >  
> >  	sigaction(SIGBUS, &sa_new, &sa_old);
> >  	if (sigsetjmp(jmpbuf, 1) == 0) {
> > -		memset(mem, 0xaa, map_size);
> > +		memset(mem, 0xaa, size);
> >  		TEST_ASSERT(false, "memset() should have triggered SIGBUS.");
> >  	}
> >  	sigaction(SIGBUS, &sa_old, NULL);
> >  
> > +	return mem;
> 
> I think returning the userspace address from a test is a little hard to
> follow. This one feels even more unexpected because a valid address is
> being returned (and used) from a test that has sigbus in its name.

Yeah, and it's fugly all around.  If we pass in the "accessible" size, then we
can reduce the amount of copy+paste, eliminate the weird return and split mmap()
versus munmap(), and get bonus coverage that reads SIGBUS as well.

How's this look?

static void test_fault_sigbus(int fd, size_t accessible_size, size_t mmap_size)
{
	struct sigaction sa_old, sa_new = {
		.sa_handler = fault_sigbus_handler,
	};
	const uint8_t val = 0xaa;
	uint8_t *mem;
	size_t i;

	mem = kvm_mmap(mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);

	sigaction(SIGBUS, &sa_new, &sa_old);
	if (sigsetjmp(jmpbuf, 1) == 0) {
		memset(mem, val, mmap_size);
		TEST_FAIL("memset() should have triggered SIGBUS");
	}
	if (sigsetjmp(jmpbuf, 1) == 0) {
		(void)READ_ONCE(mem[accessible_size]);
		TEST_FAIL("load at first unaccessible byte should have triggered SIGBUS");
	}
	sigaction(SIGBUS, &sa_old, NULL);

	for (i = 0; i < accessible_size; i++)
		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);

	kvm_munmap(mem, mmap_size);
}

static void test_fault_overflow(int fd, size_t total_size)
{
	test_fault_sigbus(fd, total_size, total_size * 4);
}

static void test_fault_private(int fd, size_t total_size)
{
	test_fault_sigbus(fd, 0, total_size);
}

> > +static void test_fault_private(int fd, size_t total_size)
> > +{
> > +	void *mem = test_fault_sigbus(fd, total_size);
> > +
> > +	kvm_munmap(mem, total_size);
> > +}
> > +
> 
> Testing that faults fail when GUEST_MEMFD_FLAG_DEFAULT_SHARED is not set
> is a good idea. Perhaps it could be even clearer if further split up:
> 
> + test_mmap_supported()
>     + kvm_mmap()
>     + kvm_munmap()
> + test_mmap_supported_fault_supported()
>     + kvm_mmap()
>     + successful accesses to offsets within the size of the fd
>     + kvm_munmap()
> + test_mmap_supported_fault_sigbus()
>     + kvm_mmap()
>     + expect SIGBUS from accesses to offsets within the size of the fd
>     + kvm_munmap()
> 
> >  static void test_mmap_not_supported(int fd, size_t total_size)
> >  {
> >  	char *mem;
> > @@ -274,9 +299,12 @@ static void __test_guest_memfd(struct kvm_vm *vm, uint64_t flags)
> >  
> >  	gmem_test(file_read_write, vm, flags);
> >  
> > -	if (flags & GUEST_MEMFD_FLAG_MMAP) {
> > +	if (flags & GUEST_MEMFD_FLAG_MMAP &&
> > +	    flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
> >  		gmem_test(mmap_supported, vm, flags);
> >  		gmem_test(fault_overflow, vm, flags);
> > +	} else if (flags & GUEST_MEMFD_FLAG_MMAP) {
> > +		gmem_test(fault_private, vm, flags);
> 
> test_fault_private() makes me think the test is testing for private
> faults, but there's nothing private about this fault,

It's a user fault on private memory, not sure how else to describe that :-)
The CoCo shared vs. private and MAP_{SHARED,PRIVATE} collision is unfortunate,
but I think we should prioritize standardizing on CoCo shared vs. private since
that is what KVM will care about 99.9% of the time, i.e. in literally everything
except kvm_gmem_mmap().

> and the fault doesn't even come from the guest.

Sure, but I don't see what that has to do with anything, e.g. fault_overflow()
isn't a fault from the guest either.

> >  	} else {
> >  		gmem_test(mmap_not_supported, vm, flags);
> >  	}
> 
> If split up as described above, this could be
> 
> 	if (flags & GUEST_MEMFD_FLAG_MMAP &&
> 	    flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
> 		gmem_test(mmap_supported_fault_supported, vm, flags);
> 		gmem_test(fault_overflow, vm, flags);
> 	} else if (flags & GUEST_MEMFD_FLAG_MMAP) {
> 		gmem_test(mmap_supported_fault_sigbus, vm, flags);

I find these unintuitive, e.g. is this one "mmap() supported, test fault sigbus",
or is it "mmap(), test supported fault sigbus".  I also don't like that some of
the test names describe the _result_ (SIBGUS), where as others describe _what_
is being tested.

In general, I don't like test names that describe the result, because IMO what
is being tested is far more interesting.  E.g. from a test coverage persective,
I don't care if attempting to fault in (CoCO) private memory gets SIGBUS versus
SIGSEGV, but I most definitely care that we have test coverage for the "what".

Looking at everything, I think the only that doesn't fit well is the CoW
scenario.  What if we extract that to its own helper?  That would eliminate the
ugly test_mmap_common(), 

So my vote would be to keep things largely the same:

	if (flags & GUEST_MEMFD_FLAG_MMAP &&
	    flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
		gmem_test(mmap_supported, vm, flags);
		gmem_test(mmap_cow, vm, flags);
		gmem_test(fault_overflow, vm, flags);
		gmem_test(mbind, vm, flags);
		gmem_test(numa_allocation, vm, flags);
	} else if (flags & GUEST_MEMFD_FLAG_MMAP) {
		gmem_test(fault_private, vm, flags);
	} else {
		gmem_test(mmap_not_supported, vm, flags);
	}
Re: [PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails
Posted by Ackerley Tng 1 day, 14 hours ago 
Sean Christopherson <seanjc@google.com> writes:

> On Mon, Sep 29, 2025, Ackerley Tng wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>> 
>> 
>> [...snip...]
>> 
>> > -static void test_fault_overflow(int fd, size_t total_size)
>> > +static void *test_fault_sigbus(int fd, size_t size)
>> >  {
>> >  	struct sigaction sa_old, sa_new = {
>> >  		.sa_handler = fault_sigbus_handler,
>> >  	};
>> > -	size_t map_size = total_size * 4;
>> > -	const char val = 0xaa;
>> > -	char *mem;
>> > -	size_t i;
>> > +	void *mem;
>> >  
>> > -	mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>> > +	mem = test_mmap_common(fd, size);
>> >  
>> >  	sigaction(SIGBUS, &sa_new, &sa_old);
>> >  	if (sigsetjmp(jmpbuf, 1) == 0) {
>> > -		memset(mem, 0xaa, map_size);
>> > +		memset(mem, 0xaa, size);
>> >  		TEST_ASSERT(false, "memset() should have triggered SIGBUS.");
>> >  	}
>> >  	sigaction(SIGBUS, &sa_old, NULL);
>> >  
>> > +	return mem;
>> 
>> I think returning the userspace address from a test is a little hard to
>> follow. This one feels even more unexpected because a valid address is
>> being returned (and used) from a test that has sigbus in its name.
>
> Yeah, and it's fugly all around.  If we pass in the "accessible" size, then we
> can reduce the amount of copy+paste, eliminate the weird return and split mmap()
> versus munmap(), and get bonus coverage that reads SIGBUS as well.
>
> How's this look?
>
> static void test_fault_sigbus(int fd, size_t accessible_size, size_t mmap_size)
> {
> 	struct sigaction sa_old, sa_new = {
> 		.sa_handler = fault_sigbus_handler,
> 	};
> 	const uint8_t val = 0xaa;
> 	uint8_t *mem;
> 	size_t i;
>
> 	mem = kvm_mmap(mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>
> 	sigaction(SIGBUS, &sa_new, &sa_old);
> 	if (sigsetjmp(jmpbuf, 1) == 0) {
> 		memset(mem, val, mmap_size);
> 		TEST_FAIL("memset() should have triggered SIGBUS");
> 	}
> 	if (sigsetjmp(jmpbuf, 1) == 0) {
> 		(void)READ_ONCE(mem[accessible_size]);
> 		TEST_FAIL("load at first unaccessible byte should have triggered SIGBUS");
> 	}
> 	sigaction(SIGBUS, &sa_old, NULL);
>
> 	for (i = 0; i < accessible_size; i++)
> 		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
>
> 	kvm_munmap(mem, mmap_size);
> }
>
> static void test_fault_overflow(int fd, size_t total_size)
> {
> 	test_fault_sigbus(fd, total_size, total_size * 4);
> }
>

Is it intentional that the same SIGBUS on offset mem + total_size is
triggered twice? The memset would have worked fine until offset mem +
total_size, which is the same SIGBUS case as mem[accessible_size]. Or
was it meant to test that both read and write trigger SIGBUS?

> static void test_fault_private(int fd, size_t total_size)
> {
> 	test_fault_sigbus(fd, 0, total_size);
> }
>

I would prefer more unrolling to avoid mental hoops within test code,
perhaps like (not compile tested):

static void assert_host_fault_sigbus(uint8_t *mem) 
{
 	struct sigaction sa_old, sa_new = {
 		.sa_handler = fault_sigbus_handler,
 	};

 	sigaction(SIGBUS, &sa_new, &sa_old);
 	if (sigsetjmp(jmpbuf, 1) == 0) {
 		(void)READ_ONCE(*mem);
 		TEST_FAIL("Reading %p should have triggered SIGBUS", mem);
 	}
        sigaction(SIGBUS, &sa_old, NULL);
}

static void test_fault_overflow(int fd, size_t total_size)
{
	uint8_t *mem = kvm_mmap(total_size * 2, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
        int i;

 	for (i = 0; i < total_size; i++)
 		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);

        assert_host_fault_sigbus(mem + total_size);

        kvm_munmap(mem, mmap_size);
}

static void test_fault_private(int fd, size_t total_size)
{
	uint8_t *mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
        int i;

        assert_host_fault_sigbus(mem);

        kvm_munmap(mem, mmap_size);
}

assert_host_fault_sigbus() can then be flexibly reused for conversion
tests (coming up) at various offsets from the mmap()-ed addresses.

At some point, sigaction, sigsetjmp, etc could perhaps even be further
wrapped. For testing memory_failure() for guest_memfd we will want to
check for SIGBUS on memory failure injection instead of on host fault.

Would be nice if it looked like this (maybe not in this patch series):

+ TEST_ASSERT_WILL_SIGBUS(READ_ONCE(mem[i]))
+ TEST_ASSERT_WILL_SIGBUS(WRITE_ONCE(mem[i]))
+ TEST_ASSERT_WILL_SIGBUS(madvise(MADV_HWPOISON))

>> > +static void test_fault_private(int fd, size_t total_size)
>> > +{
>> > +	void *mem = test_fault_sigbus(fd, total_size);
>> > +
>> > +	kvm_munmap(mem, total_size);
>> > +}
>> > +
>> 
>> Testing that faults fail when GUEST_MEMFD_FLAG_DEFAULT_SHARED is not set
>> is a good idea. Perhaps it could be even clearer if further split up:
>> 
>> + test_mmap_supported()
>>     + kvm_mmap()
>>     + kvm_munmap()
>> + test_mmap_supported_fault_supported()
>>     + kvm_mmap()
>>     + successful accesses to offsets within the size of the fd
>>     + kvm_munmap()
>> + test_mmap_supported_fault_sigbus()
>>     + kvm_mmap()
>>     + expect SIGBUS from accesses to offsets within the size of the fd
>>     + kvm_munmap()
>> 
>> >  static void test_mmap_not_supported(int fd, size_t total_size)
>> >  {
>> >  	char *mem;
>> > @@ -274,9 +299,12 @@ static void __test_guest_memfd(struct kvm_vm *vm, uint64_t flags)
>> >  
>> >  	gmem_test(file_read_write, vm, flags);
>> >  
>> > -	if (flags & GUEST_MEMFD_FLAG_MMAP) {
>> > +	if (flags & GUEST_MEMFD_FLAG_MMAP &&
>> > +	    flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
>> >  		gmem_test(mmap_supported, vm, flags);
>> >  		gmem_test(fault_overflow, vm, flags);
>> > +	} else if (flags & GUEST_MEMFD_FLAG_MMAP) {
>> > +		gmem_test(fault_private, vm, flags);
>> 
>> test_fault_private() makes me think the test is testing for private
>> faults, but there's nothing private about this fault,
>
> It's a user fault on private memory, not sure how else to describe that :-)
> The CoCo shared vs. private and MAP_{SHARED,PRIVATE} collision is unfortunate,
> but I think we should prioritize standardizing on CoCo shared vs. private since
> that is what KVM will care about 99.9% of the time, i.e. in literally everything
> except kvm_gmem_mmap().
>
>> and the fault doesn't even come from the guest.
>
> Sure, but I don't see what that has to do with anything, e.g. fault_overflow()
> isn't a fault from the guest either.
>

Maybe it's the frame of mind I'm working in (conversions), where all
private faults must be from the guest or from KVM. Feel free to ignore this.

>> >  	} else {
>> >  		gmem_test(mmap_not_supported, vm, flags);
>> >  	}
>> 
>> If split up as described above, this could be
>> 
>> 	if (flags & GUEST_MEMFD_FLAG_MMAP &&
>> 	    flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
>> 		gmem_test(mmap_supported_fault_supported, vm, flags);
>> 		gmem_test(fault_overflow, vm, flags);
>> 	} else if (flags & GUEST_MEMFD_FLAG_MMAP) {
>> 		gmem_test(mmap_supported_fault_sigbus, vm, flags);
>
> I find these unintuitive, e.g. is this one "mmap() supported, test fault sigbus",
> or is it "mmap(), test supported fault sigbus".  I also don't like that some of
> the test names describe the _result_ (SIBGUS), where as others describe _what_
> is being tested.
>

I think of the result (SIGBUS) as part of what's being tested. So
test_supported_fault_sigbus() is testing that mmap is supported, and
faulting will result in a SIGBUS.

> In general, I don't like test names that describe the result, because IMO what
> is being tested is far more interesting.  E.g. from a test coverage persective,
> I don't care if attempting to fault in (CoCO) private memory gets SIGBUS versus
> SIGSEGV, but I most definitely care that we have test coverage for the "what".
>

The SIGBUS is part of the contract with userspace and that's also part
of what's being tested IMO.

That said, I agree we don't need sigbus in the name, I guess I just
meant that there are a few layers to test here and I couldn't find a
better name:

1. mmap() succeeds to start with
2. mmap() succeeds, and faulting also succeeds
    + mmap() works, and faulting does not succeed because memory is not
      intended to be accessible to the host
3. mmap() succeed, and faulting also succeeds, but only within the size of
   guest_memfd

> Looking at everything, I think the only that doesn't fit well is the CoW
> scenario.  What if we extract that to its own helper?  That would eliminate the
> ugly test_mmap_common(), 
>

Extracting the CoW scenario is good, thanks!

> So my vote would be to keep things largely the same:
>
> 	if (flags & GUEST_MEMFD_FLAG_MMAP &&
> 	    flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
> 		gmem_test(mmap_supported, vm, flags);
> 		gmem_test(mmap_cow, vm, flags);
> 		gmem_test(fault_overflow, vm, flags);
> 		gmem_test(mbind, vm, flags);
> 		gmem_test(numa_allocation, vm, flags);
> 	} else if (flags & GUEST_MEMFD_FLAG_MMAP) {
> 		gmem_test(fault_private, vm, flags);
> 	} else {
> 		gmem_test(mmap_not_supported, vm, flags);
> 	}
Re: [PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails
Posted by Sean Christopherson 1 day, 7 hours ago 
On Tue, Sep 30, 2025, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
> > How's this look?
> >
> > static void test_fault_sigbus(int fd, size_t accessible_size, size_t mmap_size)
> > {
> > 	struct sigaction sa_old, sa_new = {
> > 		.sa_handler = fault_sigbus_handler,
> > 	};
> > 	const uint8_t val = 0xaa;
> > 	uint8_t *mem;
> > 	size_t i;
> >
> > 	mem = kvm_mmap(mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> >
> > 	sigaction(SIGBUS, &sa_new, &sa_old);
> > 	if (sigsetjmp(jmpbuf, 1) == 0) {
> > 		memset(mem, val, mmap_size);
> > 		TEST_FAIL("memset() should have triggered SIGBUS");
> > 	}
> > 	if (sigsetjmp(jmpbuf, 1) == 0) {
> > 		(void)READ_ONCE(mem[accessible_size]);
> > 		TEST_FAIL("load at first unaccessible byte should have triggered SIGBUS");
> > 	}
> > 	sigaction(SIGBUS, &sa_old, NULL);
> >
> > 	for (i = 0; i < accessible_size; i++)
> > 		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
> >
> > 	kvm_munmap(mem, mmap_size);
> > }
> >
> > static void test_fault_overflow(int fd, size_t total_size)
> > {
> > 	test_fault_sigbus(fd, total_size, total_size * 4);
> > }
> >
> 
> Is it intentional that the same SIGBUS on offset mem + total_size is
> triggered twice? The memset would have worked fine until offset mem +
> total_size, which is the same SIGBUS case as mem[accessible_size]. Or
> was it meant to test that both read and write trigger SIGBUS?

The latter (test both read and write).  I plan on adding this in a separate
commit, i.e. it should be obvious in the actual patches.

> > static void test_fault_private(int fd, size_t total_size)
> > {
> > 	test_fault_sigbus(fd, 0, total_size);
> > }
> >
> 
> I would prefer more unrolling to avoid mental hoops within test code,
> perhaps like (not compile tested):
> 
> static void assert_host_fault_sigbus(uint8_t *mem) 
> {
>  	struct sigaction sa_old, sa_new = {
>  		.sa_handler = fault_sigbus_handler,
>  	};
> 
>  	sigaction(SIGBUS, &sa_new, &sa_old);
>  	if (sigsetjmp(jmpbuf, 1) == 0) {
>  		(void)READ_ONCE(*mem);
>  		TEST_FAIL("Reading %p should have triggered SIGBUS", mem);
>  	}
>         sigaction(SIGBUS, &sa_old, NULL);
> }
> 
> static void test_fault_overflow(int fd, size_t total_size)
> {
> 	uint8_t *mem = kvm_mmap(total_size * 2, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>         int i;
> 
>  	for (i = 0; i < total_size; i++)
>  		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
> 
>         assert_host_fault_sigbus(mem + total_size);
> 
>         kvm_munmap(mem, mmap_size);
> }
> 
> static void test_fault_private(int fd, size_t total_size)
> {
> 	uint8_t *mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>         int i;
> 
>         assert_host_fault_sigbus(mem);
> 
>         kvm_munmap(mem, mmap_size);
> }

Why?  That loses coverage for read to private memory getting SIBUGS.  I genuinely
don't understand the desire to copy+paste uninteresting code.

> assert_host_fault_sigbus() can then be flexibly reused for conversion

assert_host_fault_sigbus() is a misleading name in the sense that it suggests
that the _only_ thing the helper does is assert that a SIGBUS occurred.  It's
not at all obvious that there's a write to "mem" in there.

> tests (coming up) at various offsets from the mmap()-ed addresses.
> 
> At some point, sigaction, sigsetjmp, etc could perhaps even be further
> wrapped. For testing memory_failure() for guest_memfd we will want to
> check for SIGBUS on memory failure injection instead of on host fault.
> 
> Would be nice if it looked like this (maybe not in this patch series):
> 
> + TEST_ASSERT_WILL_SIGBUS(READ_ONCE(mem[i]))
> + TEST_ASSERT_WILL_SIGBUS(WRITE_ONCE(mem[i]))
> + TEST_ASSERT_WILL_SIGBUS(madvise(MADV_HWPOISON))

Ooh, me likey.  Definitely can do it now.  Using a macro means we can print out
the actual action that didn't generate a SIGUBS, e.g. hacking the test to read
byte 0 generates:

	'(void)READ_ONCE(mem[0])' should have triggered SIGBUS

Hmm, how about TEST_EXPECT_SIGBUS?  TEST_ASSERT_xxx() typically asserts on a
value, i.e. on the result of a previous action.  And s/WILL/EXPECT to make it
clear that the action is expected to SIGBUS _now_.

And if we use a descriptive global variable, we can extract the macro to e.g.
test_util.h or kvm_util.h (not sure we want to do that right away; probably best
left to the future).

static sigjmp_buf expect_sigbus_jmpbuf;
void fault_sigbus_handler(int signum)
{
	siglongjmp(expect_sigbus_jmpbuf, 1);
}

#define TEST_EXPECT_SIGBUS(action)						\
do {										\
	struct sigaction sa_old, sa_new = {					\
		.sa_handler = fault_sigbus_handler,				\
	};									\
										\
	sigaction(SIGBUS, &sa_new, &sa_old);					\
	if (sigsetjmp(expect_sigbus_jmpbuf, 1) == 0) {				\
		action;								\
		TEST_FAIL("'%s' should have triggered SIGBUS", #action);	\
	}									\
	sigaction(SIGBUS, &sa_old, NULL);					\
} while (0)

static void test_fault_sigbus(int fd, size_t accessible_size, size_t map_size)
{
	const char val = 0xaa;
	char *mem;
	size_t i;

	mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);

	TEST_EXPECT_SIGBUS(memset(mem, val, map_size));
	TEST_EXPECT_SIGBUS((void)READ_ONCE(mem[accessible_size]));

	for (i = 0; i < accessible_size; i++)
		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);

	kvm_munmap(mem, map_size);
}

> >> If split up as described above, this could be
> >> 
> >> 	if (flags & GUEST_MEMFD_FLAG_MMAP &&
> >> 	    flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
> >> 		gmem_test(mmap_supported_fault_supported, vm, flags);
> >> 		gmem_test(fault_overflow, vm, flags);
> >> 	} else if (flags & GUEST_MEMFD_FLAG_MMAP) {
> >> 		gmem_test(mmap_supported_fault_sigbus, vm, flags);
> >
> > I find these unintuitive, e.g. is this one "mmap() supported, test fault sigbus",
> > or is it "mmap(), test supported fault sigbus".  I also don't like that some of
> > the test names describe the _result_ (SIBGUS), where as others describe _what_
> > is being tested.
> >
> 
> I think of the result (SIGBUS) as part of what's being tested. So
> test_supported_fault_sigbus() is testing that mmap is supported, and
> faulting will result in a SIGBUS.

For an utility helper, e.g. test_fault_sigbus(), or test_write_sigbus(), that's
a-ok.  But it doesn't work for the top-level test functions because trying to
follow that pattern effectively prevents bundling multiple individual testcases,
e.g. test_fallocate() becomes what?  And test_invalid_punch_hole_einval() is
quite obnoxious.

> > In general, I don't like test names that describe the result, because IMO what
> > is being tested is far more interesting.  E.g. from a test coverage persective,
> > I don't care if attempting to fault in (CoCO) private memory gets SIGBUS versus
> > SIGSEGV, but I most definitely care that we have test coverage for the "what".
> >
> 
> The SIGBUS is part of the contract with userspace and that's also part
> of what's being tested IMO.

I don't disagree, but IMO bleeding those details into the top-level functions
isn't necessary.  Random developer that comes along isn't going to care whether
KVM is supposed to SIGBUS or SIGSEGV unless there is a failure.  And as above,
doing so either singles out sigbus or necessitates truly funky names.
Re: [PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails
Posted by Ackerley Tng 12 hours ago 
Sean Christopherson <seanjc@google.com> writes:

> On Tue, Sep 30, 2025, Ackerley Tng wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>> 
>> [...snip...]
>> 
>> 
>> At some point, sigaction, sigsetjmp, etc could perhaps even be further
>> wrapped. For testing memory_failure() for guest_memfd we will want to
>> check for SIGBUS on memory failure injection instead of on host fault.
>> 
>> Would be nice if it looked like this (maybe not in this patch series):
>> 
>> + TEST_ASSERT_WILL_SIGBUS(READ_ONCE(mem[i]))
>> + TEST_ASSERT_WILL_SIGBUS(WRITE_ONCE(mem[i]))
>> + TEST_ASSERT_WILL_SIGBUS(madvise(MADV_HWPOISON))
>
> Ooh, me likey.  Definitely can do it now.  Using a macro means we can print out
> the actual action that didn't generate a SIGUBS, e.g. hacking the test to read
> byte 0 generates:
>
> 	'(void)READ_ONCE(mem[0])' should have triggered SIGBUS
>
> Hmm, how about TEST_EXPECT_SIGBUS?  TEST_ASSERT_xxx() typically asserts on a
> value, i.e. on the result of a previous action.  And s/WILL/EXPECT to make it
> clear that the action is expected to SIGBUS _now_.
>
> And if we use a descriptive global variable, we can extract the macro to e.g.
> test_util.h or kvm_util.h (not sure we want to do that right away; probably best
> left to the future).
>
> static sigjmp_buf expect_sigbus_jmpbuf;
> void fault_sigbus_handler(int signum)
> {
> 	siglongjmp(expect_sigbus_jmpbuf, 1);
> }
>
> #define TEST_EXPECT_SIGBUS(action)						\
> do {										\
> 	struct sigaction sa_old, sa_new = {					\
> 		.sa_handler = fault_sigbus_handler,				\
> 	};									\
> 										\
> 	sigaction(SIGBUS, &sa_new, &sa_old);					\
> 	if (sigsetjmp(expect_sigbus_jmpbuf, 1) == 0) {				\
> 		action;								\
> 		TEST_FAIL("'%s' should have triggered SIGBUS", #action);	\
> 	}									\
> 	sigaction(SIGBUS, &sa_old, NULL);					\
> } while (0)
>
> static void test_fault_sigbus(int fd, size_t accessible_size, size_t map_size)
> {
> 	const char val = 0xaa;
> 	char *mem;
> 	size_t i;
>
> 	mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>
> 	TEST_EXPECT_SIGBUS(memset(mem, val, map_size));
> 	TEST_EXPECT_SIGBUS((void)READ_ONCE(mem[accessible_size]));
>
> 	for (i = 0; i < accessible_size; i++)
> 		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
>
> 	kvm_munmap(mem, map_size);
> }
>

Awesome! Thanks!

And thanks for the explanations on the other suggestions.

>> 
>> [...snip...]
>>
Re: [PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails
Posted by Sean Christopherson 2 days, 4 hours ago 
On Mon, Sep 29, 2025, Sean Christopherson wrote:
> How's this look?
> 
> static void test_fault_sigbus(int fd, size_t accessible_size, size_t mmap_size)
> {
> 	struct sigaction sa_old, sa_new = {
> 		.sa_handler = fault_sigbus_handler,
> 	};
> 	const uint8_t val = 0xaa;
> 	uint8_t *mem;
> 	size_t i;
> 
> 	mem = kvm_mmap(mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> 
> 	sigaction(SIGBUS, &sa_new, &sa_old);
> 	if (sigsetjmp(jmpbuf, 1) == 0) {
> 		memset(mem, val, mmap_size);
> 		TEST_FAIL("memset() should have triggered SIGBUS");
> 	}
> 	if (sigsetjmp(jmpbuf, 1) == 0) {
> 		(void)READ_ONCE(mem[accessible_size]);
> 		TEST_FAIL("load at first unaccessible byte should have triggered SIGBUS");
> 	}
> 	sigaction(SIGBUS, &sa_old, NULL);
> 
> 	for (i = 0; i < accessible_size; i++)
> 		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
> 
> 	kvm_munmap(mem, mmap_size);
> }
> 
> static void test_fault_overflow(int fd, size_t total_size)
> {
> 	test_fault_sigbus(fd, total_size, total_size * 4);
> }
> 
> static void test_fault_private(int fd, size_t total_size)
> {
> 	test_fault_sigbus(fd, 0, total_size);
> }

And if I don't wantonly change variable names/types, the diff is much cleaner:

diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index 8ed08be72c43..8e375de2d7d8 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -83,12 +83,11 @@ void fault_sigbus_handler(int signum)
        siglongjmp(jmpbuf, 1);
 }
 
-static void test_fault_overflow(int fd, size_t total_size)
+static void test_fault_sigbus(int fd, size_t accessible_size, size_t map_size)
 {
        struct sigaction sa_old, sa_new = {
                .sa_handler = fault_sigbus_handler,
        };
-       size_t map_size = total_size * 4;
        const char val = 0xaa;
        char *mem;
        size_t i;
@@ -102,12 +101,22 @@ static void test_fault_overflow(int fd, size_t total_size)
        }
        sigaction(SIGBUS, &sa_old, NULL);
 
-       for (i = 0; i < total_size; i++)
+       for (i = 0; i < accessible_size; i++)
                TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
 
        kvm_munmap(mem, map_size);
 }
 
+static void test_fault_overflow(int fd, size_t total_size)
+{
+       test_fault_sigbus(fd, total_size, total_size * 4);
+}
+
+static void test_fault_private(int fd, size_t total_size)
+{
+       test_fault_sigbus(fd, 0, total_size);
+}
+
 static void test_mmap_not_supported(int fd, size_t total_size)
 {
        char *mem;
@@ -279,10 +288,13 @@ static void __test_guest_memfd(struct kvm_vm *vm, uint64_t flags)
 
        gmem_test(file_read_write, vm, flags);
 
-       if (flags & GUEST_MEMFD_FLAG_MMAP) {
+       if (flags & GUEST_MEMFD_FLAG_MMAP &&
+           flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
                gmem_test(mmap_supported, vm, flags);
                gmem_test(mmap_cow, vm, flags);
                gmem_test(fault_overflow, vm, flags);
+       } else if (flags & GUEST_MEMFD_FLAG_MMAP) {
+               gmem_test(fault_private, vm, flags);
        } else {
                gmem_test(mmap_not_supported, vm, flags);
        }
@@ -300,9 +312,11 @@ static void test_guest_memfd(unsigned long vm_type)
 
        __test_guest_memfd(vm, 0);
 
-       if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP))
+       if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP)) {
+               __test_guest_memfd(vm, GUEST_MEMFD_FLAG_MMAP);
                __test_guest_memfd(vm, GUEST_MEMFD_FLAG_MMAP |
                                       GUEST_MEMFD_FLAG_DEFAULT_SHARED);
+       }
 
        kvm_vm_free(vm);
 }
Re: [PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails
Posted by David Hildenbrand 2 days, 13 hours ago 
On 26.09.25 18:31, Sean Christopherson wrote:
> Add a guest_memfd testcase to verify that faulting in private memory gets
> a SIGBUS.  For now, test only the case where memory is private by default
> since KVM doesn't yet support in-place conversion.
> 
> Cc: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb
Re: [PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails
Posted by Fuad Tabba 2 days, 13 hours ago 
On Fri, 26 Sept 2025 at 17:31, Sean Christopherson <seanjc@google.com> wrote:
>
> Add a guest_memfd testcase to verify that faulting in private memory gets
> a SIGBUS.  For now, test only the case where memory is private by default
> since KVM doesn't yet support in-place conversion.
>
> Cc: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>


Reviewed-by: Fuad Tabba <tabba@google.com>
Tested-by: Fuad Tabba <tabba@google.com>

Cheers,
/fuad


> ---
>  .../testing/selftests/kvm/guest_memfd_test.c  | 62 ++++++++++++++-----
>  1 file changed, 46 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 5dd40b77dc07..b5a631aca933 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> @@ -40,17 +40,26 @@ static void test_file_read_write(int fd, size_t total_size)
>                     "pwrite on a guest_mem fd should fail");
>  }
>
> -static void test_mmap_supported(int fd, size_t total_size)
> +static void *test_mmap_common(int fd, size_t size)
>  {
> -       const char val = 0xaa;
> -       char *mem;
> -       size_t i;
> -       int ret;
> +       void *mem;
>
> -       mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> +       mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
>         TEST_ASSERT(mem == MAP_FAILED, "Copy-on-write not allowed by guest_memfd.");
>
> -       mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> +       mem = kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> +
> +       return mem;
> +}
> +
> +static void test_mmap_supported(int fd, size_t total_size)
> +{
> +       const char val = 0xaa;
> +       char *mem;
> +       size_t i;
> +       int ret;
> +
> +       mem = test_mmap_common(fd, total_size);
>
>         memset(mem, val, total_size);
>         for (i = 0; i < total_size; i++)
> @@ -78,31 +87,47 @@ void fault_sigbus_handler(int signum)
>         siglongjmp(jmpbuf, 1);
>  }
>
> -static void test_fault_overflow(int fd, size_t total_size)
> +static void *test_fault_sigbus(int fd, size_t size)
>  {
>         struct sigaction sa_old, sa_new = {
>                 .sa_handler = fault_sigbus_handler,
>         };
> -       size_t map_size = total_size * 4;
> -       const char val = 0xaa;
> -       char *mem;
> -       size_t i;
> +       void *mem;
>
> -       mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> +       mem = test_mmap_common(fd, size);
>
>         sigaction(SIGBUS, &sa_new, &sa_old);
>         if (sigsetjmp(jmpbuf, 1) == 0) {
> -               memset(mem, 0xaa, map_size);
> +               memset(mem, 0xaa, size);
>                 TEST_ASSERT(false, "memset() should have triggered SIGBUS.");
>         }
>         sigaction(SIGBUS, &sa_old, NULL);
>
> +       return mem;
> +}
> +
> +static void test_fault_overflow(int fd, size_t total_size)
> +{
> +       size_t map_size = total_size * 4;
> +       const char val = 0xaa;
> +       char *mem;
> +       size_t i;
> +
> +       mem = test_fault_sigbus(fd, map_size);
> +
>         for (i = 0; i < total_size; i++)
>                 TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
>
>         kvm_munmap(mem, map_size);
>  }
>
> +static void test_fault_private(int fd, size_t total_size)
> +{
> +       void *mem = test_fault_sigbus(fd, total_size);
> +
> +       kvm_munmap(mem, total_size);
> +}
> +
>  static void test_mmap_not_supported(int fd, size_t total_size)
>  {
>         char *mem;
> @@ -274,9 +299,12 @@ static void __test_guest_memfd(struct kvm_vm *vm, uint64_t flags)
>
>         gmem_test(file_read_write, vm, flags);
>
> -       if (flags & GUEST_MEMFD_FLAG_MMAP) {
> +       if (flags & GUEST_MEMFD_FLAG_MMAP &&
> +           flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED) {
>                 gmem_test(mmap_supported, vm, flags);
>                 gmem_test(fault_overflow, vm, flags);
> +       } else if (flags & GUEST_MEMFD_FLAG_MMAP) {
> +               gmem_test(fault_private, vm, flags);
>         } else {
>                 gmem_test(mmap_not_supported, vm, flags);
>         }
> @@ -294,9 +322,11 @@ static void test_guest_memfd(unsigned long vm_type)
>
>         __test_guest_memfd(vm, 0);
>
> -       if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP))
> +       if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP)) {
> +               __test_guest_memfd(vm, GUEST_MEMFD_FLAG_MMAP);
>                 __test_guest_memfd(vm, GUEST_MEMFD_FLAG_MMAP |
>                                        GUEST_MEMFD_FLAG_DEFAULT_SHARED);
> +       }
>
>         kvm_vm_free(vm);
>  }
> --
> 2.51.0.536.g15c5d4f767-goog
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.