Re: [PATCH v10 11/41] KVM: guest_memfd: Ensure pages are not in use before conversion

[Yan Zhao <yan.y.zhao@intel.com>]

On Fri, Aug 07, 2026 at 02:52:50PM -0700, Ackerley Tng via B4 Relay wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> When converting memory to private in guest_memfd, it is necessary to ensure
> that the pages are not currently being accessed by any other part of the
> kernel or userspace to avoid any current user writing to guest private
> memory.
> 
> guest_memfd checks for unexpected refcounts to determine whether a page is
> still in use. The only expected refcounts after unmapping the range
> requested for conversion are those that are held by guest_memfd itself.
> 
> Update the kvm_memory_attributes2 structure to include an error_offset
> field. This allows KVM to report the exact offset where a conversion
> failed to userspace. If the safety check fails, return -EAGAIN and copy
> the error_offset back to userspace so that it can potentially retry the
> operation or handle the failure gracefully.
> 
> Update documentation to document the error_offset field and the possible
> -EAGAIN error.
> 
> Suggested-by: David Hildenbrand <david@kernel.org>
> Co-developed-by: Vishal Annapurve <vannapurve@google.com>
> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> Reviewed-by: Fuad Tabba <tabba@google.com>
> Tested-by: Shivank Garg <shivankg@amd.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
>  Documentation/virt/kvm/api.rst | 19 ++++++++++--
>  include/uapi/linux/kvm.h       |  3 +-
>  virt/kvm/guest_memfd.c         | 66 ++++++++++++++++++++++++++++++++++++++----
>  3 files changed, 80 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 1a3f664dbb197..1e64026d7c1e9 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6583,7 +6583,7 @@ KVM_S390_KEYOP_SSKE
>  :Capability: KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES
>  :Architectures: all
>  :Type: guest_memfd ioctl
> -:Parameters: struct kvm_memory_attributes2 (in)
> +:Parameters: struct kvm_memory_attributes2 (in/out)
>  :Returns: 0 on success, <0 on error
>  
>  Errors:
> @@ -6592,6 +6592,8 @@ Errors:
>    EINVAL     The specified `offset` or `size` was invalid (e.g. not
>               page aligned, causes an overflow, or size is zero).
>    EFAULT     The parameter address was invalid.
> +  EAGAIN     Some page within requested range had unexpected refcounts. The
> +             offset of the page will be returned in `error_offset`.
>    ENOMEM     Ran out of memory trying to track private/shared state
>    ========== ===============================================================
>  
> @@ -6605,6 +6607,7 @@ Attribute values are shared with KVM_SET_MEMORY_ATTRIBUTES.
>  ::
>  
>    struct kvm_memory_attributes2 {
> +	/* in */
>  	union {
>  		__u64 address;
>  		__u64 offset;
> @@ -6612,7 +6615,9 @@ Attribute values are shared with KVM_SET_MEMORY_ATTRIBUTES.
>  	__u64 size;
>  	__u64 attributes;
>  	__u64 flags;
> -	__u64 reserved[12];
> +	/* out */
> +	__u64 error_offset;
> +	__u64 reserved[11];
>    };
>  
>    #define KVM_MEMORY_ATTRIBUTE_PRIVATE           (1ULL << 3)
> @@ -6634,6 +6639,16 @@ which includes operations such as unmapping pages from the host or
>  stage-2 page tables, may result in side effects on memory contents
>  that vary across different trusted firmware implementations.
>  
> +If this ioctl returns -EAGAIN, the offset of the page with unexpected
> +refcounts will be returned in `error_offset`. This can occur if there
> +are transient refcounts on the pages, taken by other parts of the
> +kernel.
> +
> +Userspace is expected to figure out how to remove all known refcounts
> +on the shared pages, such as refcounts taken by get_user_pages(), and
> +try the ioctl again. A possible source of these long term refcounts is
> +if the guest_memfd memory was pinned in IOMMU page tables.
> +
>  See also: :ref: `KVM_SET_MEMORY_ATTRIBUTES`.
>  
>  .. _kvm_run:
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 80985e28e3b21..129d6f6303251 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1661,7 +1661,8 @@ struct kvm_memory_attributes2 {
>  	__u64 size;
>  	__u64 attributes;
>  	__u64 flags;
> -	__u64 reserved[12];
> +	__u64 error_offset;
> +	__u64 reserved[11];
>  };
>  
>  #define KVM_MEMORY_ATTRIBUTE_PRIVATE           (1ULL << 3)
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 3783e63476569..13c3989136f67 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -524,8 +524,42 @@ static int kvm_gmem_mas_preallocate(struct ma_state *mas, u64 attributes,
>  	return mas_preallocate(mas, xa_mk_value(attributes), GFP_KERNEL);
>  }
>  
> +static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
> +					    size_t nr_pages, pgoff_t *err_index)
> +{
> +	struct address_space *mapping = inode->i_mapping;
> +	const int filemap_get_folios_refcount = 1;
> +	pgoff_t last = start + nr_pages - 1;
> +	struct folio_batch fbatch;
> +	bool safe = true;
> +	pgoff_t next;
> +	int i;
> +
> +	folio_batch_init(&fbatch);
> +
> +	next = start;
> +	while (safe && filemap_get_folios(mapping, &next, last, &fbatch)) {
> +		for (i = 0; i < folio_batch_count(&fbatch); ++i) {
> +			struct folio *folio = fbatch.folios[i];
> +
> +			if (folio_ref_count(folio) !=
> +			    folio_nr_pages(folio) + filemap_get_folios_refcount) {
> +				safe = false;
> +				*err_index = max(start, folio->index);
> +				break;
> +			}
> +		}
> +
> +		folio_batch_release(&fbatch);
> +		cond_resched();
> +	}
> +
> +	return safe;
> +}
> +
>  static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
> -				     size_t nr_pages, uint64_t attrs)
> +				     size_t nr_pages, uint64_t attrs,
> +				     pgoff_t *err_index)
>  {
>  	bool to_private = attrs & KVM_MEMORY_ATTRIBUTE_PRIVATE;
>  	struct address_space *mapping = inode->i_mapping;
> @@ -542,8 +576,21 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>  
>  	mas_init(&mas, mt, start);
>  	r = kvm_gmem_mas_preallocate(&mas, attrs, start, nr_pages);
> -	if (r)
> +	if (r) {
> +		*err_index = start;
>  		goto out;
> +	}
> +
> +	if (to_private) {
> +		unmap_mapping_pages(mapping, start, nr_pages, false);
> +
> +		if (!kvm_gmem_is_safe_for_conversion(inode, start, nr_pages,
> +						     err_index)) {
Note: conversion failures could occur if another vCPU is attempting to map a GFN
within this range.

CPU 0 (setting attributes)          CPU 1 (attempting to map)
--------------------------          --------------------
                                 A: mmu_invalidate_retry_gfn_unsafe
                                    filemap_invalidate_lock_shared
                                    __kvm_gmem_get_pfn ==> folio refcount++
                                    filemap_invalidate_unlock_shared

filemap_invalidate_lock
filemap_get_folios
check folio_ref_count(folio) ==> Not match !!
filemap_invalidate_unlock

                                 B: read_lock(&vcpu->kvm->mmu_lock);
                                    is_page_fault_stale
                                    kvm_mmu_finish_page_fault ==>folio recount--
				    read_unlock(&vcpu->kvm->mmu_lock);


Retrying in kvm_gmem_is_safe_for_conversion() or moving the invocation of
kvm_mmu_invalidate_start() + kvm_mmu_invalidate_range_add() to an earlier
position does not help as long as CPU 1 stays at stage A.

So, should we avoid this failure?
e.g., by moving filemap_invalidate_unlock_shared() from stage A to after
stage B?


> +			mas_destroy(&mas);
> +			r = -EAGAIN;
> +			goto out;
> +		}
> +	}
>  
>  	/*
>  	 * From this point on guest_memfd has performed necessary
> @@ -564,9 +611,10 @@ static long kvm_gmem_set_attributes(struct file *file, void __user *argp)
>  	struct gmem_file *f = file->private_data;
>  	struct inode *inode = file_inode(file);
>  	struct kvm_memory_attributes2 attrs;
> +	pgoff_t err_index;
>  	size_t nr_pages;
>  	pgoff_t index;
> -	int i;
> +	int i, r;
>  
>  	if (copy_from_user(&attrs, argp, sizeof(attrs)))
>  		return -EFAULT;
> @@ -592,8 +640,16 @@ static long kvm_gmem_set_attributes(struct file *file, void __user *argp)
>  
>  	nr_pages = attrs.size >> PAGE_SHIFT;
>  	index = attrs.offset >> PAGE_SHIFT;
> -	return __kvm_gmem_set_attributes(inode, index, nr_pages,
> -					 attrs.attributes);
> +	r = __kvm_gmem_set_attributes(inode, index, nr_pages, attrs.attributes,
> +				      &err_index);
> +	if (r) {
> +		attrs.error_offset = ((uint64_t)err_index) << PAGE_SHIFT;
> +
> +		if (copy_to_user(argp, &attrs, sizeof(attrs)))
> +			return -EFAULT;
> +	}
> +
> +	return r;
>  }
>  
>  static long kvm_gmem_ioctl(struct file *file, unsigned int ioctl,
> 
> -- 
> 2.55.0.654.g21b8a5bc05-goog
> 
>