On Thu, May 21, 2026, Fuad Tabba wrote:
> 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) ||
Eww, TIL I'm not a fan of check_add_overflow(). Burying an out-param in an
if-statement is nasty.
> + end > i_size_read(inode))
This is all rather silly. @offset and and @slot->npages are fundamentally
unsigned values. I don't see any reason to convert them to signed values, only
to convert them *back* to unsigned values (when stored in start/end, because
xarrays
operate on "unsigned long" indices).
i_size_read() obviously has to return a positive value, so can't we just do
this?
diff --git virt/kvm/guest_memfd.c virt/kvm/guest_memfd.c
index a35a55571a2d..9c6dbb54e800 100644
--- virt/kvm/guest_memfd.c
+++ virt/kvm/guest_memfd.c
@@ -640,9 +640,9 @@ int kvm_gmem_create(struct kvm *kvm, struct
kvm_create_guest_memfd *args)
}
int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
- unsigned int fd, loff_t offset)
+ unsigned int fd, u64 offset)
{
- loff_t size = slot->npages << PAGE_SHIFT;
+ u64 size = slot->npages << PAGE_SHIFT;
unsigned long start, end;
struct gmem_file *f;
struct inode *inode;
@@ -664,8 +664,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot
*slot,
inode = file_inode(file);
- if (offset < 0 || !PAGE_ALIGNED(offset) ||
- offset + size > i_size_read(inode))
+ if (!PAGE_ALIGNED(offset) || offset + size > i_size_read(inode))
goto err;
filemap_invalidate_lock(inode->i_mapping);
diff --git virt/kvm/kvm_mm.h virt/kvm/kvm_mm.h
index 9fcc5d5b7f8d..3cb5ef86d0d9 100644
--- virt/kvm/kvm_mm.h
+++ virt/kvm/kvm_mm.h
@@ -72,7 +72,7 @@ int kvm_gmem_init(struct module *module);
void kvm_gmem_exit(void);
int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args);
int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
- unsigned int fd, loff_t offset);
+ unsigned int fd, u64 offset);
void kvm_gmem_unbind(struct kvm_memory_slot *slot);
#else
static inline int kvm_gmem_init(struct module *module)
@@ -80,9 +80,8 @@ static inline int kvm_gmem_init(struct module *module)
return 0;
}
static inline void kvm_gmem_exit(void) {};
-static inline int kvm_gmem_bind(struct kvm *kvm,
- struct kvm_memory_slot *slot,
- unsigned int fd, loff_t offset)
+static inline int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
+ unsigned int fd, u64 offset)
{
WARN_ON_ONCE(1);
return -EIO;
