Re: [PATCH v3 3/3] KVM: selftests: Add guest_memfd regression test signed offset+size bug

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The hardcoded guest_memfd offset literal is 68 bits long and only 4KB-aligned, causing the test to pass for the wrong reasons and failing to verify the actual signed offset+size bug.
--

commit 418cb333b0d34d015bff4f1526de77942a583c81
Author: Sean Christopherson <seanjc@google.com>

KVM: selftests: Add guest_memfd regression test signed offset+size bug

This commit adds a regression testcase to ensure KVM rejects an offset and
size that would result in a negative value when computed as a signed 64-bit
value.

> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 246bb408ecc0c..95a6ddfd80237 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c

[ ... ]

> @@ -345,6 +345,16 @@ static void test_invalid_punch_hole(int fd, size_t total_size)
>  	}
>  }
>  
> +static void test_invalid_binding(struct kvm_vm *vm, int fd, size_t size)
> +{
> +	int r;
> +
> +	r = __vm_set_user_memory_region2(vm, 0, KVM_MEM_GUEST_MEMFD, 0, size, 0,
> +					 fd, 0x7ffffffffffff000ull);

[Severity: Medium]
Is this hardcoded offset value correct?

The literal 0x7ffffffffffff000ull contains 17 hex digits (68 bits), which
will be truncated to 64 bits (0xfffffffffffff000ull) by the compiler. Because
this truncated value is very close to the maximum 64-bit unsigned integer,
adding the size to it will wrap around zero.

Will this trigger KVM's existing unsigned overflow check instead of the signed
offset plus size bug this test is meant to verify, causing the test to pass
for the wrong reason?

Also, since the trailing 000 makes the offset strictly 4KB-aligned, will
this fail immediately during the page alignment check on architectures with
larger page sizes (like 64KB on ARM64)?

Should the offset be dynamically calculated at runtime based on the page size
to ensure proper alignment and accurately test the signed boundary?

> +	TEST_ASSERT(r && errno == EINVAL,
> +		    "Memslot with out-of-range offset+size should fail");
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528021117.107984-1-seanjc@google.com?part=3