Re: [PATCH RFC v3 42/88] KVM: guest_memfd: Export kvm_gmem_revoke for provider-driven revocation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] KVM: guest_memfd: race between kvm_gmem_revoke() and inode
  teardown
- [Low] KVM: guest_memfd: stray @mmap kernel-doc entry in guest_memfd.h
--

commit 3bc97f7ff873e4720a82787260ba8378c8ce9990
Author: Ackerley Tng <ackerleytng@google.com>

KVM: guest_memfd: Export kvm_gmem_revoke for provider-driven revocation

This commit introduces __kvm_gmem_revoke() and exports kvm_gmem_revoke()
so external memory providers can revoke physical memory ranges from
guest_memfd without calling back into .put_pfn_range().

> diff --git a/include/linux/guest_memfd.h b/include/linux/guest_memfd.h
> index 697f8f8c9b0ae..fb2279a001ebf 100644
> --- a/include/linux/guest_memfd.h
> +++ b/include/linux/guest_memfd.h
> @@ -28,8 +28,13 @@ struct mempolicy;
>   *           @pfn with uniform @pfn_attrs in @max_order. Once @get_pfn succeeds,
>   *           the provider must keep the @index-to-@pfn mapping and @pfn_attrs
>   *           constant, and the memory accessible, until guest_memfd stops using
> - *           this page. Returns a negative error code on failure. The provider
> - *           must define @lookup_pfn_range when @get_pfn is defined.
> + *           this page. Do not take locks held across kvm_gmem_revoke().
> + *           Returns a negative error code on failure. The provider must define
> + *           @lookup_pfn_range when @get_pfn is defined.
> + * @mmap: Optional callback to set up user mappings. Called when userspace calls
> + *        mmap() on guest_memfd. The provider can customize VMA flags,
> + *        cacheability/protections, or reject the mapping. If not defined,
> + *        guest_memfd falls back to default mapping behavior.

[Severity: Low]
Does this @mmap kernel-doc entry belong in this commit?
struct guest_memfd_provider_operations has no mmap field here, which
triggers a scripts/kernel-doc warning and leaves a duplicate @mmap entry
once commit "KVM: guest_memfd: Support mmap provider operation" adds its
own description later in the series.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 1b6fc44ba00bb..023bf095340be 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c

[ ... ]

> @@ -469,6 +482,45 @@ static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
>  	return 0;
>  }
>  
> +static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
> +{
> +	return __kvm_gmem_revoke(inode, offset >> PAGE_SHIFT, len >> PAGE_SHIFT,
> +				 NULL, true);
> +}
> +
> +/**
> + * kvm_gmem_revoke - Revoke and reclaim a range of memory in guest_memfd.
> + * @inode:    The guest_memfd inode.
> + * @start:    Starting page index within the file.
> + * @nr_pages: Number of pages to revoke.
> + * @pfn:      The physical address / frame number being revoked. Directly
> + *            reclaims this physical range without querying the provider's
> + *            .lookup_pfn_range().
> + *
> + * guest_memfd will do everything to make sure the range revoked is no longer in
> + * use across guest and host mappings, as well as reclaim any confidential
> + * architecture state.
> + *
> + * Before calling kvm_gmem_revoke(), the caller must ensure that future
> + * .get_pfn() calls for the revoked range will fail so new mappings cannot be
> + * faulted back in after kvm_gmem_revoke() releases the inode's invalidate_lock.
> + * Must be called before accessing, freeing, resetting, or powering down the
> + * backing resource, and the caller must keep the @start-to-PFN mapping valid
> + * and accessible until kvm_gmem_revoke() returns. .put_pfn_range() will not be
> + * called.
> + *
> + * Acquires @inode->i_mapping->invalidate_lock exclusively; callers must not
> + * hold any lock that is also acquired inside provider callbacks.
> + *
> + * Return: 0 on success, or a negative error code on failure.
> + */
> +int kvm_gmem_revoke(struct inode *inode, pgoff_t start, size_t nr_pages,
> +		    const unsigned long *pfn)
> +{
> +	return __kvm_gmem_revoke(inode, start, nr_pages, pfn, false);
> +}
> +EXPORT_SYMBOL_GPL(kvm_gmem_revoke);

[Severity: High]
Can kvm_gmem_revoke() race with inode teardown in
kvm_gmem_destroy_inode()?

virt/kvm/guest_memfd.c:kvm_gmem_destroy_inode() {
    ...
	if (gi->provider_ops && kvm_gmem_is_pfn_mode(gi)) {
		pgoff_t nr_pages = i_size_read(inode) >> PAGE_SHIFT;

		kvm_gmem_pfn_reclaim(gi, 0, nr_pages);
		gmem_provider_put_pfn_range(gi, 0, nr_pages);
	}

	gmem_provider_release(gi);
    ...
}

Because provider callbacks cannot take provider locks held across
kvm_gmem_revoke(), inode->i_mapping->invalidate_lock is the only lock
serializing kvm_gmem_revoke() against reclaim and .put_pfn_range(), yet
kvm_gmem_destroy_inode() runs both without holding
filemap_invalidate_lock() before gmem_provider_release() detaches the
provider:

CPU0                                  CPU1
iput(inode)
  evict()
    kvm_gmem_destroy_inode()
      kvm_gmem_pfn_reclaim()
        gmem_provider_lookup_pfn_range()
                                      /* provider marks range absent */
                                      kvm_gmem_revoke(inode, ..., &pfn)
                                        filemap_invalidate_lock()
                                        kvm_arch_gmem_reclaim(*pfn, ...)
                                        filemap_invalidate_unlock()
                                      /* provider frees or resets memory */
        kvm_arch_gmem_reclaim(pfn, ...)
          <-- concurrent reclaim / use-after-free
      gmem_provider_put_pfn_range()

Since kvm_gmem_destroy_inode() does not hold filemap_invalidate_lock(),
kvm_gmem_revoke() can run kvm_arch_gmem_reclaim() concurrently on the same
PFNs and return 0 to the provider while CPU0 is still reclaiming and
releasing the backing pages.  In folio mode, evict() also runs
truncate_inode_pages_final() and clear_inode() before
kvm_gmem_destroy_inode() calls gmem_provider_release(), allowing a
concurrent kvm_gmem_revoke() to call truncate_inode_pages_range() during
or after final inode truncation.

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