Hi Ackerley,
On Mon, 31 Aug 2026 at 01:25, Ackerley Tng via B4 Relay
<[email protected]> wrote:
>
> From: Ackerley Tng <[email protected]>
>
> 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 any outstanding references to determine whether a
> page is still in use. The only expected references 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. 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 <[email protected]>
> Co-developed-by: Vishal Annapurve <[email protected]>
> Signed-off-by: Vishal Annapurve <[email protected]>
> Reviewed-by: Fuad Tabba <[email protected]>
> Tested-by: Shivank Garg <[email protected]>
> Signed-off-by: Ackerley Tng <[email protected]>
> ---
> Documentation/virt/kvm/api.rst | 19 +++++++++--
> include/uapi/linux/kvm.h | 3 +-
> virt/kvm/guest_memfd.c | 77
> +++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 91 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 4dbf452f6e809..25cb15970f1ae 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6613,7 +6613,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:
> @@ -6622,6 +6622,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
> ========== ===============================================================
>
> @@ -6635,6 +6637,7 @@ Attribute values are shared with
> KVM_SET_MEMORY_ATTRIBUTES.
> ::
>
> struct kvm_memory_attributes2 {
> + /* in */
> union {
> __u64 address;
> __u64 offset;
> @@ -6642,7 +6645,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)
> @@ -6664,6 +6669,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`.
>
> 4.145 KVM_PPC_GET_COMPAT_CAPS
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index ac371a50041c9..8dff2fc1972e9 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1665,7 +1665,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 803c7cdbbe0f6..fe02c47c85fb5 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -538,8 +538,46 @@ 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_has_outstanding_references(struct inode *inode,
> + pgoff_t start, size_t
> nr_pages,
> + pgoff_t *err_index)
> +{
> + struct address_space *mapping = inode->i_mapping;
> + pgoff_t last = start + nr_pages - 1;
> + bool has_outstanding = false;
> + struct folio_batch fbatch;
> + pgoff_t next;
> + int i;
> +
> + folio_batch_init(&fbatch);
> +
> + next = start;
> + while (has_outstanding && filemap_get_folios(mapping, &next, last,
> &fbatch)) {
has_outstanding starts as false, so the loop never runs and the function
always returns false. The outstanding-reference check is dead at this
patch, so a to-private conversion would not be rejected even when a page
still has an outstanding reference.
It's fixed later in "KVM: guest_memfd: Handle lru_add fbatch refcounts
during conversion safety check", which changes the condition to
!has_outstanding. I think that fix belongs in this patch, so the check
works when it is introduced and the series bisects cleanly.
Missed it on the previous review, but with this fixed it still holds.
Cheers,
/fuad
> + for (i = 0; i < folio_batch_count(&fbatch); ++i) {
> + struct folio *folio = fbatch.folios[i];
> +
> + /*
> + * Outstanding references are anything other than
> those
> + * from the page cache, plus 1 temporary reference
> held
> + * by filemap_get_folios() in the folio batch.
> + */
> + if (folio_ref_count(folio) != folio_nr_pages(folio) +
> 1) {
> + has_outstanding = true;
> + *err_index = max(start, folio->index);
> + break;
> + }
> + }
> +
> + folio_batch_release(&fbatch);
> + cond_resched();
> + }
> +
> + return has_outstanding;
> +}
> +
> 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;
> @@ -556,8 +594,28 @@ 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) {
> + /*
> + * Forcefully unmap the pages from all userspace page tables,
> + * and then verify there are no outstanding references, e.g.
> + * acquired via GUP or similar. Tell userspace to try again
> if
> + * there are outstanding references and hope that whatever has
> + * pinned the page will put its reference "soon".
> + */
> + unmap_mapping_pages(mapping, start, nr_pages, false);
> +
> + if (kvm_gmem_has_outstanding_references(inode, start,
> nr_pages,
> + err_index)) {
> + mas_destroy(&mas);
> + r = -EAGAIN;
> + goto out;
> + }
> + }
>
> /*
> * From this point on guest_memfd has performed necessary
> @@ -578,9 +636,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;
> @@ -606,8 +665,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.897.gb25b4bd76c-goog
>
>
