Re: [PATCH v7 16/42] KVM: guest_memfd: Determine invalidation filter from memory attributes

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free of KVM memory slots during guest_memfd invalidation due to missing SRCU read lock.
--

commit 405f31b1513a8790de48221d335a28b8c5c7bf77
Author: Ackerley Tng <ackerleytng@google.com>
KVM: guest_memfd: Determine invalidation filter from memory attributes

This commit updates kvm_gmem_get_invalidate_filter to inspect the memory
attributes maple tree for a given range. This allows KVM to identify if
the range contains private pages, shared pages, or both, and set the filter
bits accordingly.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index a361b3f672516..4570e90f8c042 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
[ ... ]
> @@ -244,7 +256,7 @@ static void kvm_gmem_invalidate_begin(struct inode *inode, pgoff_t start,
>  	enum kvm_gfn_range_filter attr_filter;
>  	struct gmem_file *f;
>  
> -	attr_filter = kvm_gmem_get_invalidate_filter(inode);
> +	attr_filter = kvm_gmem_get_invalidate_filter(inode, start, end);
>  
>  	kvm_gmem_for_each_file(f, inode)
>  		__kvm_gmem_invalidate_begin(f, start, end, attr_filter);

[Severity: High]
This is a pre-existing issue, but does __kvm_gmem_invalidate_begin() safely
access the memory slots when iterating over f->bindings?

In __kvm_gmem_invalidate_begin(), the code iterates over f->bindings using
xa_for_each_range(). The retrieved slot pointers are protected by the kvm
srcu, but the read lock doesn't appear to be held during the iteration:

static void __kvm_gmem_invalidate_begin(...)
{
	...
	xa_for_each_range(&f->bindings, index, slot, start, end - 1) {
		pgoff_t pgoff = slot->gmem.pgoff;
	...

When a memory slot is deleted, the code calls kvm_gmem_unbind() and
kfree(slot) after synchronize_srcu(&kvm->srcu). If the read lock isn't held,
could a concurrent memslot deletion free the slot while it is still being
accessed in this loop?

This race might occur when __kvm_gmem_invalidate_begin() is invoked without
holding a file descriptor reference, such as from kvm_gmem_error_folio()
after a memory failure. If the file descriptor is closed (refcount is 0),
kvm_gmem_unbind() observes a dead file and skips acquiring the
filemap_invalidate_lock. This allows concurrent xarray modifications,
potentially leading to a use-after-free when dereferencing slot->gmem.pgoff.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-gmem-inplace-conversion-v7-0-2f0fae496530@google.com?part=16