[PATCH 0/3] guest_memfd fixes for bind and populate

[PATCH 0/3] guest_memfd fixes for bind and populate

[Ackerley Tng via B4 Relay]

This series is a group of fixes for the bind and populate flows for
guest_memfd, and fixes some issues reported by Sashiko after reviewing the
guest_memfd in-place conversions series [1] and another fixup series Sean
posted [3].

Sashiko pointed out

+ Possible write to read-only page [1]
    => Fixed in patch 1
+ Signed integer overflow in kvm_gmem_bind() twice: [2][3]
    => Fixed in patch 2
+ Unchecked xa_store_range() [3]
    => Fixed in patch 3

[1] https://lore.kernel.org/all/CA+EHjTwrygfMrZZSw4y7-ry8fidW2x0C7iuF2Q=dnPNHUmNtUg@mail.gmail.com/
[2] https://lore.kernel.org/all/CA+EHjTxcadguOfOo7RpJVtAzcY5JAFZTbrAT_wcN6akMi8gCUg@mail.gmail.com/
[3] https://lore.kernel.org/all/20260522180530.EE9101F00A3E@smtp.kernel.org/

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
Ackerley Tng (1):
      KVM: guest_memfd: Handle errors from xa_store_range() when binding

Sean Christopherson (2):
      KVM: guest_memfd: Use write permissions when GUP-ing source pages
      KVM: guest_memfd: Fix possible signed integer overflow

 arch/x86/kvm/svm/sev.c   |  1 +
 arch/x86/kvm/vmx/tdx.c   |  2 +-
 include/linux/kvm_host.h |  3 ++-
 virt/kvm/guest_memfd.c   | 18 ++++++++++--------
 virt/kvm/kvm_mm.h        |  2 +-
 5 files changed, 15 insertions(+), 11 deletions(-)
---
base-commit: b7fbe9a1bf9ee6c967ef77d366ca58c35fcf1887
change-id: 20260522-fix-sev-gmem-post-populate-a36bef7f0698

Best regards,
--
Ackerley Tng <ackerleytng@google.com>

[PATCH 1/3] KVM: guest_memfd: Use write permissions when GUP-ing source pages

[Ackerley Tng via B4 Relay]

From: Sean Christopherson <seanjc@google.com>

sev_gmem_post_populate() may write to the source page if there was an error
while performing SNP_LAUNCH_UPDATE.

Since GUP requested only reads, there is a chance sev_gmem_post_populate()
could be writing to some read-only page.

sev_gmem_post_populate() will only ever write the source page if the type
of page being LAUNCH_UPDATEd is a CPUID page. Hence, request a writable
page only when loading the CPUID page.

Since TDX never writes to the source page, always pass false to
kvm_gmem_populate().

With this, even if a read-only mapping or the global zero page was provided
as the source page, GUP will do a copy-on-write, making it writable before
the write happens in gvm_post_populate.

Fixes: 2a62345b30529 ("KVM: guest_memfd: GUP source pages prior to populating guest memory")
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
 arch/x86/kvm/svm/sev.c   | 1 +
 arch/x86/kvm/vmx/tdx.c   | 2 +-
 include/linux/kvm_host.h | 3 ++-
 virt/kvm/guest_memfd.c   | 6 ++++--
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 940b97d4a8523..2f254c447923e 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2469,6 +2469,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	sev_populate_args.type = params.type;
 
 	count = kvm_gmem_populate(kvm, params.gfn_start, src, npages,
+				  params.type == KVM_SEV_SNP_PAGE_TYPE_CPUID,
 				  sev_gmem_post_populate, &sev_populate_args);
 	if (count < 0) {
 		argp->error = sev_populate_args.fw_error;
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index b8c3d3d8bbfe5..00dcfcbc47f68 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -3185,7 +3185,7 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *c
 		};
 		gmem_ret = kvm_gmem_populate(kvm, gpa_to_gfn(region.gpa),
 					     u64_to_user_ptr(region.source_addr),
-					     1, tdx_gmem_post_populate, &arg);
+					     1, false, tdx_gmem_post_populate, &arg);
 		if (gmem_ret < 0) {
 			ret = gmem_ret;
 			break;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 4c14aee1fb063..2c5ad9a6d5ce8 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2596,7 +2596,8 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord
 typedef int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 				    struct page *page, void *opaque);
 
