Re: [PATCH v3 1/3] KVM: guest_memfd: Treat memslot binding offset+size as unsigned values

[Michael Roth]

On Wed, May 27, 2026 at 07:11:15PM -0700, Sean Christopherson wrote:
> When binding a memslot to a guest_memfd file, treat the offset and size as
> unsigned values to fix a bug where the sum of the two can result in a false
> negative when checking for overflow against the size of the file.  Passing
> unsigned values also avoids relying on somewhat obscure checks in other
> flows for safety, and tracks the offset and size as they are intended to be
> tracked, as unsigned values.
> 
> On 64-bit kernels, the number of pages a memslot contains and thus the size
> (and offset) of its guest_memfd binding are unsigned 64-bit values.  Taking
> the offset+size as an loff_t instead of a uoff_t inadvertently converts
> the unsigned value to a signed value if the offset and/or size is massive.
> 
> Locally storing the offset and size as signed values is benign in and of
> itself (though even that is *extremely* difficult to discern), but
> operating on their sum is not.
> 
> For the offset, KVM explicitly checks against a negative value, which might
> seem like a bug as KVM could incorrectly reject a legitimate binding, but
> that's not actually the case as KVM_CREATE_GUEST_MEMFD takes a signed value
> for its size, i.e. a would-be-negative offset is also greater than the
> maximum possible size of any guest_memfd file.
> 
> Regarding the size, while KVM lacks an explicit check for a negative value,
> i.e. seemingly has a flawed overflow check, KVM restricts the number of
> pages in a single memslot to the largest positive signed 32-bit value:
> 
>         if (id < KVM_USER_MEM_SLOTS &&
>             (mem->memory_size >> PAGE_SHIFT) > KVM_MEM_MAX_NR_PAGES)
>                 return -EINVAL;
> 
> and so that maximum "size" will ever be is 0x7fffffff000.
> 
> The sum of the two is, however, problematic.  While the size is restricted
> by KVM's memslot logic, the offset is not, i.e. the offset is completely
> unchecked until the "offset + size > i_size_read(inode)" check.  If the
> offset is the (nearly) largest possible _positive_ value, then adding size
> to the offset can result in a signed, negative 64-bit value.  When compared
> against the size of the file (guaranteed to be positive), the negative sum
> is always smaller, and KVM incorrectly allows the absurd offset.
> 
> Opportunistically add missing includes in kvm_mm.h (instead of relying on
> its parents).
> 
> Fixes: a7800aa80ea4 ("KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory")
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>

Reviewed-by: Michael Roth <michael.roth@xxxxxxx>