[RFC PATCH v3 0/5] KVM: guest_memfd: Add KVM_LINK_GUEST_MEMFD ioctl

[RFC PATCH v3 0/5] KVM: guest_memfd: Add KVM_LINK_GUEST_MEMFD ioctl

[Tao Gong]

Hello,

This series adds KVM_LINK_GUEST_MEMFD, a VM ioctl that takes an existing
guest_memfd fd and returns a new guest_memfd fd, bound to the VM on which
the ioctl is invoked, that refers to the same underlying guest_memfd
inode.

It is v3 of the gmem linking RFC [1][2], rebased onto kvm-x86/coco and
reduced to the linking part: the prerequisites from RFC v2 are upstream
now, and the KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM memory context changes will
be posted as a follow-up series on top of this one.

Problem
-------

A guest_memfd is bound to the VM that created it: binding a memslot to
a guest_memfd requires the file's owning VM to be the memslot's VM, and
there is no way to reassign a guest_memfd instance to a different KVM
instance. Passing the fd to another process does not change that.

For VMs with private memory this is a hard limit. Private memory is not
accessible to the host, so it cannot be copied through userspace into a
new VM's guest_memfd. The only way to move a confidential VM to a new VM
instance on the same host (intra-host migration, e.g. to replace the
VMM) is for the new VM to take over the same memory in place.

Design
------

guest_memfd already splits state between the file and the inode: memslot
bindings and the owning VM belong to the file (struct gmem_file), while
the memory itself, its flags and (with in-place conversion) the
shared/private attributes belong to the inode (struct gmem_inode).
Linking creates another file for the same inode, so a second VM can bind
the same guest memory to its memslots without the memory being copied or
going through userspace.

Linking is only allowed between distinct, compatible VMs. On x86 both
VMs must have the same VM type, so that e.g. memory private to a TDX
guest can never be mapped into a non-TDX VM; the generic fallback
requires both VMs to agree on kvm_arch_has_private_mem(). These checks
are defensive; the underlying rule is that KVM must never create a
stage-2 mapping for a confidential VM to another confidential VM's
private memory (see the cutover question below). Linking therefore
does not make private memory shareable between VMs: a private page
remains usable only by the VM that owns it.

Use cases
---------

1. Intra-host migration of VMs with private memory, where the
   destination VM takes over the source VM's guest memory in place:

   a. Userspace passes the source VM's guest_memfd fd to the destination
      VM, e.g. over a Unix socket.
   b. The destination VM calls KVM_LINK_GUEST_MEMFD to get a new fd for
      the same inode, bound to the destination VM.
   c. The destination VM binds the new fd to its memslots with
      KVM_SET_USER_MEMORY_REGION2, at the same offsets as the source.
   d. The remaining VM state is moved with KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM
      (SEV only today; generalized by the follow-up series) and the
      destination VM is run as normal.

   Linking does not revoke the source VM's access; the source simply
   stops using the memory, so every private page keeps a single owner.

2. Sharing guest memory between non-confidential VMs. Both VMs bind the
   same inode at the same time, each through its own file, and the
   memory keeps its guest_memfd properties: neither VMM needs a
   userspace mapping of it, and inode-level operations such as a hole
   punch apply to every linked VM.

   A related use raised in discussion is Nitro Enclaves-style memory
   donation, where a guest gives part of its memory to a sibling VM on
   the same host ("nesting without real nesting").

Alternatives considered
-----------------------

* Passing the VM fd to the new VMM and adding a way to reset the VM
  while keeping its vCPUs and guest_memfds. A KVM VM is bound to the mm
  that created it (VM, vCPU and device ioctls from another process fail
  with -EIO), and its memslots and MMU notifier refer to that address
  space. Re-homing a VM would be considerably more invasive than
  creating a new VM and moving state into it, which is the model KVM
  already uses for SEV intra-host migration
  (KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM); linking extends that model to
  guest_memfd.

* Keeping the memory and its attributes in a "memory provider" and
  creating two independent guest_memfd instances on top of it. The
  guest_memfd inode already plays that role today (page cache,
  attributes and the list of files), so linking gets the same result
  without a new abstraction. If guest_memfd grows a provider layer,
  linking could be expressed on top of it.

Open questions
--------------

* Cutover: for confidential VMs, an explicit cutover that delinks the
  source VM's file at handover, instead of relying on the source to
  stop, may be required for security: it is what guarantees that a
  private page is never usable by two VMs when memory is transferred.
  That would be a separate operation on top of this series;
  non-confidential VMs probably do not need it.

* IOMMU and DMA: whether iommufd page tables and in-flight DMA keep
  working across a handover, and how they would be transferred, is not
  addressed here. The pages themselves stay alive for as long as any
  linked file exists.

* Conversions: with in-place conversion the shared/private attributes
  live on the inode, so a conversion by one VM applies to, and
  invalidates, every linked VM. That is the intended behavior for
  migration, where only one VM is live. Between confidential VMs it is
  also required: with today's hardware they can at best share their
  shared ranges.

Patches 1-2 add the ioctl, patch 3 advertises KVM_CAP_GUEST_MEMFD_LINK,
and patches 4-5 add selftests, including a test that transfers private
memory between two KVM_X86_SW_PROTECTED_VMs.