-long kvm_gmem_populate(struct kvm *kvm, gfn_t gfn, void __user *src, long npages,
+long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src,
+		       long npages, bool may_writeback_src,
 		       kvm_gmem_populate_cb post_populate, void *opaque);
 #endif
 
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 69c9d6d546b28..07d8db344872b 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -858,7 +858,8 @@ static long __kvm_gmem_populate(struct kvm *kvm, struct kvm_memory_slot *slot,
 	return ret;
 }
 
-long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long npages,
+long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src,
+		       long npages, bool may_writeback_src,
 		       kvm_gmem_populate_cb post_populate, void *opaque)
 {
 	struct kvm_memory_slot *slot;
@@ -892,8 +893,9 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
 
 		if (src) {
 			unsigned long uaddr = (unsigned long)src + i * PAGE_SIZE;
+			unsigned int flags = may_writeback_src ? FOLL_WRITE : 0;
 
-			ret = get_user_pages_fast(uaddr, 1, 0, &src_page);
+			ret = get_user_pages_fast(uaddr, 1, flags, &src_page);
 			if (ret < 0)
 				break;
 			if (ret != 1) {

-- 
2.54.0.794.g4f17f83d09-goog

[PATCH 2/3] KVM: guest_memfd: Fix possible signed integer overflow

[Ackerley Tng via B4 Relay]

From: Sean Christopherson <seanjc@google.com>

The caller, kvm_set_memory_region(), checks for an overflow in an unsigned
u64 guest_memfd_offset. When guest_memfd_offset is passed to kvm_gmem_bind,
it is cast into a signed 64-bit integer.

Hence, a large 64-bit offset could result in a negative loff_t, which could
result in the overflow checks failing.

Make kvm_gmem_bind() take u64 instead of loff_t to consistently deal with
unsigned values to avoid this issue.

Fixes: a7800aa80ea4d ("KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory")
Signed-off-by: Sean Christopherson <seanjc@google.com>
[Use size_t for size instead of u64]
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
 virt/kvm/guest_memfd.c | 7 +++----
 virt/kvm/kvm_mm.h      | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 07d8db344872b..d203135969d13 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -640,9 +640,9 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
 }
 
 int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
-		  unsigned int fd, loff_t offset)
+		  unsigned int fd, u64 offset)
 {
-	loff_t size = slot->npages << PAGE_SHIFT;
+	size_t size = slot->npages << PAGE_SHIFT;
 	unsigned long start, end;
 	struct gmem_file *f;
 	struct inode *inode;
@@ -664,8 +664,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
 
 	inode = file_inode(file);
 
-	if (offset < 0 || !PAGE_ALIGNED(offset) ||
-	    offset + size > i_size_read(inode))
+	if (!PAGE_ALIGNED(offset) || offset + size > i_size_read(inode))
 		goto err;
 
 	filemap_invalidate_lock(inode->i_mapping);
diff --git a/virt/kvm/kvm_mm.h b/virt/kvm/kvm_mm.h
index 9fcc5d5b7f8d0..23813d74ce709 100644
--- a/virt/kvm/kvm_mm.h
+++ b/virt/kvm/kvm_mm.h
@@ -72,7 +72,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_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
-		  unsigned int fd, loff_t offset);
+		  unsigned int fd, u64 offset);
 void kvm_gmem_unbind(struct kvm_memory_slot *slot);
 #else
 static inline int kvm_gmem_init(struct module *module)

-- 
2.54.0.794.g4f17f83d09-goog

[PATCH 3/3] KVM: guest_memfd: Handle errors from xa_store_range() when binding

[Ackerley Tng via B4 Relay]

From: Ackerley Tng <ackerleytng@google.com>

Unhandled errors from xa_store_range() means kvm_gmem_bind() might falsely
reporting success, leading to false assumptions in guest_memfd's lifecycle
later.

Handle these errors by checking and returning the error to the userspace.

Fixes: a7800aa80ea4d ("KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory")
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
 virt/kvm/guest_memfd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index d203135969d13..104f0f3d6a0b3 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -648,6 +648,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
 	struct inode *inode;
 	struct file *file;
 	int r = -EINVAL;
+	void *result;
 
 	BUILD_BUG_ON(sizeof(gfn_t) != sizeof(slot->gmem.pgoff));
 
