Re: [RFC PATCH v2 01/11] KVM: guest_memfd: Introduce guest memory ops and route native gmem through them

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: ignored xa_store_range failure in kvm_gmem_native_bind allows memory access to freed host pages
--

commit ab5f37736d076d164ccad403ab4a50bd2032eb9b
Author: David Woodhouse <dwmw@amazon.co.uk>

KVM: guest_memfd: Introduce guest memory ops and route native gmem through them

This commit introduces a vtable (struct kvm_gmem_ops) to allow alternate
subsystems to supply guest memory backing. The built-in guest_memfd
implementation is refactored to route its memory-side and file-level
callbacks through this new shared dispatch mechanism.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index db57c5766ab60..057f90eb835a1 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c

[ ... ]

> +static int kvm_gmem_native_bind(struct file *file, struct kvm *kvm,
> +				struct kvm_memory_slot *slot, loff_t offset)
> +{

[ ... ]

> +	/*
> +	 * memslots of flag KVM_MEM_GUEST_MEMFD are immutable, so bind must
> +	 * occur on a new memslot.  Because the memslot is not visible yet,
> +	 * kvm_gmem_get_pfn() is guaranteed to see the file.
> +	 */
> +	WRITE_ONCE(slot->gmem.file, file);
> +	slot->gmem.pgoff = start;
> +	if (kvm_gmem_supports_mmap(inode))
> +		slot->flags |= KVM_MEMSLOT_GMEM_ONLY;
> +
> +	xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
> +	r = 0;

[Severity: High]
Does this code handle potential memory allocation failures from 
xa_store_range()?

xa_store_range() can fail with -ENOMEM when allocating internal nodes for the
XArray. Because the return value is ignored and the path unconditionally
returns 0, the memslot is successfully bound from KVM's perspective, but is
missing from the f->bindings tracking structure.

During operations like fallocate (hole punch) or file release, KVM iterates
over f->bindings to find and zap the guest mappings. If the slot is missing
from the XArray, the guest mappings will not be zapped, which could allow the
guest to retain read/write access to physical memory that has been returned
to the host allocator.

> +out:
> +	filemap_invalidate_unlock(inode->i_mapping);
> +	return r;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014093733.90312-1-griffoul@gmail.com?part=1