The series is based on kvm-x86/coco at commit ad06cf5338bc ("KVM:
guest_memfd: Avoid deadlock in error_remove_folio()").

Tested with the KVM selftests (guest_memfd_test, set_memory_region_test,
guest_memfd_conversions_test, private_mem_conversions_test,
private_mem_kvm_exits_test, private_mem_migrate_tests) on an Intel host,
both with and without kvm.gmem_in_place_conversion.

Changes from RFC v2 [2]:
+ Dropped the anon inode and guest_memfd inode patches, which are
  upstream now (anon_inode_make_secure_inode() and commit a63ca4236e67
  ("KVM: guest_memfd: Use guest mem inodes instead of anonymous
  inodes")).
+ Dropped the KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM memory context patches
  and their selftests, to be posted separately on top of this series.
+ Rebased onto kvm-x86/coco: uses struct gmem_file and the inode's
  gmem_file_list, and takes the mapping's invalidate_lock when adding a
  linked file to the list.
+ Renamed kvm_gmem_alloc_view() to kvm_gmem_alloc_file().
+ Only allow linking between compatible VMs (same VM type on x86).
+ Reject non-zero reserved fields and fds larger than INT_MAX.
+ Dropped the i_nlink increment, which had no matching decrement.
+ Take the inode reference before creating the linked file.
+ Added KVM_CAP_GUEST_MEMFD_LINK and gated the selftests on it.
+ Documented KVM_LINK_GUEST_MEMFD in Documentation/virt/kvm/api.rst.
+ Selftests: fixed the negative tests to exercise the intended checks,
  added tests for out-of-range fds, non-zero reserved fields and VM type
  mismatch, and made private_mem_migrate_tests work with in-place
  conversion.

[1] https://lore.kernel.org/all/cover.1691446946.git.ackerleytng@google.com/
[2] https://lore.kernel.org/all/cover.1747368092.git.afranji@google.com/

Ackerley Tng (4):
  KVM: guest_memfd: Refactor out kvm_gmem_alloc_file()
  KVM: guest_memfd: Add ioctl KVM_LINK_GUEST_MEMFD
  KVM: selftests: Add tests for KVM_LINK_GUEST_MEMFD ioctl
  KVM: selftests: Test transferring private memory to another VM

Vishal Annapurve (1):
  KVM: guest_memfd: Advertise support for KVM_LINK_GUEST_MEMFD

 Documentation/virt/kvm/api.rst                |  50 ++++++
 arch/x86/kvm/x86.c                            |  10 ++
 include/linux/kvm_host.h                      |   1 +
 include/uapi/linux/kvm.h                      |   9 ++
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../testing/selftests/kvm/guest_memfd_test.c  |  64 ++++++++
 .../testing/selftests/kvm/include/kvm_util.h  |  18 +++
 .../kvm/x86/private_mem_migrate_tests.c       | 118 ++++++++++++++
 virt/kvm/guest_memfd.c                        | 150 ++++++++++++++----
 virt/kvm/guest_memfd.h                        |   1 +
 virt/kvm/kvm_main.c                           |  12 ++
 11 files changed, 407 insertions(+), 27 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86/private_mem_migrate_tests.c


base-commit: ad06cf5338bc5501bd989584db6dc71972bb436a
-- 
2.56.0.257.gf7497c7d27-goog

[RFC PATCH v3 1/5] KVM: guest_memfd: Refactor out kvm_gmem_alloc_file()

[Tao Gong]

From: Ackerley Tng <ackerleytng@google.com>

The file represents a KVM instance's view into the underlying memory
that is managed by the guest_memfd inode. Refactoring out the file
creation enables this function to be reused when creating multiple views
into the same guest_memfd inode.

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Co-developed-by: Ryan Afranji <afranji@google.com>
Signed-off-by: Ryan Afranji <afranji@google.com>
Co-developed-by: Tao Gong <gongtao@google.com>
Signed-off-by: Tao Gong <gongtao@google.com>
---
 virt/kvm/guest_memfd.c | 67 +++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 27 deletions(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 93f3d29d5751a..3ca6f64d48040 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -937,10 +937,47 @@ static int kvm_gmem_init_inode(struct inode *inode, loff_t size, u64 flags)
 	return r;
 }
 
+/*
+ * Allocate a new guest_memfd file for @kvm that refers to @inode.  On success,
+ * the returned file takes over the caller's reference to @inode.
+ */
+static struct file *kvm_gmem_alloc_file(struct kvm *kvm, struct inode *inode,
+					const char *name)
+{
+	struct gmem_file *f;
+	struct file *file;
+
+	f = kzalloc_obj(*f);
+	if (!f)
+		return ERR_PTR(-ENOMEM);
+
+	/* __fput() will take care of fops_put(). */
+	if (!fops_get(&kvm_gmem_fops)) {
+		kfree(f);
+		return ERR_PTR(-ENOENT);
+	}
+
+	file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR, &kvm_gmem_fops);
+	if (IS_ERR(file)) {
+		fops_put(&kvm_gmem_fops);
+		kfree(f);
+		return file;
+	}
+
+	file->f_flags |= O_LARGEFILE;
+	file->private_data = f;
+
+	kvm_get_kvm(kvm);
+	f->kvm = kvm;
+	xa_init(&f->bindings);
+	list_add(&f->entry, &GMEM_I(inode)->gmem_file_list);
+
+	return file;
+}
+
 static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
 {
 	static const char *name = "[kvm-gmem]";
-	struct gmem_file *f;
 	struct inode *inode;
 	struct file *file;
 	int fd, err;
@@ -949,51 +986,27 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
 	if (fd < 0)
 		return fd;
 
-	f = kzalloc_obj(*f);
-	if (!f) {
-		err = -ENOMEM;
-		goto err_fd;
-	}
-
-	/* __fput() will take care of fops_put(). */
-	if (!fops_get(&kvm_gmem_fops)) {
-		err = -ENOENT;
-		goto err_gmem;
-	}
-
 	inode = anon_inode_make_secure_inode(kvm_gmem_mnt->mnt_sb, name, NULL);
 	if (IS_ERR(inode)) {
 		err = PTR_ERR(inode);
-		goto err_fops;
+		goto err_fd;
 	}
 
 	err = kvm_gmem_init_inode(inode, size, flags);
 	if (err)
 		goto err_inode;
 
-	file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR, &kvm_gmem_fops);
+	file = kvm_gmem_alloc_file(kvm, inode, name);
 	if (IS_ERR(file)) {
 		err = PTR_ERR(file);
 		goto err_inode;
 	}
 
-	file->f_flags |= O_LARGEFILE;
-	file->private_data = f;
-
-	kvm_get_kvm(kvm);
-	f->kvm = kvm;
-	xa_init(&f->bindings);
-	list_add(&f->entry, &GMEM_I(inode)->gmem_file_list);
-
 	fd_install(fd, file);
 	return fd;
 
 err_inode:
 	iput(inode);
-err_fops:
-	fops_put(&kvm_gmem_fops);
-err_gmem:
-	kfree(f);
 err_fd:
 	put_unused_fd(fd);
 	return err;
-- 
2.56.0.257.gf7497c7d27-goog

[RFC PATCH v3 2/5] KVM: guest_memfd: Add ioctl KVM_LINK_GUEST_MEMFD

[Tao Gong]

From: Ackerley Tng <ackerleytng@google.com>

A guest_memfd file is bound to the KVM instance that created it and
cannot be bound to another VM's memslots. Add KVM_LINK_GUEST_MEMFD to
link an existing guest_memfd's underlying inode to a new file (and fd)
bound to a target VM, allowing multiple VMs on the same host to use the
same underlying guest_memfd memory without copying (e.g. for intra-host
migration).

Only allow linking between distinct, compatible VMs (same VM type on x86)
so that private memory of a confidential VM cannot be mapped into an
incompatible VM, and serialize additions to the inode's gmem_file_list
with the mapping's invalidate_lock.

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Co-developed-by: Ryan Afranji <afranji@google.com>
Signed-off-by: Ryan Afranji <afranji@google.com>
Co-developed-by: Tao Gong <gongtao@google.com>
Signed-off-by: Tao Gong <gongtao@google.com>
---
 Documentation/virt/kvm/api.rst | 42 +++++++++++++++++
 arch/x86/kvm/x86.c             | 10 ++++
 include/linux/kvm_host.h       |  1 +
 include/uapi/linux/kvm.h       |  8 ++++
 virt/kvm/guest_memfd.c         | 83 ++++++++++++++++++++++++++++++++++
 virt/kvm/guest_memfd.h         |  1 +
 virt/kvm/kvm_main.c            | 10 ++++
 7 files changed, 155 insertions(+)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 3d6a4bf01c700..7c527a1e7c487 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6759,6 +6759,48 @@ if the guest_memfd memory was pinned in IOMMU page tables.
 
 See also: :ref:`KVM_SET_MEMORY_ATTRIBUTES`.
 
+4.147 KVM_LINK_GUEST_MEMFD
+--------------------------
+
+:Capability: KVM_CAP_GUEST_MEMFD_LINK
+:Architectures: all
+:Type: vm ioctl
+:Parameters: struct kvm_link_guest_memfd (in)
+:Returns: A file descriptor on success, <0 on error
+
+Errors:
+
+  ========== ===============================================================
+  EINVAL     ``fd`` is not an open guest_memfd file descriptor, ``flags`` or
+             ``reserved`` are not zero, the target VM is not compatible with
+             the VM ``fd`` is associated with (e.g. on x86, the VMs are of
+             different VM types), or ``fd`` is already associated with the
+             target VM.
+  EFAULT     The parameter address was invalid.
+  ENOMEM     Ran out of memory.
+  ========== ===============================================================
+
+::
+
+  struct kvm_link_guest_memfd {
+	__u64 fd;
+	__u64 flags;
+	__u64 reserved[6];
+  };
+
+KVM_LINK_GUEST_MEMFD creates a new guest_memfd file descriptor that is
+associated with the target VM (the VM on which the ioctl is invoked) and refers
+to the same underlying memory as the existing guest_memfd referred to by ``fd``,
+similar to how link() creates another name for an existing file.  The new file
+descriptor can be used in KVM_SET_USER_MEMORY_REGION2 for the target VM, and
+the memory's contents, size, flags and (with in-place conversion) memory
+attributes are shared by all linked guest_memfd instances.
+
+The primary use case is intra-host migration, where the destination VM needs to
+access the same guest memory as the source VM without copying it.
+
+No flags are defined yet; ``flags`` and ``reserved`` must be zero.
+
 .. _kvm_run:
 
 5. The kvm_run structure
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c081e4b9b25de..4427d32290c0f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10648,6 +10648,16 @@ bool kvm_arch_supports_gmem_init_shared(struct kvm *kvm)
 	return gmem_in_place_conversion || !kvm_arch_has_private_mem(kvm);
 }
 
+bool kvm_arch_gmem_link_allowed(struct kvm *dst, struct kvm *src)
+{
+	/*
+	 * Different VM types have different requirements for, and different
+	 * (vendor) state associated with, guest_memfd memory, e.g. memory that
+	 * is private to a TDX guest must never be mapped into a non-TDX VM.
+	 */
+	return dst->arch.vm_type == src->arch.vm_type;
+}
+
 #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT
 int kvm_arch_gmem_make_private(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 			       kvm_pfn_t nr_pages)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 485f18454eb45..5cffb7ad376e1 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2585,6 +2585,7 @@ static inline bool kvm_is_private_gfn(struct kvm *kvm, gfn_t gfn)
 #ifdef CONFIG_KVM_GUEST_MEMFD
 bool kvm_gmem_is_private_gfn(struct kvm *kvm, gfn_t gfn);
 bool kvm_arch_supports_gmem_init_shared(struct kvm *kvm);
+bool kvm_arch_gmem_link_allowed(struct kvm *dst, struct kvm *src);
 
 static inline u64 kvm_gmem_get_supported_flags(struct kvm *kvm)
 {
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 8dff2fc1972e9..53fe8dce57db2 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1690,4 +1690,12 @@ struct kvm_pre_fault_memory {
 	__u64 padding[5];
 };
 
+#define KVM_LINK_GUEST_MEMFD	_IOWR(KVMIO,  0xd6, struct kvm_link_guest_memfd)
+
+struct kvm_link_guest_memfd {
+	__u64 fd;
+	__u64 flags;
+	__u64 reserved[6];
+};
+
 #endif /* __LINUX_KVM_H */
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 3ca6f64d48040..54b11747a8c48 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -892,6 +892,15 @@ bool __weak kvm_arch_supports_gmem_init_shared(struct kvm *kvm)
 	return true;
 }
 
+/*
+ * Returns true if a guest_memfd instance associated with @src may be linked
+ * into @dst, i.e. if both VMs can safely share the underlying memory.
+ */
+bool __weak kvm_arch_gmem_link_allowed(struct kvm *dst, struct kvm *src)
+{
+	return kvm_arch_has_private_mem(dst) == kvm_arch_has_private_mem(src);
+}
+
 static int kvm_gmem_init_inode(struct inode *inode, loff_t size, u64 flags)
 {
 	struct gmem_inode *gi = GMEM_I(inode);
@@ -970,7 +979,10 @@ static struct file *kvm_gmem_alloc_file(struct kvm *kvm, struct inode *inode,
 	kvm_get_kvm(kvm);
 	f->kvm = kvm;
 	xa_init(&f->bindings);
+
+	filemap_invalidate_lock(inode->i_mapping);
 	list_add(&f->entry, &GMEM_I(inode)->gmem_file_list);
+	filemap_invalidate_unlock(inode->i_mapping);
 
 	return file;
 }
@@ -1026,6 +1038,77 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
 	return __kvm_gmem_create(kvm, size, flags);
 }
 
+int kvm_gmem_link(struct kvm *kvm, struct kvm_link_guest_memfd *args)
+{
+	static const char *name = "[kvm-gmem]";
+	u64 flags = args->flags;
+	u64 valid_flags = 0;
+	struct file *dst_file, *src_file;
+	struct gmem_file *gmem;
+	struct timespec64 ts;
+	struct inode *inode;
+	struct fd f;
+	int ret, fd, i;
+
+	if (flags & ~valid_flags)
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(args->reserved); i++) {
+		if (args->reserved[i])
+			return -EINVAL;
+	}
+
+	/* Don't let fdget() silently truncate the fd. */
+	if (args->fd > INT_MAX)
+		return -EINVAL;
+
+	f = fdget(args->fd);
+	src_file = fd_file(f);
+	if (!src_file)
+		return -EINVAL;
+
+	ret = -EINVAL;
+	if (src_file->f_op != &kvm_gmem_fops)
+		goto out;
+
+	/* Cannot link a gmem file with the same VM again. */
+	gmem = src_file->private_data;
+	if (gmem->kvm == kvm)
+		goto out;
+
+	if (!kvm_arch_gmem_link_allowed(kvm, gmem->kvm))
+		goto out;
+
+	fd = get_unused_fd_flags(0);
+	if (fd < 0) {
+		ret = fd;
+		goto out;
+	}
+
+	inode = file_inode(src_file);
+	ihold(inode);
+
+	dst_file = kvm_gmem_alloc_file(kvm, inode, name);
+	if (IS_ERR(dst_file)) {
+		iput(inode);
+		ret = PTR_ERR(dst_file);
+		goto out_fd;
+	}
+
+	ts = inode_set_ctime_current(inode);
+	inode_set_atime_to_ts(inode, ts);
+
+	fd_install(fd, dst_file);
+	fdput(f);
+	return fd;
+
+out_fd:
+	put_unused_fd(fd);
+out:
+	fdput(f);
+	return ret;
+}
+
 int kvm_gmem_prepare_memory_region(struct kvm *kvm, struct kvm_memory_slot *slot,
 				   unsigned int fd, uoff_t offset)
 {
diff --git a/virt/kvm/guest_memfd.h b/virt/kvm/guest_memfd.h
index 01bd359d27e30..7ffa4fa7337e9 100644
--- a/virt/kvm/guest_memfd.h
+++ b/virt/kvm/guest_memfd.h
@@ -8,6 +8,7 @@
 int kvm_gmem_init(struct module *module);
 void kvm_gmem_exit(void);
 int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args);