@@ -688,7 +689,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
 	if (kvm_gmem_supports_mmap(inode))
 		slot->flags |= KVM_MEMSLOT_GMEM_ONLY;
 
-	xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
+	result = xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
 	filemap_invalidate_unlock(inode->i_mapping);
 
 	/*
@@ -696,7 +697,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
 	 * not the other way 'round.  Active bindings are invalidated if the
 	 * file is closed before memslots are destroyed.
 	 */
-	r = 0;
+	r = xa_is_err(result) ? xa_err(result) : 0;
 err:
 	fput(file);
 	return r;

-- 
2.54.0.794.g4f17f83d09-goog

Re: [PATCH 1/3] KVM: guest_memfd: Use write permissions when GUP-ing source pages

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Modifications to the pinned user space source page (`src_page`) are not tracked with `set_page_dirty_lock()`, risking silent data loss if the page is evicted.
- [High] Userspace can maliciously trigger a `WARN_ON_ONCE` kernel panic by passing a zero `uaddr` for non-ZERO page types, leading to a potential denial of service.
--

commit 6a7184de221e96f7e4e45a9996f99d0f23dabd6a
Author: Sean Christopherson <seanjc@google.com>

KVM: guest_memfd: Use write permissions when GUP-ing source pages

sev_gmem_post_populate() may write to the source page if there was an error
while performing SNP_LAUNCH_UPDATE.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2469,6 +2469,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	sev_populate_args.type = params.type;
>  
>  	count = kvm_gmem_populate(kvm, params.gfn_start, src, npages,
> +				  params.type == KVM_SEV_SNP_PAGE_TYPE_CPUID,
>  				  sev_gmem_post_populate, &sev_populate_args);

[Severity: High]
This is a pre-existing issue, but could userspace intentionally trigger a
WARN_ON_ONCE kernel panic here by passing a zero uaddr for non-ZERO page
types?

In snp_launch_update(), if userspace provides params.uaddr = 0 alongside a
non-ZERO page type (e.g., KVM_SEV_SNP_PAGE_TYPE_CPUID), src is set to NULL
because 0 bypasses the PAGE_ALIGNED(src) validation.

When kvm_gmem_populate() is called with src = NULL, it skips
get_user_pages_fast() and leaves src_page as NULL. The __kvm_gmem_populate()
function then invokes the sev_gmem_post_populate() callback with
src_page = NULL, which triggers the WARN_ON_ONCE in the callback. On
production systems configured with panic_on_warn=1, this could allow an
unprivileged local user to crash the host.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
[ ... ]
> @@ -892,8 +893,9 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
>  
>  		if (src) {
>  			unsigned long uaddr = (unsigned long)src + i * PAGE_SIZE;
> +			unsigned int flags = may_writeback_src ? FOLL_WRITE : 0;
>  
> -			ret = get_user_pages_fast(uaddr, 1, 0, &src_page);
> +			ret = get_user_pages_fast(uaddr, 1, flags, &src_page);

[Severity: High]
This is a pre-existing issue, but does this miss tracking modifications to
the pinned user space source page (src_page)?

While the FOLL_WRITE flag correctly handles the copy-on-write,
in sev_gmem_post_populate(), if the SEV-SNP firmware returns an invalid
parameter error for a CPUID page, KVM copies the expected values into the
user-provided src_page via kmap_local_page() and memcpy().

However, kvm_gmem_populate() uses get_user_pages_fast() to pin the page
and subsequently calls put_page(src_page) without explicitly marking the
page as dirty (e.g., using set_page_dirty_lock()).

Because the modification bypasses the user's page tables and directly
writes to the kernel's direct map, the user PTE's hardware dirty bit
remains unset. If the page is file-backed or swap-backed and memory
pressure forces it to be reclaimed, the memory subsystem might see both
the PTE and the struct page as clean, discarding the page without writing
the changes to the backing store. This would cause the user-space process
to silently read stale CPUID data.

>  			if (ret < 0)
>  				break;
>  			if (ret != 1) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-fix-sev-gmem-post-populate-v1-0-9fc8d6437b65@google.com?part=1

Re: [PATCH 2/3] KVM: guest_memfd: Fix possible signed integer overflow

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The fallback stub definition of `kvm_gmem_bind()` for `!CONFIG_KVM_GUEST_MEMFD` was not updated to reflect the new `u64 offset` parameter type.
--

commit d25bb3b9adbe143acc8b3b7339c4bf637a62df32
Author: Sean Christopherson <seanjc@google.com>

KVM: guest_memfd: Fix possible signed integer overflow

This commit changes the type of the offset parameter in kvm_gmem_bind() from
loff_t to u64 to prevent potential signed integer overflow issues when passing
large 64-bit offsets. It also updates the local size variable to size_t.

> diff --git a/virt/kvm/kvm_mm.h b/virt/kvm/kvm_mm.h
> index 9fcc5d5b7f8d0..23813d74ce709 100644
> --- a/virt/kvm/kvm_mm.h
> +++ b/virt/kvm/kvm_mm.h
> @@ -72,7 +72,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_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
> -		  unsigned int fd, loff_t offset);
> +		  unsigned int fd, u64 offset);
>  void kvm_gmem_unbind(struct kvm_memory_slot *slot);
>  #else
>  static inline int kvm_gmem_init(struct module *module)

