Re: [PATCH RFC v3 04/88] KVM: guest_memfd: Support provider folio invalidation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] KVM: guest_memfd: AS_RELEASE_ALWAYS set without release_folio()
  in kvm_gmem_aops
--

commit 2a00824af3a696cc40b4c36a78f74b7c53a20eb3
Author: Ackerley Tng <ackerleytng@google.com>

KVM: guest_memfd: Support provider folio invalidation

This commit adds a provider .invalidate_folio() callback to guest_memfd
and sets mapping_set_release_always() on the guest_memfd address_space so
truncate paths invoke kvm_gmem_invalidate_folio().

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 545c3e646e9c1..ee823f2ef0368 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c

[ ... ]

> @@ -942,10 +950,24 @@ static void kvm_gmem_free_folio(struct folio *folio)
>  }
>  #endif
>  
> +static void kvm_gmem_invalidate_folio(struct folio *folio, size_t offset,
> +				      size_t len)
> +{
> +	struct inode *inode = folio->mapping->host;
> +	struct gmem_inode *gi = GMEM_I(inode);
> +
> +	/* guest_memfd only allows truncating full folios. */
> +	if (WARN_ON_ONCE(offset != 0 || len != folio_size(folio)))
> +		return;
> +
> +	gmem_provider_invalidate_folio(gi, folio);
> +}
> +
>  static const struct address_space_operations kvm_gmem_aops = {
>  	.dirty_folio = noop_dirty_folio,
>  	.migrate_folio	= kvm_gmem_migrate_folio,
>  	.error_remove_folio = kvm_gmem_error_folio,
> +	.invalidate_folio = kvm_gmem_invalidate_folio,
>  #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM
>  	.free_folio = kvm_gmem_free_folio,
>  #endif
> @@ -984,6 +1006,7 @@ static int kvm_gmem_init_inode(struct inode *inode, loff_t size, u64 flags)
>  	 */
>  	mapping_set_inaccessible(inode->i_mapping);
>  	WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
> +	mapping_set_release_always(inode->i_mapping);

[Severity: High]
Does calling mapping_set_release_always() without a .release_folio
callback in kvm_gmem_aops cause filemap_release_folio() to fall back to
try_to_free_buffers() and evict guest_memfd folios without invalidation?

Because mapping_set_release_always() makes folio_needs_release() return
true, filemap_release_folio() runs on the folio and falls back to
try_to_free_buffers() when a_ops->release_folio is absent:

mm/filemap.c:filemap_release_folio() {
	if (mapping && mapping->a_ops->release_folio)
		return mapping->a_ops->release_folio(folio, gfp);
	return try_to_free_buffers(folio);
}

fs/buffer.c:try_to_free_buffers() {
	/* Misconfigured folio check */
	if (WARN_ON_ONCE(!folio_buffers(folio)))
		return true;
...
}

When soft_offline_page() -> soft_offline_in_use_page() ->
mapping_evict_folio() runs on a clean guest_memfd folio with no extra
references (for example, a private folio mapped only in the guest page
tables, as kvm_gmem_get_pfn() drops its folio reference before returning),
mapping_evict_folio() calls filemap_release_folio().  try_to_free_buffers()
triggers a kernel warning and returns true, allowing mapping_evict_folio()
to call remove_mapping() directly.  That removes the folio from
inode->i_mapping without calling kvm_gmem_invalidate_folio() (leaking the
provider's per-folio accounting) and without holding
filemap_invalidate_lock or zapping guest page tables.

Implementing a .release_folio callback in kvm_gmem_aops that returns false
would satisfy the mapping_set_release_always() contract and prevent
mapping_evict_folio() from evicting guest_memfd folios behind KVM's back.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-gmem-alternative-providers-v3-0-8484dde66a7e@google.com?part=4