[PATCH 0/4] kvm: sev: Fix issues reported by Sashiko

[PATCH 0/4] kvm: sev: Fix issues reported by Sashiko

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Hi,

On the post of my direct-VMSA patch-set Sashiko reported a few real
pre-existing issues in the SEV-SNP launch_update code. This patch-set
addresses three of them:

	* Fix user-triggerable WARN_ON on LAUNCH_UPDATE path.

	* Check that CPUID pages are writable before writing error
	  information to it.

	* Fix kunmap_local() order.

Please review.

-Joerg

Joerg Roedel (4):
  kvm: sev: Fix user-space triggerable WARN_ON on snp_launch_update path
  kvm: sev: Unmap pages in correct order in sev_gmem_post_populate()
  KVM: guest_memfd: Add `write` parameter to kvm_gmem_populate()
  kvm: sev: Acquire a writeable page reference for CPUID pages

 arch/x86/kvm/svm/sev.c   | 15 +++++++++++++--
 arch/x86/kvm/vmx/tdx.c   |  2 +-
 include/linux/kvm_host.h |  4 +++-
 virt/kvm/guest_memfd.c   |  4 ++--
 4 files changed, 19 insertions(+), 6 deletions(-)

-- 
2.53.0

[PATCH 1/4] kvm: sev: Fix user-space triggerable WARN_ON on snp_launch_update path

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Sashiko reported on an unrelated patch:

  [Severity: High]
  This is a pre-existing issue, but can a host userspace process trigger a
  kernel warning by passing a NULL user address (uaddr = 0) here?

  If params.uaddr is 0, src becomes NULL and passes the PAGE_ALIGNED(src)
  check. kvm_gmem_populate() skips fetching the user page and passes
  src_page = NULL to sev_gmem_post_populate().

  That function then unconditionally evaluates:

  WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO &&
               !src_page)

  Since the type isn't ZERO, won't this allow an unprivileged user to spam
  the kernel log?

The assessment is correct, so check for this condition earlier in the
snp_launch_update() path to avoid the WARN_ON_ONCE.

Fixes: dee5a47cc7a45 ("KVM: SEV: Add KVM_SEV_SNP_LAUNCH_UPDATE command")
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm/sev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 6c6a6d663e29..41dcba5180ca 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2438,6 +2438,13 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	if (!PAGE_ALIGNED(src))
 		return -EINVAL;
 
