[PATCH] KVM: SEV: Fix page dirtying in sev_gmem_post_populate()

[PATCH] KVM: SEV: Fix page dirtying in sev_gmem_post_populate()

[Jann Horn]

set_page_dirty() requires that the caller holds some kind of lock to
ensure that the page's mapping does not concurrently go away.
That is not the case for a random page we got from get_user_pages_fast(),
so use set_page_dirty_lock().

Fixes: 97cd21d57e9b ("KVM: SEV: Mark source page dirty when writing back CPUID data on failure")
Signed-off-by: Jann Horn <jannh@google.com>
---
Note that I have only convinced myself that this is a UAF bug by reading
the source code, and I have only compile-tested this patch; I do not
have SEV-SNP hardware to actually test this.

This bug was introduced in v7.2-rc1, so if we can get the fix in before
the 7.2 release, this won't require a stable backport.

For context, I have seen similar bugs (set_page_dirty() on a page from
GUP) in two out-of-tree drivers in the past, where this led to UAF.
Evidently set_page_dirty() is a bit of a footgun for GUP users...
---
 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 0f0ea7896af5..49feef14787a 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2389,7 +2389,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 		void *dst_vaddr = kmap_local_pfn(pfn);
 
 		memcpy(src_vaddr, dst_vaddr, PAGE_SIZE);
-		set_page_dirty(src_page);
+		set_page_dirty_lock(src_page);
 
 		kunmap_local(dst_vaddr);
 		kunmap_local(src_vaddr);

---
base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
change-id: 20260810-x86-kvm-setpagedirty-a04e49f066f0

Best regards,
--  
Jann Horn <jannh@google.com>

Re: [PATCH] KVM: SEV: Fix page dirtying in sev_gmem_post_populate()

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Leaked SEV-SNP pages (via snp_leak_pages) are incorrectly returned to the buddy allocator, causing host panics (RMP violations) upon reuse.
- [High] Unprivileged host userspace can trigger a kernel warning (WARN_ON_ONCE) in sev_gmem_post_populate() by passing a NULL user address.
--

--- Patch [1]: [PATCH] KVM: SEV: Fix page dirtying in sev_gmem_post_populate() ---
commit 6e5ffd2f92a045c6125ad2d714ea329e1a9b8c9a
Author: Jann Horn <jannh@google.com>

KVM: SEV: Fix page dirtying in sev_gmem_post_populate()

This patch fixes a potential use-after-free by replacing set_page_dirty()
with set_page_dirty_lock() for pages obtained from get_user_pages_fast().
This ensures the page's mapping cannot concurrently go away during the
dirtying process.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 0f0ea7896af5b..49feef14787af 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2337,7 +2337,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>  	bool assigned = false;
>  	int level;
>  	int ret;
>  
>  	if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page))

[Severity: High]
This isn't a bug introduced by this patch, but can an unprivileged host
userspace process trigger this WARN_ON_ONCE by passing a NULL user address?

If a userspace process invokes the KVM_SEV_SNP_LAUNCH_UPDATE ioctl with
params.type == KVM_SEV_SNP_PAGE_TYPE_CPUID and params.uaddr == 0, the
snp_launch_update() function computes src = NULL and bypasses the
PAGE_ALIGNED check:

arch/x86/kvm/svm/sev.c:snp_launch_update() {
    src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL :
          u64_to_user_ptr(params.uaddr);
    if (!PAGE_ALIGNED(src))
        return -EINVAL;
}

Since 0 is page-aligned, src remains NULL. When kvm_gmem_populate() is
subsequently called, it skips fetching user pages because src is NULL:

virt/kvm/guest_memfd.c:kvm_gmem_populate() {
    if (src) {
        unsigned long uaddr = (unsigned long)src + i * PAGE_SIZE;
        ...
    }
}

This leaves src_page as NULL when it is passed to sev_gmem_post_populate(),
which triggers the WARN_ON_ONCE. Does this allow a malicious user to trigger
a host-side denial of service if the kernel boots with panic_on_warn?

>  		return -EINVAL;

[ ... ]

> @@ -2381,15 +2381,15 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>  	 * Copy this page back into the source buffer so userspace can use this
>  	 * information to provide information on which CPUID leaves/fields
>  	 * failed CPUID validation.
>  	 */
>  	if (ret && !snp_page_reclaim(kvm, pfn) &&

[Severity: Critical]
This is a pre-existing issue, but does snp_page_reclaim() properly manage
reference counts for leaked pages?

If snp_page_reclaim() fails, it permanently leaks the page via
snp_leak_pages(). However, the page is added to the snp_leaked_pages_list
without taking an additional reference:

arch/x86/virt/svm/sev.c:__snp_leak_pages() {
    if (likely(!PageCompound(page)) || ...)
        list_add_tail(&page->buddy_list, &snp_leaked_pages_list);
}

For pages from guest_memfd, the owning subsystem controls the reference. When
operations drop references to the leaked folio (such as when the file is
closed or truncated), the reference count can drop to zero:

virt/kvm/guest_memfd.c:__kvm_gmem_populate() {
out_put_folio:
    folio_put(folio);
}

Could this cause the leaked SEV-SNP page to be erroneously returned to the
buddy allocator while still in a hardware-enforced private state, leading to
fatal RMP violations upon subsequent reuse?

>  	    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);
> -		set_page_dirty(src_page);
> +		set_page_dirty_lock(src_page);
>  
>  		kunmap_local(dst_vaddr);
>  		kunmap_local(src_vaddr);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-x86-kvm-setpagedirty-v1-1-85f180892d4f@google.com?part=1

Re: [PATCH] KVM: SEV: Fix page dirtying in sev_gmem_post_populate()

[Sean Christopherson]

On Mon, Aug 10, 2026, Jann Horn wrote:
> set_page_dirty() requires that the caller holds some kind of lock to
> ensure that the page's mapping does not concurrently go away.
> That is not the case for a random page we got from get_user_pages_fast(),
> so use set_page_dirty_lock().

FWIW, I verified that __kvm_gmem_populate() unlocks the destination folio before
post_populate(), so we shouldn't run into deadlock when in-place conversion comes
along and userspace does something odd.

> Fixes: 97cd21d57e9b ("KVM: SEV: Mark source page dirty when writing back CPUID data on failure")
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> Note that I have only convinced myself that this is a UAF bug by reading
> the source code, and I have only compile-tested this patch; I do not
> have SEV-SNP hardware to actually test this.
> 
> This bug was introduced in v7.2-rc1, so if we can get the fix in before
> the 7.2 release, this won't require a stable backport.

Ya, that would be lovely.  Paolo?

> For context, I have seen similar bugs (set_page_dirty() on a page from
> GUP) in two out-of-tree drivers in the past, where this led to UAF.
> Evidently set_page_dirty() is a bit of a footgun for GUP users...

Heh, and that's a bit of an understatement :-D

Reviewed-by: Sean Christopherson <seanjc@google.com>