Re: [PATCH 1/2] KVM: guest_memfd: Gracefully handle xarray errors when binding a memslot

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Partial failure of xa_store_range() leaves dangling pointers to a freed memslot in the guest_memfd bindings xarray, leading to a Use-After-Free when the file is released.
--

commit 31a2b2a91bb8b7043e1a087271db5def7eb591b8
Author: Sean Christopherson <seanjc@google.com>

KVM: guest_memfd: Gracefully handle xarray errors when binding a memslot

This commit updates kvm_gmem_bind() to handle errors returned by
xa_store_range(). If inserting a memslot into a guest_memfd bindings xarray
fails, the error is propagated to the caller to abort memslot creation.

Cc: stable@vger.kernel.org
Closes: https://lore.kernel.org/all/20260823135031.4F6DC1F000E9%40smtp.kernel.org

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c

[ ... ]

> @@ -654,7 +658,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
>  	if (kvm_gmem_supports_mmap(inode))
>  		slot->flags |= KVM_MEMSLOT_GMEM_ONLY;
>  
> -	xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
> +	xar = xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
>  	filemap_invalidate_unlock(inode->i_mapping);
>  
>  	/*
> @@ -662,7 +666,12 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
>  	 * not the other way 'round.  Active bindings are invalidated if the
>  	 * file is closed before memslots are destroyed.
>  	 */
> -	r = 0;
> +	r = xa_is_err(xar) ? xa_err(xar) : 0;
> +	if (r) {
> +		slot->gmem.file = NULL;
> +		slot->gmem.pgoff = 0;
> +		slot->flags &= ~KVM_MEMSLOT_GMEM_ONLY;
> +	}

[Severity: High]
If xa_store_range() encounters an error halfway through the range (for
example, -ENOMEM), can it partially succeed and leave some entries pointing
to the memslot in the f->bindings xarray?

If this happens, the error is propagated back to kvm_set_memory_region(),
which then aborts the operation and frees the memslot structure. Does this
leave dangling pointers in the f->bindings xarray?

Later, when the guest_memfd file is closed, kvm_gmem_release() iterates
over the xarray:

virt/kvm/guest_memfd.c:kvm_gmem_release() {
    ...
    xa_for_each(&f->bindings, index, slot)
        WRITE_ONCE(slot->gmem.file, NULL);
    ...
}

Could this trigger a use-after-free when kvm_gmem_release() writes NULL to
the already freed memslot? Should any partially inserted xarray elements be
erased from f->bindings before propagating the error?

>  err:
>  	fput(file);
>  	return r;

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