+int kvm_gmem_link(struct kvm *kvm, struct kvm_link_guest_memfd *args);
 int kvm_gmem_prepare_memory_region(struct kvm *kvm, struct kvm_memory_slot *slot,
 				   unsigned int fd, uoff_t offset);
 int kvm_gmem_commit_memory_region(struct kvm *kvm, struct kvm_memory_slot *slot);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 3ed481c4a15b6..70521371b64ab 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5453,6 +5453,16 @@ static long kvm_vm_ioctl(struct file *filp,
 		r = kvm_gmem_create(kvm, &guest_memfd);
 		break;
 	}
+	case KVM_LINK_GUEST_MEMFD: {
+		struct kvm_link_guest_memfd params;
+
+		r = -EFAULT;
+		if (copy_from_user(&params, argp, sizeof(params)))
+			goto out;
+
+		r = kvm_gmem_link(kvm, &params);
+		break;
+	}
 #endif
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
-- 
2.56.0.257.gf7497c7d27-goog

[RFC PATCH v3 3/5] KVM: guest_memfd: Advertise support for KVM_LINK_GUEST_MEMFD

[Tao Gong]

From: Vishal Annapurve <vannapurve@google.com>

Advertise KVM_CAP_GUEST_MEMFD_LINK via KVM_CHECK_EXTENSION so that
userspace VMMs and selftests can discover support for linking
guest_memfd instances across VMs via KVM_LINK_GUEST_MEMFD.

