[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