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

[Sean Christopherson <seanjc@google.com>]

On Sat, May 30, 2026, Ackerley Tng wrote:
> sashiko-bot@kernel.org writes:
> 
> >
> > [...snip...]
> >
> >> @@ -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.
> >
> 
> Sashiko miscounted the number of hex digits, it's actually 16.  That
> said, humans also have to count to understand what the test is
> doing... How about this, which I think highlights the signed overflow
> requirement on the selected offset?
> 
>  static void test_invalid_binding(struct kvm_vm *vm, int fd, size_t size)
>  {
> +       const u64 large_offset = align_down(INT64_MAX, getpagesize());

Eh, I don't see any need for a local variable and an extra assert.  The size of
the gmem file _must_ be >= page_size, otherwise creation would fail due to lack
of alignment.

This can+should use ALIGN_DOWN().  Actually, why does align_down() even exists?
Ugh, because KVM selftests predate commit 10a04ff09bcc ("tools: move alignment-related
macros to new <linux/align.h>") by many years.  I'll send a separate series to
drop align_{down,up}(), and hopefully align_ptr_up() as well (should need a cast
version since ALIGN() is a macro, i.e. doesn't do implicit type conversion).

v4 incoming...