Signed-off-by: Vishal Annapurve <vannapurve@google.com>
Co-developed-by: Tao Gong <gongtao@google.com>
Signed-off-by: Tao Gong <gongtao@google.com>
---
 Documentation/virt/kvm/api.rst | 8 ++++++++
 include/uapi/linux/kvm.h       | 1 +
 virt/kvm/kvm_main.c            | 2 ++
 3 files changed, 11 insertions(+)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 7c527a1e7c487..bc54212e48c32 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -9738,6 +9738,14 @@ take care to differentiate between these cases.
 The presence of this capability indicates that the nested KVM guest can
 start in ESA mode.
 
+8.48 KVM_CAP_GUEST_MEMFD_LINK
+-----------------------------
+
+:Architectures: all
+
+The presence of this capability indicates that KVM supports linking an
+existing guest_memfd file to another VM via KVM_LINK_GUEST_MEMFD.
+
 9. Known KVM API problems
 =========================
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 53fe8dce57db2..09926fa85fe6c 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1000,6 +1000,7 @@ struct kvm_enable_cap {
 #define KVM_CAP_PPC_COMPAT_CAPS 250
 #define KVM_CAP_ARM_PMU_V3_STRICT 251
 #define KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES 252
+#define KVM_CAP_GUEST_MEMFD_LINK 253
 
 struct kvm_irq_routing_irqchip {
 	__u32 irqchip;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 70521371b64ab..135e7bb7609a8 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5008,6 +5008,8 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
 			return 0;
 
 		return KVM_MEMORY_ATTRIBUTE_PRIVATE;
+	case KVM_CAP_GUEST_MEMFD_LINK:
+		return 1;
 #endif
 	default:
 		break;
-- 
2.56.0.257.gf7497c7d27-goog

[RFC PATCH v3 4/5] KVM: selftests: Add tests for KVM_LINK_GUEST_MEMFD ioctl

[Tao Gong]

From: Ackerley Tng <ackerleytng@google.com>

Test that

+ Invalid inputs should be rejected with EINVAL
+ Successful inputs return a new (destination) fd
+ Destination and source fds have the same inode number
+ No crash on program exit

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Co-developed-by: Ryan Afranji <afranji@google.com>
Signed-off-by: Ryan Afranji <afranji@google.com>
Co-developed-by: Tao Gong <gongtao@google.com>
Signed-off-by: Tao Gong <gongtao@google.com>
---
 .../testing/selftests/kvm/guest_memfd_test.c  | 64 +++++++++++++++++++
 .../testing/selftests/kvm/include/kvm_util.h  | 18 ++++++
 2 files changed, 82 insertions(+)

diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index 2233d871a38f4..ccde4d735c77c 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -418,6 +418,68 @@ static void test_guest_memfd_flags(struct kvm_vm *vm)
 	}
 }
 