[Severity: Low]
Does the fallback stub definition of kvm_gmem_bind() for !CONFIG_KVM_GUEST_MEMFD
also need to be updated to match this new signature?

It looks like the static inline stub in the #else block might still be expecting
loff_t offset, which could cause an inconsistent API signature and potentially
trigger compiler warnings.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-fix-sev-gmem-post-populate-v1-0-9fc8d6437b65@google.com?part=2

Re: [PATCH 3/3] KVM: guest_memfd: Handle errors from xa_store_range() when binding

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Propagating `xa_store_range()` failure causes a Use-After-Free due to a missing rollback of partially stored XArray entries.
--

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

KVM: guest_memfd: Handle errors from xa_store_range() when binding

This commit updates kvm_gmem_bind() to check the return value of
xa_store_range() and propagate any resulting errors back to userspace,
rather than falsely reporting success.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index d203135969d13..104f0f3d6a0b3 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
[ ... ]
> @@ -688,7 +689,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
>  	if (kvm_gmem_supports_mmap(inode))
>  		slot->flags |= KVM_MEMSLOT_GMEM_ONLY;
>  
> -	xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
> +	result = xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
>  	filemap_invalidate_unlock(inode->i_mapping);
>  
>  	/*
> @@ -696,7 +697,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
>  	 * not the other way 'round.  Active bindings are invalidated if the
>  	 * file is closed before memslots are destroyed.
>  	 */
> -	r = 0;
> +	r = xa_is_err(result) ? xa_err(result) : 0;

[Severity: Critical]
If xa_store_range() fails midway through storing a large range (for example,
returning -ENOMEM), does it leave the already-processed entries in the
f->bindings XArray?

When this error is propagated back, the caller __kvm_set_memory_region()
will abort the operation and free the memslot without calling
kvm_gmem_unbind().

Since the partial XArray updates aren't rolled back here, could this leave
dangling pointers to the freed memslot in f->bindings? If so, when the file
is eventually closed, kvm_gmem_release() might iterate over these dangling
pointers and write to slot->gmem.file, resulting in a use-after-free.

>  err:
>  	fput(file);
>  	return r;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-fix-sev-gmem-post-populate-v1-0-9fc8d6437b65@google.com?part=3

Re: [PATCH 2/3] KVM: guest_memfd: Fix possible signed integer overflow

[Sean Christopherson]

On Fri, May 22, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > diff --git a/virt/kvm/kvm_mm.h b/virt/kvm/kvm_mm.h
> > index 9fcc5d5b7f8d0..23813d74ce709 100644
> > --- a/virt/kvm/kvm_mm.h
> > +++ b/virt/kvm/kvm_mm.h
> > @@ -72,7 +72,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_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
> > -		  unsigned int fd, loff_t offset);
> > +		  unsigned int fd, u64 offset);
> >  void kvm_gmem_unbind(struct kvm_memory_slot *slot);
> >  #else
> >  static inline int kvm_gmem_init(struct module *module)
> 
> [Severity: Low]
> Does the fallback stub definition of kvm_gmem_bind() for !CONFIG_KVM_GUEST_MEMFD
> also need to be updated to match this new signature?

Yes.