Re: [PATCH v2 1/3] KVM: guest_memfd: Return -EEXIST for overlapping bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The return value of xa_store_range() is ignored in kvm_gmem_bind(), masking memory allocation failures and leading to a Use-After-Free vulnerability.
- [High] A signed integer overflow in the file size bounds check inside kvm_gmem_bind() allows userspace to map memslots beyond the allocated size of the guest_memfd.
--

commit 7edaac3f2255cc17c741557ce1e44f51d4cece29
Author: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>

KVM: guest_memfd: Return -EEXIST for overlapping bindings

This patch updates the KVM_SET_USER_MEMORY_REGION2 ioctl to return -EEXIST
instead of -EINVAL when a guest_memfd range overlaps an existing binding.
This aligns the error reporting with overlapping GPA memslots.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 69c9d6d546b28..46727539d08ac 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -675,6 +675,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
>  
>  	if (!xa_empty(&f->bindings) &&
>  	    xa_find(&f->bindings, &start, end - 1, XA_PRESENT)) {
> +		r = -EEXIST;

[Severity: High]
This isn't a bug introduced by this patch, but looking at the bounds check
earlier in kvm_gmem_bind():

    if (offset < 0 || !PAGE_ALIGNED(offset) ||
        offset + size > i_size_read(inode))
        goto err;

Are these variables susceptible to a signed integer overflow?

Both offset and size are evaluated as loff_t values. If userspace provides an
offset close to the signed 64-bit maximum, adding size could wrap around to a
negative value.

Would this cause the offset + size > i_size_read(inode) condition to bypass
the file size limits and allow the guest to map memslots beyond the allocated
size?


[Severity: High]
This is also a pre-existing issue, but I noticed the return value of
xa_store_range() isn't checked later in kvm_gmem_bind():

    xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
    filemap_invalidate_unlock(inode->i_mapping);

If memory allocation fails and xa_store_range() returns an error pointer,
does the memslot get omitted from f->bindings while kvm_gmem_bind() still
proceeds as if it was successful?

When the guest_memfd is closed, kvm_gmem_release() nullifies active bindings
but would miss this omitted slot, leaving slot->gmem.file set while the VFS
frees the underlying file.

Later, when the VM is destroyed, could kvm_gmem_unbind() attempting to
access the memslot's file via get_file_active() trigger a use-after-free?

>  		filemap_invalidate_unlock(inode->i_mapping);
>  		goto err;
>  	}

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