Hi Ackerley, On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay <[email protected]> wrote: > > From: Ackerley Tng <[email protected]> > > __kvm_gmem_invalidate_begin() and __kvm_gmem_invalidate_end() actually do > not specially handle -1ul. -1ul is used as a huge number, which legal > indices do not exceed, and hence the invalidation works as expected. > > Since a later patch is going to make use of the exact range, calculate the > size of the guest_memfd inode and use it as the end range for invalidating > SPTEs. > > Signed-off-by: Ackerley Tng <[email protected]>
Want to look at what Sashiko has to say? Seems to be a real issue: https://sashiko.dev/#/patchset/20260507-gmem-inplace-conversion-v6-0-91ab5a8b19a4%40google.com?part=16 If I understand correctly, the fix should simple: use check_add_overflow() to validate the offset and size parameters in kvm_gmem_bind() int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot, unsigned int fd, loff_t offset) { loff_t size = slot->npages << PAGE_SHIFT; + loff_t end; unsigned long start, end_index; struct gmem_file *f; ... - if (offset < 0 || !PAGE_ALIGNED(offset) || - offset + size > i_size_read(inode)) + if (offset < 0 || !PAGE_ALIGNED(offset) || + check_add_overflow(offset, size, &end) || + end > i_size_read(inode)) goto err; What do you think? /fuad > --- > virt/kvm/guest_memfd.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c > index 050a8c092b1a3..9f6eebfb68f6b 100644 > --- a/virt/kvm/guest_memfd.c > +++ b/virt/kvm/guest_memfd.c > @@ -370,6 +370,7 @@ static int kvm_gmem_release(struct inode *inode, struct > file *file) > struct kvm_memory_slot *slot; > struct kvm *kvm = f->kvm; > unsigned long index; > + pgoff_t end; > > /* > * Prevent concurrent attempts to *unbind* a memslot. This is the > last > @@ -396,9 +397,10 @@ static int kvm_gmem_release(struct inode *inode, struct > file *file) > * Zap all SPTEs pointed at by this file. Do not free the backing > * memory, as its lifetime is associated with the inode, not the file. > */ > - __kvm_gmem_invalidate_begin(f, 0, -1ul, > + end = i_size_read(inode) >> PAGE_SHIFT; > + __kvm_gmem_invalidate_begin(f, 0, end, > kvm_gmem_get_invalidate_filter(inode)); > - __kvm_gmem_invalidate_end(f, 0, -1ul); > + __kvm_gmem_invalidate_end(f, 0, end); > > list_del(&f->entry); > > > -- > 2.54.0.563.g4f69b47b94-goog > >
