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

[sashiko-bot@kernel.org]

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