+	/*
+	 * Make sure user-mode did not pass NULL as src with
+	 * type != KVM_SEV_SNP_PAGE_TYPE_ZERO.
+	 */
+	if (src == NULL && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
+		return -EINVAL;
+
 	npages = params.len / PAGE_SIZE;
 
 	/*
-- 
2.53.0

[PATCH 2/4] kvm: sev: Unmap pages in correct order in sev_gmem_post_populate()

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

The kmap_local() interface requires unmapping of pages in reverse
order of mapping.

Fixes: 2a62345b3052 ("KVM: guest_memfd: GUP source pages prior to populating guest memory")
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm/sev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 41dcba5180ca..f09d15f68964 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2360,8 +2360,8 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 
 		memcpy(dst_vaddr, src_vaddr, PAGE_SIZE);
 
-		kunmap_local(src_vaddr);
 		kunmap_local(dst_vaddr);
+		kunmap_local(src_vaddr);
 	}
 
 	ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,
-- 
2.53.0

[PATCH 3/4] KVM: guest_memfd: Add `write` parameter to kvm_gmem_populate()

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

The call-path of kvm_gmem_populate() might subsequently write to the
page provided by user-space. This is used to provide detailed error
information in case the page population failed.

But since kvm_gmem_populate() only acquires a read-only reference to
the user-space page via get_user_pages_fast(), the error information
might be written to a read-only page later on.

Add a parameter to kvm_gmem_populate() to optionally acquire a
writeable reference to the source page to make sure page permissions
can be enforced.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm/sev.c   | 2 +-
 arch/x86/kvm/vmx/tdx.c   | 2 +-
 include/linux/kvm_host.h | 4 +++-
 virt/kvm/guest_memfd.c   | 4 ++--
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f09d15f68964..dab8109edf26 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2475,7 +2475,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	sev_populate_args.sev_fd = argp->sev_fd;
 	sev_populate_args.type = params.type;
 
-	count = kvm_gmem_populate(kvm, params.gfn_start, src, npages,
+	count = kvm_gmem_populate(kvm, params.gfn_start, src, npages, 0,
 				  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 04ce321ebdf3..46b1d84fddf2 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, 0, 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 4c14aee1fb06..622c0b04d8c3 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2581,6 +2581,8 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord
  *       (passed to @post_populate, and incremented on each iteration
  *       if not NULL). Must be page-aligned.
  * @npages: number of pages to copy from userspace-buffer
+ * @write: user-space provided buffer must be writable. The function
+ *	 will acquire a writable reference when set to 1.
  * @post_populate: callback to issue for each gmem page that backs the GPA
  *                 range
  * @opaque: opaque data to pass to @post_populate callback
@@ -2597,7 +2599,7 @@ 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,
-		       kvm_gmem_populate_cb post_populate, void *opaque);
+		       int write, kvm_gmem_populate_cb post_populate, void *opaque);
 #endif
 
 #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 69c9d6d546b2..7a245a402a1b 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -859,7 +859,7 @@ static long __kvm_gmem_populate(struct kvm *kvm, struct kvm_memory_slot *slot,
 }
 
 long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long npages,
-		       kvm_gmem_populate_cb post_populate, void *opaque)
+		       int write, kvm_gmem_populate_cb post_populate, void *opaque)
 {
 	struct kvm_memory_slot *slot;
 	int ret = 0;
@@ -893,7 +893,7 @@ 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;
 
-			ret = get_user_pages_fast(uaddr, 1, 0, &src_page);
+			ret = get_user_pages_fast(uaddr, 1, write, &src_page);
 			if (ret < 0)
 				break;
 			if (ret != 1) {
-- 
2.53.0

[PATCH 4/4] kvm: sev: Acquire a writeable page reference for CPUID pages

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

When the PSP checks on a user-provided CPUID page fail KVM will write
back the detailed error information to the user-provided buffer.

Make sure this buffer is actually writable to not write the errors to
a read-only page.

Fixes: 2a62345b3052 ("KVM: guest_memfd: GUP source pages prior to populating guest memory")
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm/sev.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index dab8109edf26..5fd08d34be3f 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2415,6 +2415,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	struct kvm_memory_slot *memslot;
 	long npages, count;
 	void __user *src;
+	int write;
 
 	if (!sev_snp_guest(kvm) || !sev->snp_context)
 		return -EINVAL;
@@ -2475,7 +2476,10 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	sev_populate_args.sev_fd = argp->sev_fd;
 	sev_populate_args.type = params.type;
 
-	count = kvm_gmem_populate(kvm, params.gfn_start, src, npages, 0,
+	/* Acquire a write-reference for CPUID pages as kernel might write to it */
+	write = params.type == KVM_SEV_SNP_PAGE_TYPE_CPUID;
+
+	count = kvm_gmem_populate(kvm, params.gfn_start, src, npages, write,
 				  sev_gmem_post_populate, &sev_populate_args);
 	if (count < 0) {
 		argp->error = sev_populate_args.fw_error;
-- 
2.53.0

Re: [PATCH 2/4] kvm: sev: Unmap pages in correct order in sev_gmem_post_populate()

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch corrects the `kunmap_local()` LIFO unmapping order in the initial setup path but leaves an identical unmapping violation in the CPUID firmware error-handling path of `sev_gmem_post_populate()`.
--

commit 6fe2af9a226230e3ae153d850afdecbdb7d9f3cc
Author: Joerg Roedel <joerg.roedel@amd.com>

kvm: sev: Unmap pages in correct order in sev_gmem_post_populate()

This commit corrects the unmapping order of kmap_local() pages in the
initial setup path of sev_gmem_post_populate(), ensuring strict LIFO
ordering.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 41dcba5180cad..f09d15f68964b 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2360,8 +2360,8 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>  
>  		memcpy(dst_vaddr, src_vaddr, PAGE_SIZE);
>  
> -		kunmap_local(src_vaddr);
>  		kunmap_local(dst_vaddr);
> +		kunmap_local(src_vaddr);
>  	}
>  
>  	ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,

[Severity: High]
This is a pre-existing issue, but while this hunk fixes the unmapping order
in the initial setup path, does the identical LIFO unmapping violation still
exist further down in the CPUID firmware error-handling path of
sev_gmem_post_populate()?

If an untrusted userspace process supplies invalid CPUID data during
KVM_SEV_SNP_LAUNCH_UPDATE, the firmware returns SEV_RET_INVALID_PARAM.

This directly reaches the error path where src_vaddr and dst_vaddr are
mapped, but unmapped out of order:

arch/x86/kvm/svm/sev.c:sev_gmem_post_populate() {
    ...
	if (ret && !snp_page_reclaim(kvm, pfn) &&
	    sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_CPUID &&
	    sev_populate_args->fw_error == SEV_RET_INVALID_PARAM) {
		void *src_vaddr = kmap_local_page(src_page);
		void *dst_vaddr = kmap_local_pfn(pfn);

		memcpy(src_vaddr, dst_vaddr, PAGE_SIZE);

		kunmap_local(src_vaddr);
		kunmap_local(dst_vaddr);
	}
    ...
}

Should this second instance be updated to reverse the unmap order as well to
prevent kmap stack corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623091556.1500930-1-joro@8bytes.org?part=2

Re: [PATCH 2/4] kvm: sev: Unmap pages in correct order in sev_gmem_post_populate()

[Jörg Rödel]

On Tue, Jun 23, 2026 at 09:24:16AM +0000, sashiko-bot@kernel.org wrote:
> Should this second instance be updated to reverse the unmap order as well to
> prevent kmap stack corruption?

Yes, it should. Updated for v2.

Re: [PATCH 1/4] kvm: sev: Fix user-space triggerable WARN_ON on snp_launch_update path

[sashiko-bot]

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

Pre-existing issues:
- [High] `snp_launch_update` unconditionally returns `-EIO` when `kvm_gmem_populate` fails, discarding critical error codes like `-EINTR`, `-EFAULT`, and `-ENOMEM`.
- [High] `sev_gmem_post_populate` unmaps `kmap_local` pointers in the wrong order in the CPUID firmware error handling block.
--

commit 9048d7bc73d4ef4c2ccbf4550c4b2feff1f1822d
Author: Joerg Roedel <joerg.roedel@amd.com>

kvm: sev: Fix user-space triggerable WARN_ON on snp_launch_update path

This commit avoids a WARN_ON_ONCE in the snp_launch_update() path by explicitly
checking if a user-space process passed a NULL source address when the page
type is not KVM_SEV_SNP_PAGE_TYPE_ZERO.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 6c6a6d663e296..41dcba5180cad 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2438,6 +2438,13 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	if (!PAGE_ALIGNED(src))
>  		return -EINVAL;
>  
> +	/*
> +	 * Make sure user-mode did not pass NULL as src with
> +	 * type != KVM_SEV_SNP_PAGE_TYPE_ZERO.
> +	 */
> +	if (src == NULL && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
> +		return -EINVAL;
> +
>  	npages = params.len / PAGE_SIZE;

[Severity: High]
This is a pre-existing issue, but does snp_launch_update() unconditionally
return -EIO when kvm_gmem_populate() fails?

arch/x86/kvm/svm/sev.c:snp_launch_update() {
...
	count = kvm_gmem_populate(kvm, params.gfn_start, src, npages, write,
				  sev_gmem_post_populate, &sev_populate_args);
	if (count < 0) {
		argp->error = sev_populate_args.fw_error;
		pr_debug("%s: kvm_gmem_populate failed, ret %ld (fw_error %d)\n",
			 __func__, count, argp->error);
		return -EIO;
	}
...
}

Returning -EIO unconditionally here discards important error codes like
-EINTR, -EFAULT, and -ENOMEM from kvm_gmem_populate().

A userspace process could receive a signal (e.g. SIGALRM) during the ioctl
execution, causing kvm_gmem_populate() to return -EINTR, which gets
overwritten with -EIO.

This breaks standard Linux signal restart semantics and could cause VM
creation to fail under signal pressure.

[Severity: High]
This is also a pre-existing issue, but are the kmap_local pointers unmapped
in the wrong order in sev_gmem_post_populate()?

arch/x86/kvm/svm/sev.c:sev_gmem_post_populate() {
...
	if (ret && !snp_page_reclaim(kvm, pfn) &&
	    sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_CPUID &&
	    sev_populate_args->fw_error == SEV_RET_INVALID_PARAM) {
		void *src_vaddr = kmap_local_page(src_page);
		void *dst_vaddr = kmap_local_pfn(pfn);

		memcpy(src_vaddr, dst_vaddr, PAGE_SIZE);

		kunmap_local(src_vaddr);
		kunmap_local(dst_vaddr);
	}
...
}

kunmap_local() requires pointers to be unmapped in strictly reverse LIFO
order.

Here, src_vaddr is mapped first, then dst_vaddr, but src_vaddr is also
unmapped first.

This violates the API requirement and can trigger kernel panics via BUG_ON()
when CONFIG_DEBUG_KMAP_LOCAL is enabled.

While an earlier patch in this series ('kvm: sev: Unmap pages in correct
order in sev_gmem_post_populate()') fixed this in the first error block of
the function, it looks like this second error block was missed.

This can be triggered by a userspace VMM passing invalid CPUID data to
KVM_SEV_SNP_LAUNCH_UPDATE, causing firmware to reject the page.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623091556.1500930-1-joro@8bytes.org?part=1

Re: [PATCH 3/4] KVM: guest_memfd: Add `write` parameter to kvm_gmem_populate()

[sashiko-bot]

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

New issues:
- [Low] The integer `write` parameter is passed directly as the `gup_flags` bitmask to `get_user_pages_fast()`, treating a bitmask as a boolean.

Pre-existing issues:
- [Medium] Modified user pages acquired via `get_user_pages_fast()` are not marked dirty before their reference is dropped, leading to potential data loss.
--

commit 49441863078cd5114f78eb8c75f1d31489417864
Author: Joerg Roedel <joerg.roedel@amd.com>

KVM: guest_memfd: Add write parameter to kvm_gmem_populate()

This commit adds a write parameter to kvm_gmem_populate() to optionally
acquire a writable reference to the user-space page via get_user_pages_fast().
This ensures page permissions are enforced when KVM needs to write error
details back to userspace.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 69c9d6d546b28..7a245a402a1b2 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
[ ... ]
> @@ -893,7 +893,7 @@ 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;
>  
> -			ret = get_user_pages_fast(uaddr, 1, 0, &src_page);
> +			ret = get_user_pages_fast(uaddr, 1, write, &src_page);

[Severity: Low]
Does this pass an integer parameter directly into the gup_flags argument?
While the integer value 1 coincidentally evaluates to FOLL_WRITE (1 << 0),
is it safe to rely on the numeric value of an internal macro definition here?
Would it be more robust to explicitly pass FOLL_WRITE based on the write
parameter?

[Severity: Medium]
This is a pre-existing issue, but since we are explicitly handling writable
pages here, are we missing a dirty page notification before the reference
is dropped?

If a user-provided CPUID page fails validation during a SEV-SNP VM launch,
sev_gmem_post_populate() modifies the page directly via memcpy() bypassing
the user page tables. However, kvm_gmem_populate() drops the reference
shortly after:

virt/kvm/guest_memfd.c:kvm_gmem_populate() {
    ...
    ret = __kvm_gmem_populate(...);
    
    if (src_page)
        put_page(src_page);
    ...
}

Does this drop the reference without calling set_page_dirty_lock() or an
equivalent dirtying function? If so, could the memory management subsystem
reclaim the unmarked page and cause userspace to read stale data instead of
the detailed error information?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623091556.1500930-1-joro@8bytes.org?part=3

Re: [PATCH 4/4] kvm: sev: Acquire a writeable page reference for CPUID pages

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Incorrect unmap order for `kmap_local` mappings in the CPUID error reporting path leads to host kernel crashes.
- [High] Missing dirty tracking for user pages modified by the kernel, leading to potential silent data loss.
- [High] Data race and Use-After-Free on `kvm_memory_slot` between memslot teardown and `kvm_gmem_error_folio`.
--

commit 53ff0fbc410035f0e257e032a520eacbc8269a5c
Author: Joerg Roedel <joerg.roedel@amd.com>

kvm: sev: Acquire a writeable page reference for CPUID pages

When the PSP checks on a user-provided CPUID page fail KVM will write
back the detailed error information to the user-provided buffer.

Make sure this buffer is actually writable to not write the errors to
a read-only page.

Fixes: 2a62345b3052 ("KVM: guest_memfd: GUP source pages prior to populating guest memory")
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>

> 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
> @@ -2415,6 +2415,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	struct kvm_memory_slot *memslot;
>  	long npages, count;
>  	void __user *src;
> +	int write;
>  
>  	if (!sev_snp_guest(kvm) || !sev->snp_context)
>  		return -EINVAL;

[ ... ]

> @@ -2475,7 +2476,10 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	sev_populate_args.sev_fd = argp->sev_fd;
>  	sev_populate_args.type = params.type;
>  
> -	count = kvm_gmem_populate(kvm, params.gfn_start, src, npages, 0,
> +	/* Acquire a write-reference for CPUID pages as kernel might write to it */
> +	write = params.type == KVM_SEV_SNP_PAGE_TYPE_CPUID;
> +
> +	count = kvm_gmem_populate(kvm, params.gfn_start, src, npages, write,
>  				  sev_gmem_post_populate, &sev_populate_args);

[Severity: High]
This isn't a bug introduced by this patch, but does kvm_gmem_populate()
handle dirtying the page correctly after sev_gmem_post_populate() modifies it?

When a user-provided CPUID page triggers SEV_RET_INVALID_PARAM, the error
path modifies the page directly via memcpy():

arch/x86/kvm/svm/sev.c:sev_gmem_post_populate() {
    ...
    memcpy(src_vaddr, dst_vaddr, PAGE_SIZE);
    ...
}

Since this bypasses hardware PTE dirty tracking, and kvm_gmem_populate()
releases the page without marking it dirty:

virt/kvm/guest_memfd.c:kvm_gmem_populate() {
    ...
    if (src_page)
        put_page(src_page);
    ...
}

Could this result in the userspace VMM permanently losing the error details if
the memory is file-backed or pushed to swap and discarded without writeback?


[Severity: Critical]
This is a pre-existing issue, but does sev_gmem_post_populate() unmap pages
in the correct order?

If __sev_issue_cmd() fails, it maps src_vaddr and dst_vaddr, but appears to
unmap them in FIFO order:

arch/x86/kvm/svm/sev.c:sev_gmem_post_populate() {
    ...
    kunmap_local(src_vaddr);
    kunmap_local(dst_vaddr);
    ...
}

Since the kmap_local API strictly requires LIFO unmapping, could an untrusted
VMM intentionally passing an invalid CPUID page trigger a host kernel crash or
corrupt the per-CPU kmap_ctrl stack?


[Severity: High]
This is a pre-existing issue, but is there a data race and use-after-free
risk on kvm_memory_slot between memslot teardown and kvm_gmem_error_folio()?

When KVM destroys a memslot, kvm_gmem_unbind() fast-paths the teardown if
there is no file:

virt/kvm/guest_memfd.c:kvm_gmem_unbind() {
    if (!file) {
        __kvm_gmem_unbind(slot, slot->gmem.file->private_data);
        return;
    }
    ...
}

This deliberately skips acquiring filemap_invalidate_lock(file->f_mapping).

If a hardware memory failure concurrently triggers kvm_gmem_error_folio(),
it iterates over f->bindings holding only filemap_invalidate_lock_shared().

Because xa_for_each_range() drops the internal RCU lock for each element,
could it yield the slot pointer right as kvm_gmem_unbind() completes and
frees it, leading to a dereference of freed memory here?

virt/kvm/guest_memfd.c:__kvm_gmem_invalidate_begin() {
    ...
    .start = slot->base_gfn + max(pgoff, start) - pgoff,
    ...
}

>  	if (count < 0) {
>  		argp->error = sev_populate_args.fw_error;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623091556.1500930-1-joro@8bytes.org?part=4

Re: [PATCH 2/4] kvm: sev: Unmap pages in correct order in sev_gmem_post_populate()

[Sean Christopherson]

On Tue, Jun 23, 2026, Jörg Rödel wrote:
> On Tue, Jun 23, 2026 at 09:24:16AM +0000, sashiko-bot@kernel.org wrote:
> > Should this second instance be updated to reverse the unmap order as well to
> > prevent kmap stack corruption?
> 
> Yes, it should. Updated for v2.

Already fixed, commit 138f5f9cbe37 ("KVM: SEV: Unmap local kmaps in LIFO order,
per highmem requirements").

Re: [PATCH 3/4] KVM: guest_memfd: Add `write` parameter to kvm_gmem_populate()

[Sean Christopherson]

On Tue, Jun 23, 2026, Jörg Rödel wrote:
> From: Joerg Roedel <joerg.roedel@amd.com>
> 
> The call-path of kvm_gmem_populate() might subsequently write to the
> page provided by user-space. This is used to provide detailed error
> information in case the page population failed.
> 
> But since kvm_gmem_populate() only acquires a read-only reference to
> the user-space page via get_user_pages_fast(), the error information
> might be written to a read-only page later on.
> 
> Add a parameter to kvm_gmem_populate() to optionally acquire a
> writeable reference to the source page to make sure page permissions
> can be enforced.

Already fixed, commit f13e90059908 ("KVM: SEV: Pin source page for write when
adding CPUID data for SNP guest").

Re: [PATCH 4/4] kvm: sev: Acquire a writeable page reference for CPUID pages

[Ackerley Tng]

sashiko-bot@kernel.org writes:

>
> [...snip...]
>

Thanks Jorg, commit f13e90059908 ("KVM: SEV: Pin source page for write
when adding CPUID data for SNP guest") is a combination of this patch
and the previous, so this is fixed too.

This report caught my attention:

> [Severity: High]
> This is a pre-existing issue, but is there a data race and use-after-free
> risk on kvm_memory_slot between memslot teardown and kvm_gmem_error_folio()?
>
> When KVM destroys a memslot, kvm_gmem_unbind() fast-paths the teardown if
> there is no file:
>
> virt/kvm/guest_memfd.c:kvm_gmem_unbind() {
>     if (!file) {
>         __kvm_gmem_unbind(slot, slot->gmem.file->private_data);
>         return;
>     }
>     ...
> }
>
> This deliberately skips acquiring filemap_invalidate_lock(file->f_mapping).
>

Here's how kvm_gmem_unbind() relates to kvm_gmem_release():

This unbinding without filemap_invalidate_lock() covers the case where

+ During VM destruction, the memslots are being destroyed:
  kvm_gmem_unbind() is reached without holding slots_lock

and

+ There's a concurrent guest_memfd kvm_gmem_release()

slot->gmem.file is not NULL (yet, kvm_gmem_release() would set that),
but the file is already closed, hence kvm_gmem_get_file() returns
NULL. (See CLASS(gmem_get_file, file)(slot);).

Since kvm_gmem_get_file() returns NULL, there's no way to safely take
the filemap_invalidate_lock(), but we must still do the
unbinding. Without unbinding, kvm_gmem_release() would iterate
f->bindings and use slot after free.

> If a hardware memory failure concurrently triggers kvm_gmem_error_folio(),
> it iterates over f->bindings holding only filemap_invalidate_lock_shared().
>

To get to the above unbinding without holding filemap_invalidate_lock(),
the file would have to already be closed. If the file is closed, the
inode (and folios) can still be alive. Memory failure can look up the
folios and hence the inode.

But to reach __kvm_gmem_invalidate_begin() from kvm_gmem_error_folio(),
we'd have to iterate the files associated with the inode
(kvm_gmem_for_each_file() in kvm_gmem_invalidate()). Since the file is
already closed to unbind without filemap_invalidate_lock(), the
kvm_gmem_error_folio() side won't see the file.

> Because xa_for_each_range() drops the internal RCU lock for each element,
> could it yield the slot pointer right as kvm_gmem_unbind() completes and
> frees it, leading to a dereference of freed memory here?
>

No issue here. Phew.

> virt/kvm/guest_memfd.c:__kvm_gmem_invalidate_begin() {
>     ...
>     .start = slot->base_gfn + max(pgoff, start) - pgoff,
>     ...
> }
>
>>  	if (count < 0) {
>>  		argp->error = sev_populate_args.fw_error;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260623091556.1500930-1-joro@8bytes.org?part=4

Re: [PATCH 1/4] kvm: sev: Fix user-space triggerable WARN_ON on snp_launch_update path

[Sean Christopherson]

Please capitalize the scope, i.e. "KVM: SEV:".

On Tue, Jun 23, 2026, Jörg Rödel wrote:
> From: Joerg Roedel <joerg.roedel@amd.com>
> 
> Sashiko reported on an unrelated patch:
> 
>   [Severity: High]
>   This is a pre-existing issue, but can a host userspace process trigger a
>   kernel warning by passing a NULL user address (uaddr = 0) here?
> 
>   If params.uaddr is 0, src becomes NULL and passes the PAGE_ALIGNED(src)
>   check. kvm_gmem_populate() skips fetching the user page and passes
>   src_page = NULL to sev_gmem_post_populate().
> 
>   That function then unconditionally evaluates:
> 
>   WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO &&
>                !src_page)
> 
>   Since the type isn't ZERO, won't this allow an unprivileged user to spam
>   the kernel log?

Use Reported-by: + Closes to capture Sashiko's effecitve bug report instead of
copy+pasting the finding.  There's no reason to treat Sashiko any differently
than any other bot.

> The assessment is correct, so check for this condition earlier in the
> snp_launch_update() path to avoid the WARN_ON_ONCE.
> 
> Fixes: dee5a47cc7a45 ("KVM: SEV: Add KVM_SEV_SNP_LAUNCH_UPDATE command")
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/x86/kvm/svm/sev.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 6c6a6d663e29..41dcba5180ca 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2438,6 +2438,13 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	if (!PAGE_ALIGNED(src))
>  		return -EINVAL;
>  
> +	/*
> +	 * Make sure user-mode did not pass NULL as src with
> +	 * type != KVM_SEV_SNP_PAGE_TYPE_ZERO.

Meh, that's pretty obvious from the code.

> +	 */
> +	if (src == NULL && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)

I think I'd prefer this over checking for KVM_SEV_SNP_PAGE_TYPE_ZERO twice,
especially since the PAGE_ALIGNED() check for the NULL pointer case is rather
weird.

	if (params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO)
		src = NULL;
	else if (!params.uaddr || !PAGE_ALIGNED(params.uaddr))
		return -EINVAL;
	else
		src = u64_to_user_ptr(params.uaddr);


> +		return -EINVAL;

Gah, we created quite the mess for ourselves.  TDX returns -EOPNOTSUPP instead
of -EINVAL, I guess as a placeholder for in-place conversion?  I don't care which
error code is returned, but I do want KVM to be consistent.

We should also adjust TDX to pre-check the source address, because checking only
in the post-populate flow subtly relies on tdx_vcpu_init_mem_region() returning
immediately on error.  If that weren't the case (ignoring for the moment that
continuing on would be nonsensical), KVM would advace the address by PAGE_SIZE
and suddenly a NULL userspace address becomes non-NULL.

I also think it makes sense to drop the WARN in sev_gmem_post_populate(), it's
completely redundant once this bug is fixed.

Ugh, and both SNP and TDX fail to account for tags, and fail to check for
striding into kernel space.  Which I suppose is fine, since gup() handles those
correctly.  And I don't see a strong argument for disallowing tagged addresses,
because unlike the userspace address for memslots, KVM doesn't keep the address
around long-term.

So over two patches, the below?  I can send a v2, I've already got changelogs
written (I was fiddling around with extracting and reusing kvm_set_memory_region()'s
checks on the userspace address+size, but as above, convinced myself that KVM
should continue to allow tagged addresses for SNP and TDX).

---
 arch/x86/kvm/svm/sev.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 74fb15551e83..621a2eaa58f2 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2330,9 +2330,6 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 	int level;
 	int ret;
 
-	if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page))
-		return -EINVAL;
-
 	ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
 	if (ret || assigned) {
 		pr_debug("%s: Failed to ensure GFN 0x%llx RMP entry is initial shared state, ret: %d assigned: %d\n",
@@ -2421,10 +2418,12 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	     params.type != KVM_SEV_SNP_PAGE_TYPE_CPUID))
 		return -EINVAL;
 
-	src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
-
-	if (!PAGE_ALIGNED(src))
+	if (params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO)
+		src = NULL;
+	else if (!params.uaddr || !PAGE_ALIGNED(params.uaddr))
 		return -EINVAL;
+	else
+		src = u64_to_user_ptr(params.uaddr);
 
 	npages = params.len / PAGE_SIZE;

---
 arch/x86/kvm/vmx/tdx.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index ffe9d0db58c5..b0ec054732b9 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -3198,9 +3198,6 @@ static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 	if (KVM_BUG_ON(kvm_tdx->page_add_src, kvm))
 		return -EIO;
 
-	if (!src_page)
-		return -EOPNOTSUPP;
-
 	kvm_tdx->page_add_src = src_page;
 	ret = kvm_tdp_mmu_map_private_pfn(arg->vcpu, gfn, pfn);
 	kvm_tdx->page_add_src = NULL;
@@ -3247,8 +3244,8 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *c
 	if (copy_from_user(&region, u64_to_user_ptr(cmd->data), sizeof(region)))
 		return -EFAULT;
 
-	if (!PAGE_ALIGNED(region.source_addr) || !PAGE_ALIGNED(region.gpa) ||
-	    !region.nr_pages ||
+	if (!PAGE_ALIGNED(region.source_addr) || !region.source_addr ||
+	    !PAGE_ALIGNED(region.gpa) || !region.nr_pages ||
 	    region.gpa + (region.nr_pages << PAGE_SHIFT) <= region.gpa ||
 	    !vt_is_tdx_private_gpa(kvm, region.gpa) ||
 	    !vt_is_tdx_private_gpa(kvm, region.gpa + (region.nr_pages << PAGE_SHIFT) - 1))
--