+static void test_link(struct kvm_vm *vm, int src_fd, size_t total_size)
+{
+	struct kvm_link_guest_memfd params = {};
+	struct stat src_stat, dst_stat;
+	struct kvm_vm *dst_vm;
+	int dst_fd, bad_fd;
+	int ret;
+
+	/* guest_memfd can only be linked between VMs of the same type. */
+	dst_vm = vm_create_barebones_type(vm->type);
+
+	/* Linking with an fd that isn't open */
+	bad_fd = dup(src_fd);
+	TEST_ASSERT(bad_fd >= 0, "dup() failed");
+	close(bad_fd);
+	ret = __vm_link_guest_memfd(dst_vm, bad_fd, 0);
+	TEST_ASSERT_EQ(ret, -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+
+	/* Linking with an out-of-range fd, i.e. KVM must not truncate fd */
+	params.fd = (1ULL << 32) | src_fd;
+	ret = __vm_ioctl(dst_vm, KVM_LINK_GUEST_MEMFD, &params);
+	TEST_ASSERT_EQ(ret, -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+
+	/* Linking with a non-gmem fd */
+	ret = __vm_link_guest_memfd(dst_vm, dst_vm->fd, 0);
+	TEST_ASSERT_EQ(ret, -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+
+	/* Linking with invalid flags */
+	ret = __vm_link_guest_memfd(dst_vm, src_fd, 1);
+	TEST_ASSERT_EQ(ret, -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+
+	/* Linking with non-zero reserved fields */
+	params.fd = src_fd;
+	params.reserved[5] = 1;
+	ret = __vm_ioctl(dst_vm, KVM_LINK_GUEST_MEMFD, &params);
+	TEST_ASSERT_EQ(ret, -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+
+	/* Linking with an already-associated vm */
+	ret = __vm_link_guest_memfd(vm, src_fd, 0);
+	TEST_ASSERT_EQ(ret, -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+
+	dst_fd = __vm_link_guest_memfd(dst_vm, src_fd, 0);
+	TEST_ASSERT(dst_fd >= 0, "linking should succeed with valid inputs");
+	TEST_ASSERT(src_fd != dst_fd, "linking should return a different fd");
+
+	ret = fstat(src_fd, &src_stat);
+	TEST_ASSERT_EQ(ret, 0);
+	ret = fstat(dst_fd, &dst_stat);
+	TEST_ASSERT_EQ(ret, 0);
+	TEST_ASSERT(src_stat.st_ino == dst_stat.st_ino,
+		    "src and dst files should have the same inode number");
+
+	close(dst_fd);
+	kvm_vm_free(dst_vm);
+}
+
 #define ____gmem_test(__test, __vm, __flags, __gmem_size, args...)	\
 do {									\
 	int fd = vm_create_guest_memfd(__vm, __gmem_size, __flags);	\
@@ -467,6 +529,8 @@ static void __test_guest_memfd(struct kvm_vm *vm, u64 flags)
 	gmem_test(fallocate, vm, flags);
 	gmem_test(invalid_punch_hole, vm, flags);
 	gmem_test_vm(invalid_binding, vm, flags);
+	if (kvm_has_cap(KVM_CAP_GUEST_MEMFD_LINK))
+		gmem_test_vm(link, vm, flags);
 }
 
 static void test_guest_memfd(unsigned long vm_type)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 777fa3dbf88d6..f8eca3f7993d0 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -787,6 +787,24 @@ static inline int vm_create_guest_memfd(struct kvm_vm *vm, u64 size,
 	return fd;
 }
 
+static inline int __vm_link_guest_memfd(struct kvm_vm *vm, int fd, u64 flags)
+{
+	struct kvm_link_guest_memfd params = {
+		.fd = fd,
+		.flags = flags,
+	};
+
+	return __vm_ioctl(vm, KVM_LINK_GUEST_MEMFD, &params);
+}
+
+static inline int vm_link_guest_memfd(struct kvm_vm *vm, int fd, u64 flags)
+{
+	int new_fd = __vm_link_guest_memfd(vm, fd, flags);
+
+	TEST_ASSERT(new_fd >= 0, KVM_IOCTL_ERROR(KVM_LINK_GUEST_MEMFD, new_fd));
+	return new_fd;
+}
+
 void vm_set_user_memory_region(struct kvm_vm *vm, u32 slot, u32 flags,
 			       gpa_t gpa, u64 size, void *hva);
 int __vm_set_user_memory_region(struct kvm_vm *vm, u32 slot, u32 flags,
-- 
2.56.0.257.gf7497c7d27-goog

[RFC PATCH v3 5/5] KVM: selftests: Test transferring private memory to another VM

[Tao Gong]

From: Ackerley Tng <ackerleytng@google.com>

Add private_mem_migrate_tests to verify that private memory backed by
guest_memfd can be linked into another VM of the same type
(KVM_X86_SW_PROTECTED_VM) on the same host via KVM_LINK_GUEST_MEMFD,
and that data written by the source VM is visible to the destination VM.

Also verify that linking a guest_memfd into a VM of a different VM type
is rejected with EINVAL.

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Co-developed-by: Ryan Afranji <afranji@google.com>
Signed-off-by: Ryan Afranji <afranji@google.com>
Co-developed-by: Tao Gong <gongtao@google.com>
Signed-off-by: Tao Gong <gongtao@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../kvm/x86/private_mem_migrate_tests.c       | 118 ++++++++++++++++++
 2 files changed, 119 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86/private_mem_migrate_tests.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 239bc61ea384f..cffa0d165a945 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -108,6 +108,7 @@ TEST_GEN_PROGS_x86 += x86/pmu_counters_test
 TEST_GEN_PROGS_x86 += x86/pmu_event_filter_test
 TEST_GEN_PROGS_x86 += x86/private_mem_conversions_test
 TEST_GEN_PROGS_x86 += x86/private_mem_kvm_exits_test
+TEST_GEN_PROGS_x86 += x86/private_mem_migrate_tests
 TEST_GEN_PROGS_x86 += x86/save_restore_pf_stress_test
 TEST_GEN_PROGS_x86 += x86/set_boot_cpu_id
 TEST_GEN_PROGS_x86 += x86/set_sregs_test
diff --git a/tools/testing/selftests/kvm/x86/private_mem_migrate_tests.c b/tools/testing/selftests/kvm/x86/private_mem_migrate_tests.c
new file mode 100644
index 0000000000000..17bebf850b4c4
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/private_mem_migrate_tests.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "kvm_util.h"
+#include "test_util.h"
+#include "ucall_common.h"
+#include <linux/kvm.h>
+#include <linux/sizes.h>
+
+#define TRANSFER_PRIVATE_MEM_TEST_SLOT 10
+#define TRANSFER_PRIVATE_MEM_GPA ((uint64_t)(1ull << 32))
+#define TRANSFER_PRIVATE_MEM_GVA TRANSFER_PRIVATE_MEM_GPA
+#define TRANSFER_PRIVATE_MEM_VALUE 0xdeadbeef
+
+static const struct vm_shape protected_shape = {
+	.mode = VM_MODE_DEFAULT,
+	.type = KVM_X86_SW_PROTECTED_VM,
+};
+
+static void transfer_private_mem_guest_code_src(void)
+{
+	uint64_t *ptr = (uint64_t *)TRANSFER_PRIVATE_MEM_GVA;
+
+	WRITE_ONCE(*ptr, TRANSFER_PRIVATE_MEM_VALUE);
+
+	GUEST_SYNC1(READ_ONCE(*ptr));
+}
+
+static void transfer_private_mem_guest_code_dst(void)
+{
+	uint64_t *ptr = (uint64_t *)TRANSFER_PRIVATE_MEM_GVA;
+
+	GUEST_SYNC1(READ_ONCE(*ptr));
+}
+
+static void add_private_mem(struct kvm_vm *vm, int memfd)
+{
+	vm_mem_add(vm, DEFAULT_VM_MEM_SRC, TRANSFER_PRIVATE_MEM_GPA,
+		   TRANSFER_PRIVATE_MEM_TEST_SLOT, 1, KVM_MEM_GUEST_MEMFD,
+		   memfd, 0, 0);
+
+	virt_map(vm, TRANSFER_PRIVATE_MEM_GVA, TRANSFER_PRIVATE_MEM_GPA, 1);
+
+	/*
+	 * guest_memfd memory is private by default if guest_memfd tracks
+	 * memory attributes, otherwise it must be converted via the VM.
+	 */
+	if (!kvm_has_gmem_attributes)
+		vm_mem_set_private(vm, TRANSFER_PRIVATE_MEM_GPA, SZ_4K);
+}
+
+static void test_transfer_private_mem(void)
+{
+	struct kvm_vm *src_vm, *dst_vm;
+	struct kvm_vcpu *src_vcpu, *dst_vcpu;
+	int src_memfd, dst_memfd;
+	struct ucall uc;
+
+	/* Build the source VM, use it to write to private memory */
+	src_vm = __vm_create_shape_with_one_vcpu(protected_shape, &src_vcpu, 0,
+						 transfer_private_mem_guest_code_src);
+	src_memfd = vm_create_guest_memfd(src_vm, SZ_4K, 0);
+
+	add_private_mem(src_vm, src_memfd);
+
+	vcpu_run(src_vcpu);
+	TEST_ASSERT_KVM_EXIT_REASON(src_vcpu, KVM_EXIT_IO);
+	get_ucall(src_vcpu, &uc);
+	TEST_ASSERT(uc.args[0] == TRANSFER_PRIVATE_MEM_VALUE,
+		    "Source VM should be able to write to private memory");
+
+	/* Build the destination VM with linked fd */
+	dst_vm = __vm_create_shape_with_one_vcpu(protected_shape, &dst_vcpu, 0,
+						 transfer_private_mem_guest_code_dst);
+	dst_memfd = vm_link_guest_memfd(dst_vm, src_memfd, 0);
+
+	add_private_mem(dst_vm, dst_memfd);
+
+	vcpu_run(dst_vcpu);
+	TEST_ASSERT_KVM_EXIT_REASON(dst_vcpu, KVM_EXIT_IO);
+	get_ucall(dst_vcpu, &uc);
+	TEST_ASSERT(uc.args[0] == TRANSFER_PRIVATE_MEM_VALUE,
+		    "Destination VM should be able to read value transferred");
+
+	close(dst_memfd);
+	close(src_memfd);
+	kvm_vm_free(dst_vm);
+	kvm_vm_free(src_vm);
+}
+
+static void test_link_different_vm_type(void)
+{
+	struct kvm_vm *src_vm, *dst_vm;
+	int src_memfd, ret;
+
+	src_vm = vm_create_barebones_type(KVM_X86_SW_PROTECTED_VM);
+	src_memfd = vm_create_guest_memfd(src_vm, SZ_4K, 0);
+
+	dst_vm = vm_create_barebones_type(KVM_X86_DEFAULT_VM);
+
+	ret = __vm_link_guest_memfd(dst_vm, src_memfd, 0);
+	TEST_ASSERT(ret == -1 && errno == EINVAL,
+		    "Linking into a VM of a different type should fail, ret: %d, errno: %d",
+		    ret, errno);
+
+	close(src_memfd);
+	kvm_vm_free(dst_vm);
+	kvm_vm_free(src_vm);
+}
+
+int main(int argc, char *argv[])
+{
+	TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM));
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_GUEST_MEMFD_LINK));
+
+	test_transfer_private_mem();
+	test_link_different_vm_type();
+
+	return 0;
+}
-- 
2.56.0.257.gf7497c7d27-goog

Re: [RFC PATCH v3 2/5] KVM: guest_memfd: Add ioctl KVM_LINK_GUEST_MEMFD

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] KVM: guest_memfd: multiple same-VM links to one inode in kvm_gmem_link()
- [High] KVM: x86: kvm_arch_gmem_link_allowed() allows unsafe TDX and SEV guest_memfd linking
--

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

KVM: guest_memfd: Add ioctl KVM_LINK_GUEST_MEMFD

This commit introduces the KVM_LINK_GUEST_MEMFD ioctl to link an existing
guest_memfd's underlying inode to a new file descriptor bound to a target VM,
allowing compatible VMs to share the same underlying guest_memfd memory.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c081e4b9b25de..4427d32290c0f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -10648,6 +10648,16 @@ bool kvm_arch_supports_gmem_init_shared(struct kvm *kvm)
>  	return gmem_in_place_conversion || !kvm_arch_has_private_mem(kvm);
>  }
>  
> +bool kvm_arch_gmem_link_allowed(struct kvm *dst, struct kvm *src)
> +{
> +	/*
> +	 * Different VM types have different requirements for, and different
> +	 * (vendor) state associated with, guest_memfd memory, e.g. memory that
> +	 * is private to a TDX guest must never be mapped into a non-TDX VM.
> +	 */
> +	return dst->arch.vm_type == src->arch.vm_type;
> +}

[Severity: High]
Is checking dst->arch.vm_type == src->arch.vm_type in
kvm_arch_gmem_link_allowed() sufficient for confidential VM types on x86?

On Intel TDX (KVM_X86_TDX_VM), each physical page in a guest_memfd is
exclusively owned in the TDX Module PAMT by a single TD's TDR and encrypted
with that TD's private HKID. If userspace links a guest_memfd between two TDX
VMs and both VMs populate or fault in the same private page via
tdx_mem_page_add() or tdx_mem_page_aug(), the second TD's SEAMCALL will fail
with TDX_PAGE_NOT_FREE and trigger a host WARN_ON_ONCE via TDX_BUG_ON_2():

arch/x86/kvm/vmx/tdx.c:tdx_mem_page_aug() {
	err = tdh_mem_page_aug(&kvm_tdx->td, gpa, level, pfn, &entry, &level_state);
	if (unlikely(tdx_operand_busy(err)))
		return -EBUSY;

	if (TDX_BUG_ON_2(err, TDH_MEM_PAGE_AUG, entry, level_state, kvm))
		return -EIO;
}

On AMD SEV-SNP (KVM_X86_SNP_VM), each physical page's hardware RMP entry
records a single (ASID, GPA) owner. If two SNP VMs link the same guest_memfd:

1. When dst faults in a private page already assigned in the RMP to src,
   sev_gmem_make_private() checks snp_lookup_rmpentry() without verifying the
   owning ASID or GPA, and returns 0, installing an NPT mapping in dst for a
   PFN owned by src's ASID:

arch/x86/kvm/svm/sev.c:sev_gmem_make_private() {
	rc = snp_lookup_rmpentry(pfn, &assigned, &level);
	...
	if (assigned) {
		pr_debug("%s: already assigned: gfn %llx pfn %llx nr_pages %llx level %d\n",
			 __func__, gfn, pfn, nr_pages, level);
		return 0;
	}
}

2. If src closes its guest_memfd and is destroyed while dst keeps the linked
   guest_memfd open, the shared inode's folios are not freed, so
   kvm_gmem_free_folio() -> sev_gmem_make_shared() does not run.
   sev_vm_destroy() then decommissions src's SNP context and frees src's ASID
   via sev_asid_free() while physical pages in the guest_memfd remain
   Assigned=1 to that freed/recycled ASID in the RMP.

3. If dst converts pages to shared or punches a hole while src has pages
   marked Immutable=1 in the RMP via KVM_SEV_SNP_LAUNCH_UPDATE,
   sev_gmem_make_shared() -> rmp_make_shared() fails and triggers WARN_ONCE().

In addition, for legacy SEV and SEV-ES VMs initialized via KVM_SEV_INIT or
KVM_SEV_ES_INIT, sev_guest_init() leaves kvm->arch.vm_type set to
KVM_X86_DEFAULT_VM, so kvm_arch_gmem_link_allowed() treats legacy SEV/SEV-ES
VMs and non-SEV default VMs as having the same vm_type.

Should kvm_arch_gmem_link_allowed() disallow linking when either VM is a
confidential VM (or only allow KVM_X86_DEFAULT_VM without SEV enabled)?

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 3ca6f64d48040..54b11747a8c48 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
[ ... ]
> @@ -970,7 +979,10 @@ static struct file *kvm_gmem_alloc_file(struct kvm *kvm, struct inode *inode,
>  	kvm_get_kvm(kvm);
>  	f->kvm = kvm;
>  	xa_init(&f->bindings);
> +
> +	filemap_invalidate_lock(inode->i_mapping);
>  	list_add(&f->entry, &GMEM_I(inode)->gmem_file_list);
> +	filemap_invalidate_unlock(inode->i_mapping);
>  
>  	return file;
>  }
> @@ -1026,6 +1038,77 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
>  	return __kvm_gmem_create(kvm, size, flags);
>  }
>  
> +int kvm_gmem_link(struct kvm *kvm, struct kvm_link_guest_memfd *args)
> +{
[ ... ]
> +	ret = -EINVAL;
> +	if (src_file->f_op != &kvm_gmem_fops)
> +		goto out;
> +
> +	/* Cannot link a gmem file with the same VM again. */
> +	gmem = src_file->private_data;
> +	if (gmem->kvm == kvm)
> +		goto out;
> +
> +	if (!kvm_arch_gmem_link_allowed(kvm, gmem->kvm))
> +		goto out;
> +
> +	fd = get_unused_fd_flags(0);
> +	if (fd < 0) {
> +		ret = fd;
> +		goto out;
> +	}
> +
> +	inode = file_inode(src_file);
> +	ihold(inode);
> +
> +	dst_file = kvm_gmem_alloc_file(kvm, inode, name);
> +	if (IS_ERR(dst_file)) {
> +		iput(inode);
> +		ret = PTR_ERR(dst_file);
> +		goto out_fd;
> +	}

[Severity: High]
Can kvm_gmem_link() still link the same guest_memfd inode to the same VM
multiple times?

In kvm_gmem_link(), the check only verifies whether src_file's immediate
gmem_file->kvm matches the target kvm, and does so without holding
filemap_invalidate_lock(inode->i_mapping).

If userspace creates fd1 on VM1 and links fd1 to VM2 (producing fd2), it can
then:
1. Call KVM_LINK_GUEST_MEMFD(fd1) on VM2 a second time,
2. Call KVM_LINK_GUEST_MEMFD(fd2) back onto VM1, or
3. Call KVM_LINK_GUEST_MEMFD(fd1) concurrently from two threads on VM2.

In all three cases, gmem->kvm == kvm is false, so kvm_gmem_alloc_file()
inserts a second struct gmem_file for the same struct kvm onto
GMEM_I(inode)->gmem_file_list.

Having multiple struct gmem_file instances for the same VM on the same inode
breaks two assumptions elsewhere in guest_memfd.c:

First, kvm_gmem_commit_memory_region() only checks f->bindings on the single
gmem_file being bound:

virt/kvm/guest_memfd.c:kvm_gmem_commit_memory_region() {
	if (!xa_empty(&f->bindings) &&
	    xa_find(&f->bindings, &start, end - 1, XA_PRESENT)) {
		filemap_invalidate_unlock(inode->i_mapping);
		return -EEXIST;
	}
}

With two gmem_file fds for the same VM and inode, a single VM can bind two
memslots at different GPAs to the exact same pgoff range of the same
guest_memfd inode.

Second, kvm_gmem_invalidate_start() iterates over gmem_file_list and calls
__kvm_gmem_invalidate_start() per gmem_file, which acquires and releases
KVM_MMU_LOCK(kvm) per gmem_file while mmu_invalidate_in_progress is active.

Should GMEM_I(inode)->gmem_file_list be walked while holding
filemap_invalidate_lock(inode->i_mapping) to reject any link where an entry
with f->kvm == kvm already exists on the inode?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026064424.3003536-1-gongtao@google.com?part=2

Re: [RFC PATCH v3 3/5] KVM: guest_memfd: Advertise support for KVM_LINK_GUEST_MEMFD

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: guest_memfd: disallow linking guest_memfd across TDX and SEV-SNP VMs
--

commit 22378b77744900b2602bc894dbec906d31acf8be
Author: Vishal Annapurve <vannapurve@google.com>

KVM: guest_memfd: Advertise support for KVM_LINK_GUEST_MEMFD

This commit introduces KVM_CAP_GUEST_MEMFD_LINK and advertises it via
KVM_CHECK_EXTENSION so userspace can discover support for linking
guest_memfd instances across VMs with KVM_LINK_GUEST_MEMFD.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 70521371b64ab..135e7bb7609a8 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -5008,6 +5008,8 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
>  			return 0;
>  
>  		return KVM_MEMORY_ATTRIBUTE_PRIVATE;
> +	case KVM_CAP_GUEST_MEMFD_LINK:
> +		return 1;

[Severity: High]
Should KVM_CAP_GUEST_MEMFD_LINK and kvm_arch_gmem_link_allowed() restrict
linking guest_memfd instances for confidential VM types such as TDX and
SEV-SNP?

Right now, kvm_vm_ioctl_check_extension_generic() unconditionally returns 1
for all VM instances, and on x86, kvm_gmem_link() ->
kvm_arch_gmem_link_allowed() only checks whether the two VMs have the same
vm_type:

arch/x86/kvm/x86.c:kvm_arch_gmem_link_allowed() {
	return dst->arch.vm_type == src->arch.vm_type;
}

This allows userspace to link a guest_memfd between two KVM_X86_TDX_VM
instances, two KVM_X86_SNP_VM instances, or between a legacy SEV VM
(initialized via KVM_SEV_INIT on a KVM_X86_DEFAULT_VM without changing
kvm->arch.vm_type) and a default VM:

1. For KVM_X86_TDX_VM, private guest_memfd pages are encrypted with a
per-TD ephemeral HKID and tracked in the TDX Module PAMT as exclusively
owned by a single TD. If two TDX VMs link a guest_memfd and both map the
same private PFN via KVM_TDX_INIT_MEM_REGION -> tdx_mem_page_add() or a
stage-2 EPT fault -> tdx_mem_page_aug(), the second TD's SEAMCALL fails
with TDX_PAGE_NOT_FREE:

arch/x86/kvm/vmx/tdx.c:tdx_mem_page_aug() {
	err = tdh_mem_page_aug(&kvm_tdx->td, gpa, level, pfn, &entry, &level_state);
	if (unlikely(tdx_operand_busy(err)))
		return -EBUSY;

	if (TDX_BUG_ON_2(err, TDH_MEM_PAGE_AUG, entry, level_state, kvm))
		return -EIO;
}

Because TDX_PAGE_NOT_FREE is not an operand-busy error, TDX_BUG_ON_2()
triggers a host WARN_ON_ONCE().

2. For KVM_X86_SNP_VM, each physical page has a single system-wide RMP
entry binding it to one (ASID, GPA) pair. When a second SNP VM faults in a
private guest_memfd page already assigned in the RMP to the first SNP VM,
sev_gmem_make_private() sees assigned == true from snp_lookup_rmpentry()
and returns 0 without checking whether the RMP entry belongs to the current
VM's ASID:

arch/x86/kvm/svm/sev.c:sev_gmem_make_private() {
	if (assigned) {
		pr_debug("%s: already assigned: gfn %llx pfn %llx nr_pages %llx level %d\n",
			 __func__, gfn, pfn, nr_pages, level);
		return 0;
	}
}

KVM then installs the page into the second SNP VM's NPT, leading to
repeated guest RMP #NPF faults; furthermore, if the first SNP VM is
destroyed while the second SNP VM keeps the linked guest_memfd alive,
sev_vm_destroy() frees the first VM's ASID via sev_asid_free() while the
shared guest_memfd pages remain assigned to that ASID in the RMP table.

Should KVM_CAP_GUEST_MEMFD_LINK and kvm_arch_gmem_link_allowed() disallow
linking for TDX and SEV/SNP VMs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026064424.3003536-1-gongtao@google.com?part=3

Re: [RFC PATCH v3 4/5] KVM: selftests: Add tests for KVM_LINK_GUEST_MEMFD ioctl

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: selftests: test_link() hits vm->type assert in ____vm_create() on arm64
--

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

KVM: selftests: Add tests for KVM_LINK_GUEST_MEMFD ioctl

This commit adds kselftest helpers and test cases in guest_memfd_test.c for
the KVM_LINK_GUEST_MEMFD ioctl, covering invalid inputs and valid linking
between VMs.

> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 2233d871a38f4..ccde4d735c77c 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> @@ -418,6 +418,68 @@ static void test_guest_memfd_flags(struct kvm_vm *vm)
>  	}
>  }
>  
> +static void test_link(struct kvm_vm *vm, int src_fd, size_t total_size)
> +{
> +	struct kvm_link_guest_memfd params = {};
> +	struct stat src_stat, dst_stat;
> +	struct kvm_vm *dst_vm;
> +	int dst_fd, bad_fd;
> +	int ret;
> +
> +	/* guest_memfd can only be linked between VMs of the same type. */
> +	dst_vm = vm_create_barebones_type(vm->type);

[Severity: Medium]
Will passing vm->type to vm_create_barebones_type() here trigger an
assertion failure in ____vm_create() on arm64 when vm->pa_bits != 40?

On arm64, main() invokes test_guest_memfd(VM_TYPE_DEFAULT) with vm_type = 0,
and the initial vm_create_barebones_type(vm_type) call in test_guest_memfd()
reaches tools/testing/selftests/kvm/lib/kvm_util.c:____vm_create():

#ifdef __aarch64__
	TEST_ASSERT(!vm->type, "ARM doesn't support test-provided types");
	if (vm->pa_bits != 40)
		vm->type = KVM_VM_TYPE_ARM_IPA_SIZE(vm->pa_bits);
#endif

When the selected default guest mode has a physical address width other than
40 bits (for example on hosts where guest_modes_append_default() selects a
48-bit or 52-bit IPA mode), ____vm_create() overwrites vm->type in place
with a non-zero KVM_VM_TYPE_ARM_IPA_SIZE(vm->pa_bits) value.

When __test_guest_memfd() then calls test_link(), passing that mutated
vm->type into vm_create_barebones_type(vm->type) forwards a non-zero
shape.type into ____vm_create(), tripping TEST_ASSERT(!vm->type, ...).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026064424.3003536-1-gongtao@google.com?part=4